From 597e2953ae9b4a391e883c1f1a4cda5878e2dbed Mon Sep 17 00:00:00 2001 From: Yue Hu Date: Wed, 26 Apr 2023 16:44:49 +0800 Subject: erofs: fold in z_erofs_decompress() No need this helper since it's just a simple wrapper for decompress method and only one caller. So, let's fold in directly instead. Signed-off-by: Yue Hu Reviewed-by: Gao Xiang Link: https://lore.kernel.org/r/20230426084449.12781-1-zbestahu@gmail.com Signed-off-by: Gao Xiang --- fs/erofs/compress.h | 3 +-- fs/erofs/decompressor.c | 8 +------- fs/erofs/zdata.c | 4 +++- 3 files changed, 5 insertions(+), 10 deletions(-) (limited to 'fs') diff --git a/fs/erofs/compress.h b/fs/erofs/compress.h index 26fa170090b8..b1b846504027 100644 --- a/fs/erofs/compress.h +++ b/fs/erofs/compress.h @@ -89,8 +89,7 @@ static inline bool erofs_page_is_managed(const struct erofs_sb_info *sbi, int z_erofs_fixup_insize(struct z_erofs_decompress_req *rq, const char *padbuf, unsigned int padbufsize); -int z_erofs_decompress(struct z_erofs_decompress_req *rq, - struct page **pagepool); +extern const struct z_erofs_decompressor erofs_decompressors[]; /* prototypes for specific algorithms */ int z_erofs_lzma_decompress(struct z_erofs_decompress_req *rq, diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c index 7021e2cf6146..2a29943fa5cc 100644 --- a/fs/erofs/decompressor.c +++ b/fs/erofs/decompressor.c @@ -363,7 +363,7 @@ static int z_erofs_transform_plain(struct z_erofs_decompress_req *rq, return 0; } -static struct z_erofs_decompressor decompressors[] = { +const struct z_erofs_decompressor erofs_decompressors[] = { [Z_EROFS_COMPRESSION_SHIFTED] = { .decompress = z_erofs_transform_plain, .name = "shifted" @@ -383,9 +383,3 @@ static struct z_erofs_decompressor decompressors[] = { }, #endif }; - -int z_erofs_decompress(struct z_erofs_decompress_req *rq, - struct page **pagepool) -{ - return decompressors[rq->alg].decompress(rq, pagepool); -} diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index 160b3da43aec..be8d28631d77 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -1283,6 +1283,8 @@ static int z_erofs_decompress_pcluster(struct z_erofs_decompress_backend *be, struct erofs_sb_info *const sbi = EROFS_SB(be->sb); struct z_erofs_pcluster *pcl = be->pcl; unsigned int pclusterpages = z_erofs_pclusterpages(pcl); + const struct z_erofs_decompressor *decompressor = + &erofs_decompressors[pcl->algorithmformat]; unsigned int i, inputsize; int err2; struct page *page; @@ -1326,7 +1328,7 @@ static int z_erofs_decompress_pcluster(struct z_erofs_decompress_backend *be, else inputsize = pclusterpages * PAGE_SIZE; - err = z_erofs_decompress(&(struct z_erofs_decompress_req) { + err = decompressor->decompress(&(struct z_erofs_decompress_req) { .sb = be->sb, .in = be->compressed_pages, .out = be->decompressed_pages, -- cgit v1.2.3 From ef4b4b46c6aaf8edeea9a79320627fe10993f153 Mon Sep 17 00:00:00 2001 From: Yue Hu Date: Wed, 24 May 2023 14:39:44 +0800 Subject: erofs: remove the member readahead from struct z_erofs_decompress_frontend The struct member is only used to add REQ_RAHEAD during I/O submission. So it is cleaner to pass it as a parameter than keep it in the struct. Also, rename function z_erofs_get_sync_decompress_policy() to z_erofs_is_sync_decompress() for better clarity and conciseness. Signed-off-by: Yue Hu Reviewed-by: Gao Xiang Link: https://lore.kernel.org/r/20230524063944.1655-1-zbestahu@gmail.com Signed-off-by: Gao Xiang --- fs/erofs/zdata.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'fs') diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index be8d28631d77..4402c2366fe1 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -548,7 +548,6 @@ struct z_erofs_decompress_frontend { z_erofs_next_pcluster_t owned_head; enum z_erofs_pclustermode mode; - bool readahead; /* used for applying cache strategy on the fly */ bool backmost; erofs_off_t headoffset; @@ -1104,7 +1103,7 @@ out: return err; } -static bool z_erofs_get_sync_decompress_policy(struct erofs_sb_info *sbi, +static bool z_erofs_is_sync_decompress(struct erofs_sb_info *sbi, unsigned int readahead_pages) { /* auto: enable for read_folio, disable for readahead */ @@ -1672,7 +1671,7 @@ static void z_erofs_decompressqueue_endio(struct bio *bio) static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f, struct page **pagepool, struct z_erofs_decompressqueue *fgq, - bool *force_fg) + bool *force_fg, bool readahead) { struct super_block *sb = f->inode->i_sb; struct address_space *mc = MNGD_MAPPING(EROFS_SB(sb)); @@ -1763,7 +1762,7 @@ submit_bio_retry: bio->bi_iter.bi_sector = (sector_t)cur << (sb->s_blocksize_bits - 9); bio->bi_private = q[JQ_SUBMIT]; - if (f->readahead) + if (readahead) bio->bi_opf |= REQ_RAHEAD; ++nr_bios; } @@ -1799,13 +1798,13 @@ submit_bio_retry: } static void z_erofs_runqueue(struct z_erofs_decompress_frontend *f, - struct page **pagepool, bool force_fg) + struct page **pagepool, bool force_fg, bool ra) { struct z_erofs_decompressqueue io[NR_JOBQUEUES]; if (f->owned_head == Z_EROFS_PCLUSTER_TAIL) return; - z_erofs_submit_queue(f, pagepool, io, &force_fg); + z_erofs_submit_queue(f, pagepool, io, &force_fg, ra); /* handle bypass queue (no i/o pclusters) immediately */ z_erofs_decompress_queue(&io[JQ_BYPASS], pagepool); @@ -1903,8 +1902,8 @@ static int z_erofs_read_folio(struct file *file, struct folio *folio) (void)z_erofs_collector_end(&f); /* if some compressed cluster ready, need submit them anyway */ - z_erofs_runqueue(&f, &pagepool, - z_erofs_get_sync_decompress_policy(sbi, 0)); + z_erofs_runqueue(&f, &pagepool, z_erofs_is_sync_decompress(sbi, 0), + false); if (err) erofs_err(inode->i_sb, "failed to read, err [%d]", err); @@ -1922,7 +1921,6 @@ static void z_erofs_readahead(struct readahead_control *rac) struct page *pagepool = NULL, *head = NULL, *page; unsigned int nr_pages; - f.readahead = true; f.headoffset = readahead_pos(rac); z_erofs_pcluster_readmore(&f, rac, f.headoffset + @@ -1953,7 +1951,7 @@ static void z_erofs_readahead(struct readahead_control *rac) (void)z_erofs_collector_end(&f); z_erofs_runqueue(&f, &pagepool, - z_erofs_get_sync_decompress_policy(sbi, nr_pages)); + z_erofs_is_sync_decompress(sbi, nr_pages), true); erofs_put_metabuf(&f.map.buf); erofs_release_pages(&pagepool); } -- cgit v1.2.3 From 796e9149a2fcdba5543e247abd8d911a399bb9a6 Mon Sep 17 00:00:00 2001 From: Yue Hu Date: Thu, 25 May 2023 15:26:05 +0800 Subject: erofs: clean up z_erofs_pcluster_readmore() `end` parameter is no needed since it's pointless for !backmost, we can handle it with backmost internally. And we only expand the trailing edge, so the newstart can be replaced with ->headoffset. Also, remove linux/prefetch.h inclusion since that is not used anymore after commit 386292919c25 ("erofs: introduce readmore decompression strategy"). Signed-off-by: Yue Hu Reviewed-by: Gao Xiang Link: https://lore.kernel.org/r/20230525072605.17857-1-zbestahu@gmail.com [ Gao Xiang: update commit description. ] Signed-off-by: Gao Xiang --- fs/erofs/zdata.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) (limited to 'fs') diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index 4402c2366fe1..305426a71792 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -5,7 +5,6 @@ * Copyright (C) 2022 Alibaba Cloud */ #include "compress.h" -#include #include #include #include @@ -1825,28 +1824,28 @@ static void z_erofs_runqueue(struct z_erofs_decompress_frontend *f, */ static void z_erofs_pcluster_readmore(struct z_erofs_decompress_frontend *f, struct readahead_control *rac, - erofs_off_t end, - struct page **pagepool, - bool backmost) + struct page **pagepool, bool backmost) { struct inode *inode = f->inode; struct erofs_map_blocks *map = &f->map; - erofs_off_t cur; + erofs_off_t cur, end, headoffset = f->headoffset; int err; if (backmost) { + if (rac) + end = headoffset + readahead_length(rac) - 1; + else + end = headoffset + PAGE_SIZE - 1; map->m_la = end; err = z_erofs_map_blocks_iter(inode, map, EROFS_GET_BLOCKS_READMORE); if (err) return; - /* expend ra for the trailing edge if readahead */ + /* expand ra for the trailing edge if readahead */ if (rac) { - loff_t newstart = readahead_pos(rac); - cur = round_up(map->m_la + map->m_llen, PAGE_SIZE); - readahead_expand(rac, newstart, cur - newstart); + readahead_expand(rac, headoffset, cur - headoffset); return; } end = round_up(end, PAGE_SIZE); @@ -1894,10 +1893,9 @@ static int z_erofs_read_folio(struct file *file, struct folio *folio) trace_erofs_readpage(page, false); f.headoffset = (erofs_off_t)page->index << PAGE_SHIFT; - z_erofs_pcluster_readmore(&f, NULL, f.headoffset + PAGE_SIZE - 1, - &pagepool, true); + z_erofs_pcluster_readmore(&f, NULL, &pagepool, true); err = z_erofs_do_read_page(&f, page, &pagepool); - z_erofs_pcluster_readmore(&f, NULL, 0, &pagepool, false); + z_erofs_pcluster_readmore(&f, NULL, &pagepool, false); (void)z_erofs_collector_end(&f); @@ -1923,8 +1921,7 @@ static void z_erofs_readahead(struct readahead_control *rac) f.headoffset = readahead_pos(rac); - z_erofs_pcluster_readmore(&f, rac, f.headoffset + - readahead_length(rac) - 1, &pagepool, true); + z_erofs_pcluster_readmore(&f, rac, &pagepool, true); nr_pages = readahead_count(rac); trace_erofs_readpages(inode, readahead_index(rac), nr_pages, false); @@ -1947,7 +1944,7 @@ static void z_erofs_readahead(struct readahead_control *rac) page->index, EROFS_I(inode)->nid); put_page(page); } - z_erofs_pcluster_readmore(&f, rac, 0, &pagepool, false); + z_erofs_pcluster_readmore(&f, rac, &pagepool, false); (void)z_erofs_collector_end(&f); z_erofs_runqueue(&f, &pagepool, -- cgit v1.2.3 From 05b63d2beb8b0f752d1f5cdd051c8bdbf532cedd Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Sat, 27 May 2023 04:14:54 +0800 Subject: erofs: allocate extra bvec pages directly instead of retrying If non-bootstrap bvecs cannot be kept in place (very rarely), an extra short-lived page is allocated. Let's just allocate it immediately rather than do unnecessary -EAGAIN return first and retry as a cleanup. Also it's unnecessary to use __GFP_NOFAIL here since we could gracefully fail out this case instead. Signed-off-by: Gao Xiang Reviewed-by: Yue Hu Link: https://lore.kernel.org/r/20230526201459.128169-2-hsiangkao@linux.alibaba.com --- fs/erofs/zdata.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) (limited to 'fs') diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index 305426a71792..9d04fa29b742 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -242,12 +242,17 @@ static int z_erofs_bvec_enqueue(struct z_erofs_bvec_iter *iter, struct z_erofs_bvec *bvec, struct page **candidate_bvpage) { - if (iter->cur == iter->nr) { - if (!*candidate_bvpage) - return -EAGAIN; - + if (iter->cur >= iter->nr) { + struct page *nextpage = *candidate_bvpage; + + if (!nextpage) { + nextpage = alloc_page(GFP_NOFS); + if (!nextpage) + return -ENOMEM; + set_page_private(nextpage, Z_EROFS_SHORTLIVED_PAGE); + } DBG_BUGON(iter->bvset->nextpage); - iter->bvset->nextpage = *candidate_bvpage; + iter->bvset->nextpage = nextpage; z_erofs_bvset_flip(iter); iter->bvset->nextpage = NULL; @@ -906,10 +911,8 @@ static bool z_erofs_collector_end(struct z_erofs_decompress_frontend *fe) z_erofs_bvec_iter_end(&fe->biter); mutex_unlock(&pcl->lock); - if (fe->candidate_bvpage) { - DBG_BUGON(z_erofs_is_shortlived_page(fe->candidate_bvpage)); + if (fe->candidate_bvpage) fe->candidate_bvpage = NULL; - } /* * if all pending pages are added, don't hold its reference @@ -1054,24 +1057,13 @@ hitted: if (cur) tight &= (fe->mode >= Z_EROFS_PCLUSTER_FOLLOWED); -retry: err = z_erofs_attach_page(fe, &((struct z_erofs_bvec) { .page = page, .offset = offset - map->m_la, .end = end, }), exclusive); - /* should allocate an additional short-lived page for bvset */ - if (err == -EAGAIN && !fe->candidate_bvpage) { - fe->candidate_bvpage = alloc_page(GFP_NOFS | __GFP_NOFAIL); - set_page_private(fe->candidate_bvpage, - Z_EROFS_SHORTLIVED_PAGE); - goto retry; - } - - if (err) { - DBG_BUGON(err == -EAGAIN && fe->candidate_bvpage); + if (err) goto out; - } z_erofs_onlinepage_split(page); /* bump up the number of spiltted parts of a page */ -- cgit v1.2.3 From 6ab5eed6002edc5a29b683285e90459a7df6ce2b Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Sat, 27 May 2023 04:14:55 +0800 Subject: erofs: avoid on-stack pagepool directly passed by arguments On-stack pagepool is used so that short-lived temporary pages could be shared within a single I/O request (e.g. among multiple pclusters). Moving the remaining frontend-related uses into z_erofs_decompress_frontend to avoid too many arguments. Signed-off-by: Gao Xiang Reviewed-by: Yue Hu Link: https://lore.kernel.org/r/20230526201459.128169-3-hsiangkao@linux.alibaba.com --- fs/erofs/zdata.c | 64 ++++++++++++++++++++++++++------------------------------ 1 file changed, 30 insertions(+), 34 deletions(-) (limited to 'fs') diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index 9d04fa29b742..22be768a5b8a 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -240,13 +240,14 @@ static void z_erofs_bvec_iter_begin(struct z_erofs_bvec_iter *iter, static int z_erofs_bvec_enqueue(struct z_erofs_bvec_iter *iter, struct z_erofs_bvec *bvec, - struct page **candidate_bvpage) + struct page **candidate_bvpage, + struct page **pagepool) { if (iter->cur >= iter->nr) { struct page *nextpage = *candidate_bvpage; if (!nextpage) { - nextpage = alloc_page(GFP_NOFS); + nextpage = erofs_allocpage(pagepool, GFP_NOFS); if (!nextpage) return -ENOMEM; set_page_private(nextpage, Z_EROFS_SHORTLIVED_PAGE); @@ -547,6 +548,7 @@ struct z_erofs_decompress_frontend { struct erofs_map_blocks map; struct z_erofs_bvec_iter biter; + struct page *pagepool; struct page *candidate_bvpage; struct z_erofs_pcluster *pcl, *tailpcl; z_erofs_next_pcluster_t owned_head; @@ -581,8 +583,7 @@ static bool z_erofs_should_alloc_cache(struct z_erofs_decompress_frontend *fe) return false; } -static void z_erofs_bind_cache(struct z_erofs_decompress_frontend *fe, - struct page **pagepool) +static void z_erofs_bind_cache(struct z_erofs_decompress_frontend *fe) { struct address_space *mc = MNGD_MAPPING(EROFS_I_SB(fe->inode)); struct z_erofs_pcluster *pcl = fe->pcl; @@ -623,7 +624,7 @@ static void z_erofs_bind_cache(struct z_erofs_decompress_frontend *fe, * succeeds or fallback to in-place I/O instead * to avoid any direct reclaim. */ - newpage = erofs_allocpage(pagepool, gfp); + newpage = erofs_allocpage(&fe->pagepool, gfp); if (!newpage) continue; set_page_private(newpage, Z_EROFS_PREALLOCATED_PAGE); @@ -636,7 +637,7 @@ static void z_erofs_bind_cache(struct z_erofs_decompress_frontend *fe, if (page) put_page(page); else if (newpage) - erofs_pagepool_add(pagepool, newpage); + erofs_pagepool_add(&fe->pagepool, newpage); } /* @@ -734,7 +735,8 @@ static int z_erofs_attach_page(struct z_erofs_decompress_frontend *fe, !fe->candidate_bvpage) fe->candidate_bvpage = bvec->page; } - ret = z_erofs_bvec_enqueue(&fe->biter, bvec, &fe->candidate_bvpage); + ret = z_erofs_bvec_enqueue(&fe->biter, bvec, &fe->candidate_bvpage, + &fe->pagepool); fe->pcl->vcnt += (ret >= 0); return ret; } @@ -959,7 +961,7 @@ static int z_erofs_read_fragment(struct inode *inode, erofs_off_t pos, } static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe, - struct page *page, struct page **pagepool) + struct page *page) { struct inode *const inode = fe->inode; struct erofs_map_blocks *const map = &fe->map; @@ -1017,7 +1019,7 @@ repeat: fe->mode = Z_EROFS_PCLUSTER_FOLLOWED_NOINPLACE; } else { /* bind cache first when cached decompression is preferred */ - z_erofs_bind_cache(fe, pagepool); + z_erofs_bind_cache(fe); } hitted: /* @@ -1660,7 +1662,6 @@ static void z_erofs_decompressqueue_endio(struct bio *bio) } static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f, - struct page **pagepool, struct z_erofs_decompressqueue *fgq, bool *force_fg, bool readahead) { @@ -1723,8 +1724,8 @@ static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f, do { struct page *page; - page = pickup_page_for_submission(pcl, i++, pagepool, - mc); + page = pickup_page_for_submission(pcl, i++, + &f->pagepool, mc); if (!page) continue; @@ -1789,16 +1790,16 @@ submit_bio_retry: } static void z_erofs_runqueue(struct z_erofs_decompress_frontend *f, - struct page **pagepool, bool force_fg, bool ra) + bool force_fg, bool ra) { struct z_erofs_decompressqueue io[NR_JOBQUEUES]; if (f->owned_head == Z_EROFS_PCLUSTER_TAIL) return; - z_erofs_submit_queue(f, pagepool, io, &force_fg, ra); + z_erofs_submit_queue(f, io, &force_fg, ra); /* handle bypass queue (no i/o pclusters) immediately */ - z_erofs_decompress_queue(&io[JQ_BYPASS], pagepool); + z_erofs_decompress_queue(&io[JQ_BYPASS], &f->pagepool); if (!force_fg) return; @@ -1807,7 +1808,7 @@ static void z_erofs_runqueue(struct z_erofs_decompress_frontend *f, wait_for_completion_io(&io[JQ_SUBMIT].u.done); /* handle synchronous decompress queue in the caller context */ - z_erofs_decompress_queue(&io[JQ_SUBMIT], pagepool); + z_erofs_decompress_queue(&io[JQ_SUBMIT], &f->pagepool); } /* @@ -1815,8 +1816,7 @@ static void z_erofs_runqueue(struct z_erofs_decompress_frontend *f, * approximate readmore strategies as a start. */ static void z_erofs_pcluster_readmore(struct z_erofs_decompress_frontend *f, - struct readahead_control *rac, - struct page **pagepool, bool backmost) + struct readahead_control *rac, bool backmost) { struct inode *inode = f->inode; struct erofs_map_blocks *map = &f->map; @@ -1858,7 +1858,7 @@ static void z_erofs_pcluster_readmore(struct z_erofs_decompress_frontend *f, if (PageUptodate(page)) { unlock_page(page); } else { - err = z_erofs_do_read_page(f, page, pagepool); + err = z_erofs_do_read_page(f, page); if (err) erofs_err(inode->i_sb, "readmore error at page %lu @ nid %llu", @@ -1879,27 +1879,24 @@ static int z_erofs_read_folio(struct file *file, struct folio *folio) struct inode *const inode = page->mapping->host; struct erofs_sb_info *const sbi = EROFS_I_SB(inode); struct z_erofs_decompress_frontend f = DECOMPRESS_FRONTEND_INIT(inode); - struct page *pagepool = NULL; int err; trace_erofs_readpage(page, false); f.headoffset = (erofs_off_t)page->index << PAGE_SHIFT; - z_erofs_pcluster_readmore(&f, NULL, &pagepool, true); - err = z_erofs_do_read_page(&f, page, &pagepool); - z_erofs_pcluster_readmore(&f, NULL, &pagepool, false); - + z_erofs_pcluster_readmore(&f, NULL, true); + err = z_erofs_do_read_page(&f, page); + z_erofs_pcluster_readmore(&f, NULL, false); (void)z_erofs_collector_end(&f); /* if some compressed cluster ready, need submit them anyway */ - z_erofs_runqueue(&f, &pagepool, z_erofs_is_sync_decompress(sbi, 0), - false); + z_erofs_runqueue(&f, z_erofs_is_sync_decompress(sbi, 0), false); if (err) erofs_err(inode->i_sb, "failed to read, err [%d]", err); erofs_put_metabuf(&f.map.buf); - erofs_release_pages(&pagepool); + erofs_release_pages(&f.pagepool); return err; } @@ -1908,12 +1905,12 @@ static void z_erofs_readahead(struct readahead_control *rac) struct inode *const inode = rac->mapping->host; struct erofs_sb_info *const sbi = EROFS_I_SB(inode); struct z_erofs_decompress_frontend f = DECOMPRESS_FRONTEND_INIT(inode); - struct page *pagepool = NULL, *head = NULL, *page; + struct page *head = NULL, *page; unsigned int nr_pages; f.headoffset = readahead_pos(rac); - z_erofs_pcluster_readmore(&f, rac, &pagepool, true); + z_erofs_pcluster_readmore(&f, rac, true); nr_pages = readahead_count(rac); trace_erofs_readpages(inode, readahead_index(rac), nr_pages, false); @@ -1929,20 +1926,19 @@ static void z_erofs_readahead(struct readahead_control *rac) /* traversal in reverse order */ head = (void *)page_private(page); - err = z_erofs_do_read_page(&f, page, &pagepool); + err = z_erofs_do_read_page(&f, page); if (err) erofs_err(inode->i_sb, "readahead error at page %lu @ nid %llu", page->index, EROFS_I(inode)->nid); put_page(page); } - z_erofs_pcluster_readmore(&f, rac, &pagepool, false); + z_erofs_pcluster_readmore(&f, rac, false); (void)z_erofs_collector_end(&f); - z_erofs_runqueue(&f, &pagepool, - z_erofs_is_sync_decompress(sbi, nr_pages), true); + z_erofs_runqueue(&f, z_erofs_is_sync_decompress(sbi, nr_pages), true); erofs_put_metabuf(&f.map.buf); - erofs_release_pages(&pagepool); + erofs_release_pages(&f.pagepool); } const struct address_space_operations z_erofs_aops = { -- cgit v1.2.3 From 967c28b23f6c89bb8eef6a046ea88afe0d7c1029 Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Sat, 27 May 2023 04:14:56 +0800 Subject: erofs: kill hooked chains to avoid loops on deduplicated compressed images After heavily stressing EROFS with several images which include a hand-crafted image of repeated patterns for more than 46 days, I found two chains could be linked with each other almost simultaneously and form a loop so that the entire loop won't be submitted. As a consequence, the corresponding file pages will remain locked forever. It can be _only_ observed on data-deduplicated compressed images. For example, consider two chains with five pclusters in total: Chain 1: 2->3->4->5 -- The tail pcluster is 5; Chain 2: 5->1->2 -- The tail pcluster is 2. Chain 2 could link to Chain 1 with pcluster 5; and Chain 1 could link to Chain 2 at the same time with pcluster 2. Since hooked chains are all linked locklessly now, I have no idea how to simply avoid the race. Instead, let's avoid hooked chains completely until I could work out a proper way to fix this and end users finally tell us that it's needed to add it back. Actually, this optimization can be found with multi-threaded workloads (especially even more often on deduplicated compressed images), yet I'm not sure about the overall system impacts of not having this compared with implementation complexity. Fixes: 267f2492c8f7 ("erofs: introduce multi-reference pclusters (fully-referenced)") Signed-off-by: Gao Xiang Reviewed-by: Yue Hu Link: https://lore.kernel.org/r/20230526201459.128169-4-hsiangkao@linux.alibaba.com --- fs/erofs/zdata.c | 72 +++++++++----------------------------------------------- 1 file changed, 11 insertions(+), 61 deletions(-) (limited to 'fs') diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index 22be768a5b8a..d29552ea53fc 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -93,11 +93,8 @@ struct z_erofs_pcluster { /* let's avoid the valid 32-bit kernel addresses */ -/* the chained workgroup has't submitted io (still open) */ +/* the end of a chain of pclusters */ #define Z_EROFS_PCLUSTER_TAIL ((void *)0x5F0ECAFE) -/* the chained workgroup has already submitted io */ -#define Z_EROFS_PCLUSTER_TAIL_CLOSED ((void *)0x5F0EDEAD) - #define Z_EROFS_PCLUSTER_NIL (NULL) struct z_erofs_decompressqueue { @@ -504,20 +501,6 @@ out_error_pcluster_pool: enum z_erofs_pclustermode { Z_EROFS_PCLUSTER_INFLIGHT, - /* - * The current pclusters was the tail of an exist chain, in addition - * that the previous processed chained pclusters are all decided to - * be hooked up to it. - * A new chain will be created for the remaining pclusters which are - * not processed yet, so different from Z_EROFS_PCLUSTER_FOLLOWED, - * the next pcluster cannot reuse the whole page safely for inplace I/O - * in the following scenario: - * ________________________________________________________________ - * | tail (partial) page | head (partial) page | - * | (belongs to the next pcl) | (belongs to the current pcl) | - * |_______PCLUSTER_FOLLOWED______|________PCLUSTER_HOOKED__________| - */ - Z_EROFS_PCLUSTER_HOOKED, /* * a weak form of Z_EROFS_PCLUSTER_FOLLOWED, the difference is that it * could be dispatched into bypass queue later due to uptodated managed @@ -535,8 +518,8 @@ enum z_erofs_pclustermode { * ________________________________________________________________ * | tail (partial) page | head (partial) page | * | (of the current cl) | (of the previous collection) | - * | PCLUSTER_FOLLOWED or | | - * |_____PCLUSTER_HOOKED__|___________PCLUSTER_FOLLOWED____________| + * | | | + * |__PCLUSTER_FOLLOWED___|___________PCLUSTER_FOLLOWED____________| * * [ (*) the above page can be used as inplace I/O. ] */ @@ -550,7 +533,7 @@ struct z_erofs_decompress_frontend { struct page *pagepool; struct page *candidate_bvpage; - struct z_erofs_pcluster *pcl, *tailpcl; + struct z_erofs_pcluster *pcl; z_erofs_next_pcluster_t owned_head; enum z_erofs_pclustermode mode; @@ -755,19 +738,7 @@ static void z_erofs_try_to_claim_pcluster(struct z_erofs_decompress_frontend *f) return; } - /* - * type 2, link to the end of an existing open chain, be careful - * that its submission is controlled by the original attached chain. - */ - if (*owned_head != &pcl->next && pcl != f->tailpcl && - cmpxchg(&pcl->next, Z_EROFS_PCLUSTER_TAIL, - *owned_head) == Z_EROFS_PCLUSTER_TAIL) { - *owned_head = Z_EROFS_PCLUSTER_TAIL; - f->mode = Z_EROFS_PCLUSTER_HOOKED; - f->tailpcl = NULL; - return; - } - /* type 3, it belongs to a chain, but it isn't the end of the chain */ + /* type 2, it belongs to an ongoing chain */ f->mode = Z_EROFS_PCLUSTER_INFLIGHT; } @@ -828,9 +799,6 @@ static int z_erofs_register_pcluster(struct z_erofs_decompress_frontend *fe) goto err_out; } } - /* used to check tail merging loop due to corrupted images */ - if (fe->owned_head == Z_EROFS_PCLUSTER_TAIL) - fe->tailpcl = pcl; fe->owned_head = &pcl->next; fe->pcl = pcl; return 0; @@ -851,7 +819,6 @@ static int z_erofs_collector_begin(struct z_erofs_decompress_frontend *fe) /* must be Z_EROFS_PCLUSTER_TAIL or pointed to previous pcluster */ DBG_BUGON(fe->owned_head == Z_EROFS_PCLUSTER_NIL); - DBG_BUGON(fe->owned_head == Z_EROFS_PCLUSTER_TAIL_CLOSED); if (!(map->m_flags & EROFS_MAP_META)) { grp = erofs_find_workgroup(fe->inode->i_sb, @@ -870,10 +837,6 @@ static int z_erofs_collector_begin(struct z_erofs_decompress_frontend *fe) if (ret == -EEXIST) { mutex_lock(&fe->pcl->lock); - /* used to check tail merging loop due to corrupted images */ - if (fe->owned_head == Z_EROFS_PCLUSTER_TAIL) - fe->tailpcl = fe->pcl; - z_erofs_try_to_claim_pcluster(fe); } else if (ret) { return ret; @@ -1028,8 +991,7 @@ hitted: * those chains are handled asynchronously thus the page cannot be used * for inplace I/O or bvpage (should be processed in a strict order.) */ - tight &= (fe->mode >= Z_EROFS_PCLUSTER_HOOKED && - fe->mode != Z_EROFS_PCLUSTER_FOLLOWED_NOINPLACE); + tight &= (fe->mode > Z_EROFS_PCLUSTER_FOLLOWED_NOINPLACE); cur = end - min_t(unsigned int, offset + end - map->m_la, end); if (!(map->m_flags & EROFS_MAP_MAPPED)) { @@ -1398,10 +1360,7 @@ static void z_erofs_decompress_queue(const struct z_erofs_decompressqueue *io, }; z_erofs_next_pcluster_t owned = io->head; - while (owned != Z_EROFS_PCLUSTER_TAIL_CLOSED) { - /* impossible that 'owned' equals Z_EROFS_WORK_TPTR_TAIL */ - DBG_BUGON(owned == Z_EROFS_PCLUSTER_TAIL); - /* impossible that 'owned' equals Z_EROFS_PCLUSTER_NIL */ + while (owned != Z_EROFS_PCLUSTER_TAIL) { DBG_BUGON(owned == Z_EROFS_PCLUSTER_NIL); be.pcl = container_of(owned, struct z_erofs_pcluster, next); @@ -1418,7 +1377,7 @@ static void z_erofs_decompressqueue_work(struct work_struct *work) container_of(work, struct z_erofs_decompressqueue, u.work); struct page *pagepool = NULL; - DBG_BUGON(bgq->head == Z_EROFS_PCLUSTER_TAIL_CLOSED); + DBG_BUGON(bgq->head == Z_EROFS_PCLUSTER_TAIL); z_erofs_decompress_queue(bgq, &pagepool); erofs_release_pages(&pagepool); kvfree(bgq); @@ -1606,7 +1565,7 @@ fg_out: q->sync = true; } q->sb = sb; - q->head = Z_EROFS_PCLUSTER_TAIL_CLOSED; + q->head = Z_EROFS_PCLUSTER_TAIL; return q; } @@ -1624,11 +1583,7 @@ static void move_to_bypass_jobqueue(struct z_erofs_pcluster *pcl, z_erofs_next_pcluster_t *const submit_qtail = qtail[JQ_SUBMIT]; z_erofs_next_pcluster_t *const bypass_qtail = qtail[JQ_BYPASS]; - DBG_BUGON(owned_head == Z_EROFS_PCLUSTER_TAIL_CLOSED); - if (owned_head == Z_EROFS_PCLUSTER_TAIL) - owned_head = Z_EROFS_PCLUSTER_TAIL_CLOSED; - - WRITE_ONCE(pcl->next, Z_EROFS_PCLUSTER_TAIL_CLOSED); + WRITE_ONCE(pcl->next, Z_EROFS_PCLUSTER_TAIL); WRITE_ONCE(*submit_qtail, owned_head); WRITE_ONCE(*bypass_qtail, &pcl->next); @@ -1698,15 +1653,10 @@ static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f, unsigned int i = 0; bool bypass = true; - /* no possible 'owned_head' equals the following */ - DBG_BUGON(owned_head == Z_EROFS_PCLUSTER_TAIL_CLOSED); DBG_BUGON(owned_head == Z_EROFS_PCLUSTER_NIL); - pcl = container_of(owned_head, struct z_erofs_pcluster, next); + owned_head = READ_ONCE(pcl->next); - /* close the main owned chain at first */ - owned_head = cmpxchg(&pcl->next, Z_EROFS_PCLUSTER_TAIL, - Z_EROFS_PCLUSTER_TAIL_CLOSED); if (z_erofs_is_inline_pcluster(pcl)) { move_to_bypass_jobqueue(pcl, qtail, owned_head); continue; -- cgit v1.2.3 From 7b4e372c36fcd33c74ba3cbd65fa534b9c558184 Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Sat, 27 May 2023 04:14:57 +0800 Subject: erofs: adapt managed inode operations into folios This patch gets rid of erofs_try_to_free_cached_page() and fold it into .release_folio(). It also moves managed inode operations into zdata.c, which simplifies the code a bit. No logic changes. Signed-off-by: Gao Xiang Reviewed-by: Yue Hu Link: https://lore.kernel.org/r/20230526201459.128169-5-hsiangkao@linux.alibaba.com --- fs/erofs/internal.h | 3 ++- fs/erofs/super.c | 62 ----------------------------------------------------- fs/erofs/zdata.c | 59 +++++++++++++++++++++++++++++++++++++++++++------- 3 files changed, 53 insertions(+), 71 deletions(-) (limited to 'fs') diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h index 1e39c03357d1..005f3391bcd3 100644 --- a/fs/erofs/internal.h +++ b/fs/erofs/internal.h @@ -500,7 +500,6 @@ int __init z_erofs_init_zip_subsystem(void); void z_erofs_exit_zip_subsystem(void); int erofs_try_to_free_all_cached_pages(struct erofs_sb_info *sbi, struct erofs_workgroup *egrp); -int erofs_try_to_free_cached_page(struct page *page); int z_erofs_load_lz4_config(struct super_block *sb, struct erofs_super_block *dsb, struct z_erofs_lz4_cfgs *lz4, int len); @@ -511,6 +510,7 @@ void erofs_put_pcpubuf(void *ptr); int erofs_pcpubuf_growsize(unsigned int nrpages); void __init erofs_pcpubuf_init(void); void erofs_pcpubuf_exit(void); +int erofs_init_managed_cache(struct super_block *sb); #else static inline void erofs_shrinker_register(struct super_block *sb) {} static inline void erofs_shrinker_unregister(struct super_block *sb) {} @@ -530,6 +530,7 @@ static inline int z_erofs_load_lz4_config(struct super_block *sb, } static inline void erofs_pcpubuf_init(void) {} static inline void erofs_pcpubuf_exit(void) {} +static inline int erofs_init_managed_cache(struct super_block *sb) { return 0; } #endif /* !CONFIG_EROFS_FS_ZIP */ #ifdef CONFIG_EROFS_FS_ZIP_LZMA diff --git a/fs/erofs/super.c b/fs/erofs/super.c index 811ab66d805e..c2829c91812b 100644 --- a/fs/erofs/super.c +++ b/fs/erofs/super.c @@ -599,68 +599,6 @@ static int erofs_fc_parse_param(struct fs_context *fc, return 0; } -#ifdef CONFIG_EROFS_FS_ZIP -static const struct address_space_operations managed_cache_aops; - -static bool erofs_managed_cache_release_folio(struct folio *folio, gfp_t gfp) -{ - bool ret = true; - struct address_space *const mapping = folio->mapping; - - DBG_BUGON(!folio_test_locked(folio)); - DBG_BUGON(mapping->a_ops != &managed_cache_aops); - - if (folio_test_private(folio)) - ret = erofs_try_to_free_cached_page(&folio->page); - - return ret; -} - -/* - * It will be called only on inode eviction. In case that there are still some - * decompression requests in progress, wait with rescheduling for a bit here. - * We could introduce an extra locking instead but it seems unnecessary. - */ -static void erofs_managed_cache_invalidate_folio(struct folio *folio, - size_t offset, size_t length) -{ - const size_t stop = length + offset; - - DBG_BUGON(!folio_test_locked(folio)); - - /* Check for potential overflow in debug mode */ - DBG_BUGON(stop > folio_size(folio) || stop < length); - - if (offset == 0 && stop == folio_size(folio)) - while (!erofs_managed_cache_release_folio(folio, GFP_NOFS)) - cond_resched(); -} - -static const struct address_space_operations managed_cache_aops = { - .release_folio = erofs_managed_cache_release_folio, - .invalidate_folio = erofs_managed_cache_invalidate_folio, -}; - -static int erofs_init_managed_cache(struct super_block *sb) -{ - struct erofs_sb_info *const sbi = EROFS_SB(sb); - struct inode *const inode = new_inode(sb); - - if (!inode) - return -ENOMEM; - - set_nlink(inode, 1); - inode->i_size = OFFSET_MAX; - - inode->i_mapping->a_ops = &managed_cache_aops; - mapping_set_gfp_mask(inode->i_mapping, GFP_NOFS); - sbi->managed_cache = inode; - return 0; -} -#else -static int erofs_init_managed_cache(struct super_block *sb) { return 0; } -#endif - static struct inode *erofs_nfs_get_inode(struct super_block *sb, u64 ino, u32 generation) { diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index d29552ea53fc..c556906354e5 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -665,29 +665,72 @@ int erofs_try_to_free_all_cached_pages(struct erofs_sb_info *sbi, return 0; } -int erofs_try_to_free_cached_page(struct page *page) +static bool z_erofs_cache_release_folio(struct folio *folio, gfp_t gfp) { - struct z_erofs_pcluster *const pcl = (void *)page_private(page); - int ret, i; + struct z_erofs_pcluster *pcl = folio_get_private(folio); + bool ret; + int i; + + if (!folio_test_private(folio)) + return true; if (!erofs_workgroup_try_to_freeze(&pcl->obj, 1)) - return 0; + return false; - ret = 0; + ret = false; DBG_BUGON(z_erofs_is_inline_pcluster(pcl)); for (i = 0; i < pcl->pclusterpages; ++i) { - if (pcl->compressed_bvecs[i].page == page) { + if (pcl->compressed_bvecs[i].page == &folio->page) { WRITE_ONCE(pcl->compressed_bvecs[i].page, NULL); - ret = 1; + ret = true; break; } } erofs_workgroup_unfreeze(&pcl->obj, 1); + if (ret) - detach_page_private(page); + folio_detach_private(folio); return ret; } +/* + * It will be called only on inode eviction. In case that there are still some + * decompression requests in progress, wait with rescheduling for a bit here. + * An extra lock could be introduced instead but it seems unnecessary. + */ +static void z_erofs_cache_invalidate_folio(struct folio *folio, + size_t offset, size_t length) +{ + const size_t stop = length + offset; + + /* Check for potential overflow in debug mode */ + DBG_BUGON(stop > folio_size(folio) || stop < length); + + if (offset == 0 && stop == folio_size(folio)) + while (!z_erofs_cache_release_folio(folio, GFP_NOFS)) + cond_resched(); +} + +static const struct address_space_operations z_erofs_cache_aops = { + .release_folio = z_erofs_cache_release_folio, + .invalidate_folio = z_erofs_cache_invalidate_folio, +}; + +int erofs_init_managed_cache(struct super_block *sb) +{ + struct inode *const inode = new_inode(sb); + + if (!inode) + return -ENOMEM; + + set_nlink(inode, 1); + inode->i_size = OFFSET_MAX; + inode->i_mapping->a_ops = &z_erofs_cache_aops; + mapping_set_gfp_mask(inode->i_mapping, GFP_NOFS); + EROFS_SB(sb)->managed_cache = inode; + return 0; +} + static bool z_erofs_try_inplace_io(struct z_erofs_decompress_frontend *fe, struct z_erofs_bvec *bvec) { -- cgit v1.2.3 From 7674a42f35ea302b97ff3659f2e6f28be23ac9b9 Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Mon, 29 May 2023 20:37:27 +0800 Subject: erofs: use struct lockref to replace handcrafted approach Let's avoid the current handcrafted lockref although `struct lockref` inclusion usually increases extra 4 bytes with an explicit spinlock if CONFIG_DEBUG_SPINLOCK is off. Apart from the size difference, note that the meaning of refcount is also changed to active users. IOWs, it doesn't take an extra refcount for XArray tree insertion. I don't observe any significant performance difference at least on our cloud compute server but the new one indeed simplifies the overall codebase a bit. Signed-off-by: Gao Xiang Reviewed-by: Yue Hu Link: https://lore.kernel.org/r/20230529123727.79943-1-hsiangkao@linux.alibaba.com --- fs/erofs/internal.h | 38 ++--------------------- fs/erofs/utils.c | 86 +++++++++++++++++++++++++---------------------------- fs/erofs/zdata.c | 15 +++++----- 3 files changed, 51 insertions(+), 88 deletions(-) (limited to 'fs') diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h index 005f3391bcd3..36e32fa542f0 100644 --- a/fs/erofs/internal.h +++ b/fs/erofs/internal.h @@ -208,46 +208,12 @@ enum { EROFS_ZIP_CACHE_READAROUND }; -#define EROFS_LOCKED_MAGIC (INT_MIN | 0xE0F510CCL) - /* basic unit of the workstation of a super_block */ struct erofs_workgroup { - /* the workgroup index in the workstation */ pgoff_t index; - - /* overall workgroup reference count */ - atomic_t refcount; + struct lockref lockref; }; -static inline bool erofs_workgroup_try_to_freeze(struct erofs_workgroup *grp, - int val) -{ - preempt_disable(); - if (val != atomic_cmpxchg(&grp->refcount, val, EROFS_LOCKED_MAGIC)) { - preempt_enable(); - return false; - } - return true; -} - -static inline void erofs_workgroup_unfreeze(struct erofs_workgroup *grp, - int orig_val) -{ - /* - * other observers should notice all modifications - * in the freezing period. - */ - smp_mb(); - atomic_set(&grp->refcount, orig_val); - preempt_enable(); -} - -static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp) -{ - return atomic_cond_read_relaxed(&grp->refcount, - VAL != EROFS_LOCKED_MAGIC); -} - enum erofs_kmap_type { EROFS_NO_KMAP, /* don't map the buffer */ EROFS_KMAP, /* use kmap_local_page() to map the buffer */ @@ -486,7 +452,7 @@ static inline void erofs_pagepool_add(struct page **pagepool, struct page *page) void erofs_release_pages(struct page **pagepool); #ifdef CONFIG_EROFS_FS_ZIP -int erofs_workgroup_put(struct erofs_workgroup *grp); +void erofs_workgroup_put(struct erofs_workgroup *grp); struct erofs_workgroup *erofs_find_workgroup(struct super_block *sb, pgoff_t index); struct erofs_workgroup *erofs_insert_workgroup(struct super_block *sb, diff --git a/fs/erofs/utils.c b/fs/erofs/utils.c index 46627cb69abe..cc6fb9e98899 100644 --- a/fs/erofs/utils.c +++ b/fs/erofs/utils.c @@ -4,7 +4,6 @@ * https://www.huawei.com/ */ #include "internal.h" -#include struct page *erofs_allocpage(struct page **pagepool, gfp_t gfp) { @@ -33,22 +32,21 @@ void erofs_release_pages(struct page **pagepool) /* global shrink count (for all mounted EROFS instances) */ static atomic_long_t erofs_global_shrink_cnt; -static int erofs_workgroup_get(struct erofs_workgroup *grp) +static bool erofs_workgroup_get(struct erofs_workgroup *grp) { - int o; + if (lockref_get_not_zero(&grp->lockref)) + return true; -repeat: - o = erofs_wait_on_workgroup_freezed(grp); - if (o <= 0) - return -1; - - if (atomic_cmpxchg(&grp->refcount, o, o + 1) != o) - goto repeat; + spin_lock(&grp->lockref.lock); + if (__lockref_is_dead(&grp->lockref)) { + spin_unlock(&grp->lockref.lock); + return false; + } - /* decrease refcount paired by erofs_workgroup_put */ - if (o == 1) + if (!grp->lockref.count++) atomic_long_dec(&erofs_global_shrink_cnt); - return 0; + spin_unlock(&grp->lockref.lock); + return true; } struct erofs_workgroup *erofs_find_workgroup(struct super_block *sb, @@ -61,7 +59,7 @@ repeat: rcu_read_lock(); grp = xa_load(&sbi->managed_pslots, index); if (grp) { - if (erofs_workgroup_get(grp)) { + if (!erofs_workgroup_get(grp)) { /* prefer to relax rcu read side */ rcu_read_unlock(); goto repeat; @@ -80,11 +78,10 @@ struct erofs_workgroup *erofs_insert_workgroup(struct super_block *sb, struct erofs_workgroup *pre; /* - * Bump up a reference count before making this visible - * to others for the XArray in order to avoid potential - * UAF without serialized by xa_lock. + * Bump up before making this visible to others for the XArray in order + * to avoid potential UAF without serialized by xa_lock. */ - atomic_inc(&grp->refcount); + lockref_get(&grp->lockref); repeat: xa_lock(&sbi->managed_pslots); @@ -93,13 +90,13 @@ repeat: if (pre) { if (xa_is_err(pre)) { pre = ERR_PTR(xa_err(pre)); - } else if (erofs_workgroup_get(pre)) { + } else if (!erofs_workgroup_get(pre)) { /* try to legitimize the current in-tree one */ xa_unlock(&sbi->managed_pslots); cond_resched(); goto repeat; } - atomic_dec(&grp->refcount); + lockref_put_return(&grp->lockref); grp = pre; } xa_unlock(&sbi->managed_pslots); @@ -112,38 +109,34 @@ static void __erofs_workgroup_free(struct erofs_workgroup *grp) erofs_workgroup_free_rcu(grp); } -int erofs_workgroup_put(struct erofs_workgroup *grp) +void erofs_workgroup_put(struct erofs_workgroup *grp) { - int count = atomic_dec_return(&grp->refcount); + if (lockref_put_or_lock(&grp->lockref)) + return; - if (count == 1) + DBG_BUGON(__lockref_is_dead(&grp->lockref)); + if (grp->lockref.count == 1) atomic_long_inc(&erofs_global_shrink_cnt); - else if (!count) - __erofs_workgroup_free(grp); - return count; + --grp->lockref.count; + spin_unlock(&grp->lockref.lock); } static bool erofs_try_to_release_workgroup(struct erofs_sb_info *sbi, struct erofs_workgroup *grp) { - /* - * If managed cache is on, refcount of workgroups - * themselves could be < 0 (freezed). In other words, - * there is no guarantee that all refcounts > 0. - */ - if (!erofs_workgroup_try_to_freeze(grp, 1)) - return false; + int free = false; + + spin_lock(&grp->lockref.lock); + if (grp->lockref.count) + goto out; /* - * Note that all cached pages should be unattached - * before deleted from the XArray. Otherwise some - * cached pages could be still attached to the orphan - * old workgroup when the new one is available in the tree. + * Note that all cached pages should be detached before deleted from + * the XArray. Otherwise some cached pages could be still attached to + * the orphan old workgroup when the new one is available in the tree. */ - if (erofs_try_to_free_all_cached_pages(sbi, grp)) { - erofs_workgroup_unfreeze(grp, 1); - return false; - } + if (erofs_try_to_free_all_cached_pages(sbi, grp)) + goto out; /* * It's impossible to fail after the workgroup is freezed, @@ -152,10 +145,13 @@ static bool erofs_try_to_release_workgroup(struct erofs_sb_info *sbi, */ DBG_BUGON(__xa_erase(&sbi->managed_pslots, grp->index) != grp); - /* last refcount should be connected with its managed pslot. */ - erofs_workgroup_unfreeze(grp, 0); - __erofs_workgroup_free(grp); - return true; + lockref_mark_dead(&grp->lockref); + free = true; +out: + spin_unlock(&grp->lockref.lock); + if (free) + __erofs_workgroup_free(grp); + return free; } static unsigned long erofs_shrink_workstation(struct erofs_sb_info *sbi, diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index c556906354e5..637a964ff110 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -641,7 +641,7 @@ int erofs_try_to_free_all_cached_pages(struct erofs_sb_info *sbi, DBG_BUGON(z_erofs_is_inline_pcluster(pcl)); /* - * refcount of workgroup is now freezed as 1, + * refcount of workgroup is now freezed as 0, * therefore no need to worry about available decompression users. */ for (i = 0; i < pcl->pclusterpages; ++i) { @@ -674,10 +674,11 @@ static bool z_erofs_cache_release_folio(struct folio *folio, gfp_t gfp) if (!folio_test_private(folio)) return true; - if (!erofs_workgroup_try_to_freeze(&pcl->obj, 1)) - return false; - ret = false; + spin_lock(&pcl->obj.lockref.lock); + if (pcl->obj.lockref.count > 0) + goto out; + DBG_BUGON(z_erofs_is_inline_pcluster(pcl)); for (i = 0; i < pcl->pclusterpages; ++i) { if (pcl->compressed_bvecs[i].page == &folio->page) { @@ -686,10 +687,10 @@ static bool z_erofs_cache_release_folio(struct folio *folio, gfp_t gfp) break; } } - erofs_workgroup_unfreeze(&pcl->obj, 1); - if (ret) folio_detach_private(folio); +out: + spin_unlock(&pcl->obj.lockref.lock); return ret; } @@ -805,7 +806,7 @@ static int z_erofs_register_pcluster(struct z_erofs_decompress_frontend *fe) if (IS_ERR(pcl)) return PTR_ERR(pcl); - atomic_set(&pcl->obj.refcount, 1); + spin_lock_init(&pcl->obj.lockref.lock); pcl->algorithmformat = map->m_algorithmformat; pcl->length = 0; pcl->partial = true; -- cgit v1.2.3 From 43d86ec93630396b622acf3f9cb88f734d4098e8 Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Sat, 27 May 2023 04:14:59 +0800 Subject: erofs: use poison pointer to replace the hard-coded address It's safer and cleaner to replace such hard-coded illegal pointer with poison pointers. Signed-off-by: Gao Xiang Reviewed-by: Yue Hu Link: https://lore.kernel.org/r/20230526201459.128169-7-hsiangkao@linux.alibaba.com --- fs/erofs/zdata.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index 637a964ff110..264bf553c287 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -91,10 +91,8 @@ struct z_erofs_pcluster { struct z_erofs_bvec compressed_bvecs[]; }; -/* let's avoid the valid 32-bit kernel addresses */ - /* the end of a chain of pclusters */ -#define Z_EROFS_PCLUSTER_TAIL ((void *)0x5F0ECAFE) +#define Z_EROFS_PCLUSTER_TAIL ((void *) 0x700 + POISON_POINTER_DELTA) #define Z_EROFS_PCLUSTER_NIL (NULL) struct z_erofs_decompressqueue { -- cgit v1.2.3 From 9c39ec0cff4e9373ab238120ca45a50c703dbb4e Mon Sep 17 00:00:00 2001 From: Jingbo Xu Date: Thu, 1 Jun 2023 10:43:42 +0800 Subject: erofs: convert erofs_read_metabuf() to erofs_bread() for xattr buf->inode is constant once initialized during erofs_buf's lifetime. Thus call erofs_init_metabuf() and erofs_bread() separately to avoid the repetition of assigning buf->inode when iterating xattrs. Signed-off-by: Jingbo Xu Reviewed-by: Gao Xiang Link: https://lore.kernel.org/r/20230601024347.108469-2-jefflexu@linux.alibaba.com Signed-off-by: Gao Xiang --- fs/erofs/xattr.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) (limited to 'fs') diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c index bbfe7ce170d2..d9e041d27a35 100644 --- a/fs/erofs/xattr.c +++ b/fs/erofs/xattr.c @@ -81,11 +81,12 @@ static int erofs_init_inode_xattrs(struct inode *inode) } it.buf = __EROFS_BUF_INITIALIZER; + erofs_init_metabuf(&it.buf, sb); it.blkaddr = erofs_blknr(sb, erofs_iloc(inode) + vi->inode_isize); it.ofs = erofs_blkoff(sb, erofs_iloc(inode) + vi->inode_isize); /* read in shared xattr array (non-atomic, see kmalloc below) */ - it.kaddr = erofs_read_metabuf(&it.buf, sb, it.blkaddr, EROFS_KMAP); + it.kaddr = erofs_bread(&it.buf, it.blkaddr, EROFS_KMAP); if (IS_ERR(it.kaddr)) { ret = PTR_ERR(it.kaddr); goto out_unlock; @@ -109,8 +110,7 @@ static int erofs_init_inode_xattrs(struct inode *inode) /* cannot be unaligned */ DBG_BUGON(it.ofs != sb->s_blocksize); - it.kaddr = erofs_read_metabuf(&it.buf, sb, ++it.blkaddr, - EROFS_KMAP); + it.kaddr = erofs_bread(&it.buf, ++it.blkaddr, EROFS_KMAP); if (IS_ERR(it.kaddr)) { kfree(vi->xattr_shared_xattrs); vi->xattr_shared_xattrs = NULL; @@ -156,8 +156,7 @@ static inline int xattr_iter_fixup(struct xattr_iter *it) return 0; it->blkaddr += erofs_blknr(it->sb, it->ofs); - it->kaddr = erofs_read_metabuf(&it->buf, it->sb, it->blkaddr, - EROFS_KMAP); + it->kaddr = erofs_bread(&it->buf, it->blkaddr, EROFS_KMAP); if (IS_ERR(it->kaddr)) return PTR_ERR(it->kaddr); it->ofs = erofs_blkoff(it->sb, it->ofs); @@ -181,8 +180,7 @@ static int inline_xattr_iter_begin(struct xattr_iter *it, it->blkaddr = erofs_blknr(it->sb, erofs_iloc(inode) + inline_xattr_ofs); it->ofs = erofs_blkoff(it->sb, erofs_iloc(inode) + inline_xattr_ofs); - it->kaddr = erofs_read_metabuf(&it->buf, inode->i_sb, it->blkaddr, - EROFS_KMAP); + it->kaddr = erofs_bread(&it->buf, it->blkaddr, EROFS_KMAP); if (IS_ERR(it->kaddr)) return PTR_ERR(it->kaddr); return vi->xattr_isize - xattr_header_sz; @@ -403,8 +401,7 @@ static int shared_getxattr(struct inode *inode, struct getxattr_iter *it) xsid = vi->xattr_shared_xattrs[i]; it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid); it->it.ofs = erofs_xattr_blkoff(sb, xsid); - it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, - it->it.blkaddr, EROFS_KMAP); + it->it.kaddr = erofs_bread(&it->it.buf, it->it.blkaddr, EROFS_KMAP); if (IS_ERR(it->it.kaddr)) return PTR_ERR(it->it.kaddr); @@ -444,13 +441,14 @@ int erofs_getxattr(struct inode *inode, int index, if (it.name.len > EROFS_NAME_LEN) return -ERANGE; + it.it.sb = inode->i_sb; it.it.buf = __EROFS_BUF_INITIALIZER; + erofs_init_metabuf(&it.it.buf, it.it.sb); it.name.name = name; it.buffer = buffer; it.buffer_size = buffer_size; - it.it.sb = inode->i_sb; ret = inline_getxattr(inode, &it); if (ret == -ENOATTR) ret = shared_getxattr(inode, &it); @@ -608,8 +606,7 @@ static int shared_listxattr(struct listxattr_iter *it) xsid = vi->xattr_shared_xattrs[i]; it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid); it->it.ofs = erofs_xattr_blkoff(sb, xsid); - it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, - it->it.blkaddr, EROFS_KMAP); + it->it.kaddr = erofs_bread(&it->it.buf, it->it.blkaddr, EROFS_KMAP); if (IS_ERR(it->it.kaddr)) return PTR_ERR(it->it.kaddr); @@ -632,14 +629,14 @@ ssize_t erofs_listxattr(struct dentry *dentry, if (ret) return ret; + it.it.sb = dentry->d_sb; it.it.buf = __EROFS_BUF_INITIALIZER; + erofs_init_metabuf(&it.it.buf, it.it.sb); it.dentry = dentry; it.buffer = buffer; it.buffer_size = buffer_size; it.buffer_ofs = 0; - it.it.sb = dentry->d_sb; - ret = inline_listxattr(&it); if (ret >= 0 || ret == -ENOATTR) ret = shared_listxattr(&it); -- cgit v1.2.3 From 001b8ccd0650727e54ec16ef72bf1b8eeab7168e Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Thu, 1 Jun 2023 19:23:41 +0800 Subject: erofs: fix compact 4B support for 16k block size In compact 4B, two adjacent lclusters are packed together as a unit to form on-disk indexes for effective random access, as below: (amortized = 4, vcnt = 2) _____________________________________________ |___@_____ encoded bits __________|_ blkaddr _| 0 . amortized * vcnt = 8 . . . . amortized * vcnt - 4 = 4 . . .____________________________. |_type (2 bits)_|_clusterofs_| Therefore, encoded bits for each pack are 32 bits (4 bytes). IOWs, since each lcluster can get 16 bits for its type and clusterofs, the maximum supported lclustersize for compact 4B format is 16k (14 bits). Fix this to enable compact 4B format for 16k lclusters (blocks), which is tested on an arm64 server with 16k page size. Fixes: 152a333a5895 ("staging: erofs: add compacted compression indexes support") Signed-off-by: Gao Xiang Link: https://lore.kernel.org/r/20230601112341.56960-1-hsiangkao@linux.alibaba.com --- fs/erofs/zmap.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c index d37c5c89c728..920fb4dbc731 100644 --- a/fs/erofs/zmap.c +++ b/fs/erofs/zmap.c @@ -129,7 +129,7 @@ static int unpack_compacted_index(struct z_erofs_maprecorder *m, u8 *in, type; bool big_pcluster; - if (1 << amortizedshift == 4) + if (1 << amortizedshift == 4 && lclusterbits <= 14) vcnt = 2; else if (1 << amortizedshift == 2 && lclusterbits == 12) vcnt = 16; @@ -231,7 +231,6 @@ static int compacted_load_cluster_from_disk(struct z_erofs_maprecorder *m, { struct inode *const inode = m->inode; struct erofs_inode *const vi = EROFS_I(inode); - const unsigned int lclusterbits = vi->z_logical_clusterbits; const erofs_off_t ebase = sizeof(struct z_erofs_map_header) + ALIGN(erofs_iloc(inode) + vi->inode_isize + vi->xattr_isize, 8); unsigned int totalidx = erofs_iblks(inode); @@ -239,9 +238,6 @@ static int compacted_load_cluster_from_disk(struct z_erofs_maprecorder *m, unsigned int amortizedshift; erofs_off_t pos; - if (lclusterbits != 12) - return -EOPNOTSUPP; - if (lcn >= totalidx) return -EINVAL; -- cgit v1.2.3 From eba67eb6de441909a22090bc77206c91134cd58c Mon Sep 17 00:00:00 2001 From: Jingbo Xu Date: Tue, 13 Jun 2023 15:41:10 +0800 Subject: erofs: use absolute position in xattr iterator Replace blkaddr/ofs with pos in 'struct erofs_xattr_iter'. After erofs_bread() is introduced to replace raw page cache APIs for metadata I/Os handling, xattr_iter_fixup() is no longer needed anymore. In addition, it is also unnecessary to check if the iterated position is span over the block boundary as absolute offset is used instead of blkaddr + offset pairs. Signed-off-by: Jingbo Xu Reviewed-by: Gao Xiang Link: https://lore.kernel.org/r/20230613074114.120115-2-jefflexu@linux.alibaba.com Signed-off-by: Gao Xiang --- fs/erofs/xattr.c | 162 ++++++++++++++++++++++--------------------------------- 1 file changed, 65 insertions(+), 97 deletions(-) (limited to 'fs') diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c index d9e041d27a35..4c11d4f4cf07 100644 --- a/fs/erofs/xattr.c +++ b/fs/erofs/xattr.c @@ -7,26 +7,11 @@ #include #include "xattr.h" -static inline erofs_blk_t erofs_xattr_blkaddr(struct super_block *sb, - unsigned int xattr_id) -{ - return EROFS_SB(sb)->xattr_blkaddr + - erofs_blknr(sb, xattr_id * sizeof(__u32)); -} - -static inline unsigned int erofs_xattr_blkoff(struct super_block *sb, - unsigned int xattr_id) -{ - return erofs_blkoff(sb, xattr_id * sizeof(__u32)); -} - struct xattr_iter { struct super_block *sb; struct erofs_buf buf; + erofs_off_t pos; void *kaddr; - - erofs_blk_t blkaddr; - unsigned int ofs; }; static int erofs_init_inode_xattrs(struct inode *inode) @@ -82,17 +67,16 @@ static int erofs_init_inode_xattrs(struct inode *inode) it.buf = __EROFS_BUF_INITIALIZER; erofs_init_metabuf(&it.buf, sb); - it.blkaddr = erofs_blknr(sb, erofs_iloc(inode) + vi->inode_isize); - it.ofs = erofs_blkoff(sb, erofs_iloc(inode) + vi->inode_isize); + it.pos = erofs_iloc(inode) + vi->inode_isize; /* read in shared xattr array (non-atomic, see kmalloc below) */ - it.kaddr = erofs_bread(&it.buf, it.blkaddr, EROFS_KMAP); + it.kaddr = erofs_bread(&it.buf, erofs_blknr(sb, it.pos), EROFS_KMAP); if (IS_ERR(it.kaddr)) { ret = PTR_ERR(it.kaddr); goto out_unlock; } - ih = (struct erofs_xattr_ibody_header *)(it.kaddr + it.ofs); + ih = it.kaddr + erofs_blkoff(sb, it.pos); vi->xattr_shared_count = ih->h_shared_count; vi->xattr_shared_xattrs = kmalloc_array(vi->xattr_shared_count, sizeof(uint), GFP_KERNEL); @@ -103,25 +87,20 @@ static int erofs_init_inode_xattrs(struct inode *inode) } /* let's skip ibody header */ - it.ofs += sizeof(struct erofs_xattr_ibody_header); + it.pos += sizeof(struct erofs_xattr_ibody_header); for (i = 0; i < vi->xattr_shared_count; ++i) { - if (it.ofs >= sb->s_blocksize) { - /* cannot be unaligned */ - DBG_BUGON(it.ofs != sb->s_blocksize); - - it.kaddr = erofs_bread(&it.buf, ++it.blkaddr, EROFS_KMAP); - if (IS_ERR(it.kaddr)) { - kfree(vi->xattr_shared_xattrs); - vi->xattr_shared_xattrs = NULL; - ret = PTR_ERR(it.kaddr); - goto out_unlock; - } - it.ofs = 0; + it.kaddr = erofs_bread(&it.buf, erofs_blknr(sb, it.pos), + EROFS_KMAP); + if (IS_ERR(it.kaddr)) { + kfree(vi->xattr_shared_xattrs); + vi->xattr_shared_xattrs = NULL; + ret = PTR_ERR(it.kaddr); + goto out_unlock; } - vi->xattr_shared_xattrs[i] = - le32_to_cpu(*(__le32 *)(it.kaddr + it.ofs)); - it.ofs += sizeof(__le32); + vi->xattr_shared_xattrs[i] = le32_to_cpu(*(__le32 *) + (it.kaddr + erofs_blkoff(sb, it.pos))); + it.pos += sizeof(__le32); } erofs_put_metabuf(&it.buf); @@ -150,24 +129,11 @@ struct xattr_iter_handlers { unsigned int len); }; -static inline int xattr_iter_fixup(struct xattr_iter *it) -{ - if (it->ofs < it->sb->s_blocksize) - return 0; - - it->blkaddr += erofs_blknr(it->sb, it->ofs); - it->kaddr = erofs_bread(&it->buf, it->blkaddr, EROFS_KMAP); - if (IS_ERR(it->kaddr)) - return PTR_ERR(it->kaddr); - it->ofs = erofs_blkoff(it->sb, it->ofs); - return 0; -} - static int inline_xattr_iter_begin(struct xattr_iter *it, struct inode *inode) { struct erofs_inode *const vi = EROFS_I(inode); - unsigned int xattr_header_sz, inline_xattr_ofs; + unsigned int xattr_header_sz; xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) + sizeof(u32) * vi->xattr_shared_count; @@ -176,11 +142,9 @@ static int inline_xattr_iter_begin(struct xattr_iter *it, return -ENOATTR; } - inline_xattr_ofs = vi->inode_isize + xattr_header_sz; - - it->blkaddr = erofs_blknr(it->sb, erofs_iloc(inode) + inline_xattr_ofs); - it->ofs = erofs_blkoff(it->sb, erofs_iloc(inode) + inline_xattr_ofs); - it->kaddr = erofs_bread(&it->buf, it->blkaddr, EROFS_KMAP); + it->pos = erofs_iloc(inode) + vi->inode_isize + xattr_header_sz; + it->kaddr = erofs_bread(&it->buf, erofs_blknr(it->sb, it->pos), + EROFS_KMAP); if (IS_ERR(it->kaddr)) return PTR_ERR(it->kaddr); return vi->xattr_isize - xattr_header_sz; @@ -188,27 +152,29 @@ static int inline_xattr_iter_begin(struct xattr_iter *it, /* * Regardless of success or failure, `xattr_foreach' will end up with - * `ofs' pointing to the next xattr item rather than an arbitrary position. + * `pos' pointing to the next xattr item rather than an arbitrary position. */ static int xattr_foreach(struct xattr_iter *it, const struct xattr_iter_handlers *op, unsigned int *tlimit) { struct erofs_xattr_entry entry; + struct super_block *sb = it->sb; unsigned int value_sz, processed, slice; int err; - /* 0. fixup blkaddr, ofs, ipage */ - err = xattr_iter_fixup(it); - if (err) - return err; + /* 0. fixup blkaddr, pos */ + it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), EROFS_KMAP); + if (IS_ERR(it->kaddr)) + return PTR_ERR(it->kaddr); /* * 1. read xattr entry to the memory, * since we do EROFS_XATTR_ALIGN * therefore entry should be in the page */ - entry = *(struct erofs_xattr_entry *)(it->kaddr + it->ofs); + entry = *(struct erofs_xattr_entry *) + (it->kaddr + erofs_blkoff(sb, it->pos)); if (tlimit) { unsigned int entry_sz = erofs_xattr_entry_size(&entry); @@ -220,40 +186,40 @@ static int xattr_foreach(struct xattr_iter *it, *tlimit -= entry_sz; } - it->ofs += sizeof(struct erofs_xattr_entry); + it->pos += sizeof(struct erofs_xattr_entry); value_sz = le16_to_cpu(entry.e_value_size); /* handle entry */ err = op->entry(it, &entry); if (err) { - it->ofs += entry.e_name_len + value_sz; + it->pos += entry.e_name_len + value_sz; goto out; } - /* 2. handle xattr name (ofs will finally be at the end of name) */ + /* 2. handle xattr name (pos will finally be at the end of name) */ processed = 0; while (processed < entry.e_name_len) { - if (it->ofs >= it->sb->s_blocksize) { - DBG_BUGON(it->ofs > it->sb->s_blocksize); - - err = xattr_iter_fixup(it); - if (err) - goto out; - it->ofs = 0; + it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), + EROFS_KMAP); + if (IS_ERR(it->kaddr)) { + err = PTR_ERR(it->kaddr); + goto out; } - slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs, + slice = min_t(unsigned int, + sb->s_blocksize - erofs_blkoff(sb, it->pos), entry.e_name_len - processed); /* handle name */ - err = op->name(it, processed, it->kaddr + it->ofs, slice); + err = op->name(it, processed, + it->kaddr + erofs_blkoff(sb, it->pos), slice); if (err) { - it->ofs += entry.e_name_len - processed + value_sz; + it->pos += entry.e_name_len - processed + value_sz; goto out; } - it->ofs += slice; + it->pos += slice; processed += slice; } @@ -263,31 +229,31 @@ static int xattr_foreach(struct xattr_iter *it, if (op->alloc_buffer) { err = op->alloc_buffer(it, value_sz); if (err) { - it->ofs += value_sz; + it->pos += value_sz; goto out; } } while (processed < value_sz) { - if (it->ofs >= it->sb->s_blocksize) { - DBG_BUGON(it->ofs > it->sb->s_blocksize); - - err = xattr_iter_fixup(it); - if (err) - goto out; - it->ofs = 0; + it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), + EROFS_KMAP); + if (IS_ERR(it->kaddr)) { + err = PTR_ERR(it->kaddr); + goto out; } - slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs, + slice = min_t(unsigned int, + sb->s_blocksize - erofs_blkoff(sb, it->pos), value_sz - processed); - op->value(it, processed, it->kaddr + it->ofs, slice); - it->ofs += slice; + op->value(it, processed, it->kaddr + erofs_blkoff(sb, it->pos), + slice); + it->pos += slice; processed += slice; } out: /* xattrs should be 4-byte aligned (on-disk constraint) */ - it->ofs = EROFS_XATTR_ALIGN(it->ofs); + it->pos = EROFS_XATTR_ALIGN(it->pos); return err < 0 ? err : 0; } @@ -394,14 +360,15 @@ static int shared_getxattr(struct inode *inode, struct getxattr_iter *it) { struct erofs_inode *const vi = EROFS_I(inode); struct super_block *const sb = it->it.sb; - unsigned int i, xsid; + struct erofs_sb_info *sbi = EROFS_SB(sb); + unsigned int i; int ret = -ENOATTR; for (i = 0; i < vi->xattr_shared_count; ++i) { - xsid = vi->xattr_shared_xattrs[i]; - it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid); - it->it.ofs = erofs_xattr_blkoff(sb, xsid); - it->it.kaddr = erofs_bread(&it->it.buf, it->it.blkaddr, EROFS_KMAP); + it->it.pos = erofs_pos(sb, sbi->xattr_blkaddr) + + vi->xattr_shared_xattrs[i] * sizeof(__le32); + it->it.kaddr = erofs_bread(&it->it.buf, + erofs_blknr(sb, it->it.pos), EROFS_KMAP); if (IS_ERR(it->it.kaddr)) return PTR_ERR(it->it.kaddr); @@ -599,14 +566,15 @@ static int shared_listxattr(struct listxattr_iter *it) struct inode *const inode = d_inode(it->dentry); struct erofs_inode *const vi = EROFS_I(inode); struct super_block *const sb = it->it.sb; - unsigned int i, xsid; + struct erofs_sb_info *sbi = EROFS_SB(sb); + unsigned int i; int ret = 0; for (i = 0; i < vi->xattr_shared_count; ++i) { - xsid = vi->xattr_shared_xattrs[i]; - it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid); - it->it.ofs = erofs_xattr_blkoff(sb, xsid); - it->it.kaddr = erofs_bread(&it->it.buf, it->it.blkaddr, EROFS_KMAP); + it->it.pos = erofs_pos(sb, sbi->xattr_blkaddr) + + vi->xattr_shared_xattrs[i] * sizeof(__le32); + it->it.kaddr = erofs_bread(&it->it.buf, + erofs_blknr(sb, it->it.pos), EROFS_KMAP); if (IS_ERR(it->it.kaddr)) return PTR_ERR(it->it.kaddr); -- cgit v1.2.3 From 8e823961de5a3b502f47a5461954024cc1433147 Mon Sep 17 00:00:00 2001 From: Jingbo Xu Date: Tue, 13 Jun 2023 15:41:11 +0800 Subject: erofs: unify xattr_iter structures Unify xattr_iter/listxattr_iter/getxattr_iter structures into erofs_xattr_iter structure. This is in preparation for the following further cleanup. Signed-off-by: Jingbo Xu Reviewed-by: Gao Xiang Link: https://lore.kernel.org/r/20230613074114.120115-3-jefflexu@linux.alibaba.com Signed-off-by: Gao Xiang --- fs/erofs/xattr.c | 146 +++++++++++++++++++++++-------------------------------- 1 file changed, 62 insertions(+), 84 deletions(-) (limited to 'fs') diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c index 4c11d4f4cf07..b2802121e3aa 100644 --- a/fs/erofs/xattr.c +++ b/fs/erofs/xattr.c @@ -7,17 +7,27 @@ #include #include "xattr.h" -struct xattr_iter { +struct erofs_xattr_iter { struct super_block *sb; struct erofs_buf buf; erofs_off_t pos; void *kaddr; + + char *buffer; + int buffer_size, buffer_ofs; + + /* getxattr */ + int index, infix_len; + struct qstr name; + + /* listxattr */ + struct dentry *dentry; }; static int erofs_init_inode_xattrs(struct inode *inode) { struct erofs_inode *const vi = EROFS_I(inode); - struct xattr_iter it; + struct erofs_xattr_iter it; unsigned int i; struct erofs_xattr_ibody_header *ih; struct super_block *sb = inode->i_sb; @@ -121,15 +131,15 @@ out_unlock: * and need to be handled */ struct xattr_iter_handlers { - int (*entry)(struct xattr_iter *_it, struct erofs_xattr_entry *entry); - int (*name)(struct xattr_iter *_it, unsigned int processed, char *buf, + int (*entry)(struct erofs_xattr_iter *it, struct erofs_xattr_entry *entry); + int (*name)(struct erofs_xattr_iter *it, unsigned int processed, char *buf, unsigned int len); - int (*alloc_buffer)(struct xattr_iter *_it, unsigned int value_sz); - void (*value)(struct xattr_iter *_it, unsigned int processed, char *buf, + int (*alloc_buffer)(struct erofs_xattr_iter *it, unsigned int value_sz); + void (*value)(struct erofs_xattr_iter *it, unsigned int processed, char *buf, unsigned int len); }; -static int inline_xattr_iter_begin(struct xattr_iter *it, +static int inline_xattr_iter_begin(struct erofs_xattr_iter *it, struct inode *inode) { struct erofs_inode *const vi = EROFS_I(inode); @@ -154,7 +164,7 @@ static int inline_xattr_iter_begin(struct xattr_iter *it, * Regardless of success or failure, `xattr_foreach' will end up with * `pos' pointing to the next xattr item rather than an arbitrary position. */ -static int xattr_foreach(struct xattr_iter *it, +static int xattr_foreach(struct erofs_xattr_iter *it, const struct xattr_iter_handlers *op, unsigned int *tlimit) { @@ -257,18 +267,10 @@ out: return err < 0 ? err : 0; } -struct getxattr_iter { - struct xattr_iter it; - - char *buffer; - int buffer_size, index, infix_len; - struct qstr name; -}; - -static int erofs_xattr_long_entrymatch(struct getxattr_iter *it, +static int erofs_xattr_long_entrymatch(struct erofs_xattr_iter *it, struct erofs_xattr_entry *entry) { - struct erofs_sb_info *sbi = EROFS_SB(it->it.sb); + struct erofs_sb_info *sbi = EROFS_SB(it->sb); struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes + (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK); @@ -286,11 +288,9 @@ static int erofs_xattr_long_entrymatch(struct getxattr_iter *it, return 0; } -static int xattr_entrymatch(struct xattr_iter *_it, +static int xattr_entrymatch(struct erofs_xattr_iter *it, struct erofs_xattr_entry *entry) { - struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it); - /* should also match the infix for long name prefixes */ if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX) return erofs_xattr_long_entrymatch(it, entry); @@ -302,32 +302,27 @@ static int xattr_entrymatch(struct xattr_iter *_it, return 0; } -static int xattr_namematch(struct xattr_iter *_it, +static int xattr_namematch(struct erofs_xattr_iter *it, unsigned int processed, char *buf, unsigned int len) { - struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it); - if (memcmp(buf, it->name.name + it->infix_len + processed, len)) return -ENOATTR; return 0; } -static int xattr_checkbuffer(struct xattr_iter *_it, +static int xattr_checkbuffer(struct erofs_xattr_iter *it, unsigned int value_sz) { - struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it); int err = it->buffer_size < value_sz ? -ERANGE : 0; it->buffer_size = value_sz; return !it->buffer ? 1 : err; } -static void xattr_copyvalue(struct xattr_iter *_it, +static void xattr_copyvalue(struct erofs_xattr_iter *it, unsigned int processed, char *buf, unsigned int len) { - struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it); - memcpy(it->buffer + processed, buf, len); } @@ -338,41 +333,41 @@ static const struct xattr_iter_handlers find_xattr_handlers = { .value = xattr_copyvalue }; -static int inline_getxattr(struct inode *inode, struct getxattr_iter *it) +static int inline_getxattr(struct inode *inode, struct erofs_xattr_iter *it) { int ret; unsigned int remaining; - ret = inline_xattr_iter_begin(&it->it, inode); + ret = inline_xattr_iter_begin(it, inode); if (ret < 0) return ret; remaining = ret; while (remaining) { - ret = xattr_foreach(&it->it, &find_xattr_handlers, &remaining); + ret = xattr_foreach(it, &find_xattr_handlers, &remaining); if (ret != -ENOATTR) break; } return ret ? ret : it->buffer_size; } -static int shared_getxattr(struct inode *inode, struct getxattr_iter *it) +static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it) { struct erofs_inode *const vi = EROFS_I(inode); - struct super_block *const sb = it->it.sb; + struct super_block *const sb = it->sb; struct erofs_sb_info *sbi = EROFS_SB(sb); unsigned int i; int ret = -ENOATTR; for (i = 0; i < vi->xattr_shared_count; ++i) { - it->it.pos = erofs_pos(sb, sbi->xattr_blkaddr) + + it->pos = erofs_pos(sb, sbi->xattr_blkaddr) + vi->xattr_shared_xattrs[i] * sizeof(__le32); - it->it.kaddr = erofs_bread(&it->it.buf, - erofs_blknr(sb, it->it.pos), EROFS_KMAP); - if (IS_ERR(it->it.kaddr)) - return PTR_ERR(it->it.kaddr); + it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), + EROFS_KMAP); + if (IS_ERR(it->kaddr)) + return PTR_ERR(it->kaddr); - ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL); + ret = xattr_foreach(it, &find_xattr_handlers, NULL); if (ret != -ENOATTR) break; } @@ -394,7 +389,7 @@ int erofs_getxattr(struct inode *inode, int index, void *buffer, size_t buffer_size) { int ret; - struct getxattr_iter it; + struct erofs_xattr_iter it; if (!name) return -EINVAL; @@ -404,22 +399,21 @@ int erofs_getxattr(struct inode *inode, int index, return ret; it.index = index; - it.name.len = strlen(name); + it.name = (struct qstr)QSTR_INIT(name, strlen(name)); if (it.name.len > EROFS_NAME_LEN) return -ERANGE; - it.it.sb = inode->i_sb; - it.it.buf = __EROFS_BUF_INITIALIZER; - erofs_init_metabuf(&it.it.buf, it.it.sb); - it.name.name = name; - + it.sb = inode->i_sb; + it.buf = __EROFS_BUF_INITIALIZER; + erofs_init_metabuf(&it.buf, it.sb); it.buffer = buffer; it.buffer_size = buffer_size; + it.buffer_ofs = 0; ret = inline_getxattr(inode, &it); if (ret == -ENOATTR) ret = shared_getxattr(inode, &it); - erofs_put_metabuf(&it.it.buf); + erofs_put_metabuf(&it.buf); return ret; } @@ -465,25 +459,15 @@ const struct xattr_handler *erofs_xattr_handlers[] = { NULL, }; -struct listxattr_iter { - struct xattr_iter it; - - struct dentry *dentry; - char *buffer; - int buffer_size, buffer_ofs; -}; - -static int xattr_entrylist(struct xattr_iter *_it, +static int xattr_entrylist(struct erofs_xattr_iter *it, struct erofs_xattr_entry *entry) { - struct listxattr_iter *it = - container_of(_it, struct listxattr_iter, it); unsigned int base_index = entry->e_name_index; unsigned int prefix_len, infix_len = 0; const char *prefix, *infix = NULL; if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX) { - struct erofs_sb_info *sbi = EROFS_SB(_it->sb); + struct erofs_sb_info *sbi = EROFS_SB(it->sb); struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes + (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK); @@ -515,23 +499,17 @@ static int xattr_entrylist(struct xattr_iter *_it, return 0; } -static int xattr_namelist(struct xattr_iter *_it, +static int xattr_namelist(struct erofs_xattr_iter *it, unsigned int processed, char *buf, unsigned int len) { - struct listxattr_iter *it = - container_of(_it, struct listxattr_iter, it); - memcpy(it->buffer + it->buffer_ofs, buf, len); it->buffer_ofs += len; return 0; } -static int xattr_skipvalue(struct xattr_iter *_it, +static int xattr_skipvalue(struct erofs_xattr_iter *it, unsigned int value_sz) { - struct listxattr_iter *it = - container_of(_it, struct listxattr_iter, it); - it->buffer[it->buffer_ofs++] = '\0'; return 1; } @@ -543,42 +521,42 @@ static const struct xattr_iter_handlers list_xattr_handlers = { .value = NULL }; -static int inline_listxattr(struct listxattr_iter *it) +static int inline_listxattr(struct erofs_xattr_iter *it) { int ret; unsigned int remaining; - ret = inline_xattr_iter_begin(&it->it, d_inode(it->dentry)); + ret = inline_xattr_iter_begin(it, d_inode(it->dentry)); if (ret < 0) return ret; remaining = ret; while (remaining) { - ret = xattr_foreach(&it->it, &list_xattr_handlers, &remaining); + ret = xattr_foreach(it, &list_xattr_handlers, &remaining); if (ret) break; } return ret ? ret : it->buffer_ofs; } -static int shared_listxattr(struct listxattr_iter *it) +static int shared_listxattr(struct erofs_xattr_iter *it) { struct inode *const inode = d_inode(it->dentry); struct erofs_inode *const vi = EROFS_I(inode); - struct super_block *const sb = it->it.sb; + struct super_block *const sb = it->sb; struct erofs_sb_info *sbi = EROFS_SB(sb); unsigned int i; int ret = 0; for (i = 0; i < vi->xattr_shared_count; ++i) { - it->it.pos = erofs_pos(sb, sbi->xattr_blkaddr) + + it->pos = erofs_pos(sb, sbi->xattr_blkaddr) + vi->xattr_shared_xattrs[i] * sizeof(__le32); - it->it.kaddr = erofs_bread(&it->it.buf, - erofs_blknr(sb, it->it.pos), EROFS_KMAP); - if (IS_ERR(it->it.kaddr)) - return PTR_ERR(it->it.kaddr); + it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), + EROFS_KMAP); + if (IS_ERR(it->kaddr)) + return PTR_ERR(it->kaddr); - ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL); + ret = xattr_foreach(it, &list_xattr_handlers, NULL); if (ret) break; } @@ -589,7 +567,7 @@ ssize_t erofs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size) { int ret; - struct listxattr_iter it; + struct erofs_xattr_iter it; ret = erofs_init_inode_xattrs(d_inode(dentry)); if (ret == -ENOATTR) @@ -597,9 +575,9 @@ ssize_t erofs_listxattr(struct dentry *dentry, if (ret) return ret; - it.it.sb = dentry->d_sb; - it.it.buf = __EROFS_BUF_INITIALIZER; - erofs_init_metabuf(&it.it.buf, it.it.sb); + it.sb = dentry->d_sb; + it.buf = __EROFS_BUF_INITIALIZER; + erofs_init_metabuf(&it.buf, it.sb); it.dentry = dentry; it.buffer = buffer; it.buffer_size = buffer_size; @@ -608,7 +586,7 @@ ssize_t erofs_listxattr(struct dentry *dentry, ret = inline_listxattr(&it); if (ret >= 0 || ret == -ENOATTR) ret = shared_listxattr(&it); - erofs_put_metabuf(&it.it.buf); + erofs_put_metabuf(&it.buf); return ret; } -- cgit v1.2.3 From 5a8ffb1975c5b6511a996383fce7ad0f97132a5c Mon Sep 17 00:00:00 2001 From: Jingbo Xu Date: Tue, 13 Jun 2023 15:41:12 +0800 Subject: erofs: make the size of read data stored in buffer_ofs Since now xattr_iter structures have been unified, make the size of the read data stored in buffer_ofs. Don't bother reusing buffer_size for this use, which may be confusing. This is in preparation for the following further cleanup. Signed-off-by: Jingbo Xu Reviewed-by: Gao Xiang Link: https://lore.kernel.org/r/20230613074114.120115-4-jefflexu@linux.alibaba.com Signed-off-by: Gao Xiang --- fs/erofs/xattr.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c index b2802121e3aa..8a114c7b6c66 100644 --- a/fs/erofs/xattr.c +++ b/fs/erofs/xattr.c @@ -315,7 +315,7 @@ static int xattr_checkbuffer(struct erofs_xattr_iter *it, { int err = it->buffer_size < value_sz ? -ERANGE : 0; - it->buffer_size = value_sz; + it->buffer_ofs = value_sz; return !it->buffer ? 1 : err; } @@ -348,7 +348,7 @@ static int inline_getxattr(struct inode *inode, struct erofs_xattr_iter *it) if (ret != -ENOATTR) break; } - return ret ? ret : it->buffer_size; + return ret ? ret : it->buffer_ofs; } static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it) @@ -371,7 +371,7 @@ static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it) if (ret != -ENOATTR) break; } - return ret ? ret : it->buffer_size; + return ret ? ret : it->buffer_ofs; } static bool erofs_xattr_user_list(struct dentry *dentry) -- cgit v1.2.3 From 4b077b501266c6c6784656cd8721db37c090c5df Mon Sep 17 00:00:00 2001 From: Jingbo Xu Date: Tue, 13 Jun 2023 15:41:13 +0800 Subject: erofs: unify inline/shared xattr iterators for listxattr/getxattr Make inline_{list,get}xattr() as well as inline_xattr_iter_begin() unified as erofs_xattr_iter_inline(), and shared_{list,get}xattr() unified as erofs_xattr_iter_shared(). After these changes, both erofs_xattr_iter_{inline,shared}() return 0 on success, and negative error on failure. One thing worth noting is that, the logic of returning it->buffer_ofs when there's no shared xattrs in shared_listxattr() is moved to erofs_listxattr() to make the unification possible. The only difference is that, semantically the old behavior will return ENOATTR rather than it->buffer_ofs if ENOATTR encountered when listxattr is parsing upon a specific shared xattr, while now the new behavior will return it->buffer_ofs in this case. This is not an issue, as listxattr upon a specific xattr won't return ENOATTR. Signed-off-by: Jingbo Xu Reviewed-by: Gao Xiang Link: https://lore.kernel.org/r/20230613074114.120115-5-jefflexu@linux.alibaba.com Signed-off-by: Gao Xiang --- fs/erofs/xattr.c | 190 +++++++++++++++++++++---------------------------------- 1 file changed, 73 insertions(+), 117 deletions(-) (limited to 'fs') diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c index 8a114c7b6c66..abbf8e0bcdac 100644 --- a/fs/erofs/xattr.c +++ b/fs/erofs/xattr.c @@ -139,27 +139,6 @@ struct xattr_iter_handlers { unsigned int len); }; -static int inline_xattr_iter_begin(struct erofs_xattr_iter *it, - struct inode *inode) -{ - struct erofs_inode *const vi = EROFS_I(inode); - unsigned int xattr_header_sz; - - xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) + - sizeof(u32) * vi->xattr_shared_count; - if (xattr_header_sz >= vi->xattr_isize) { - DBG_BUGON(xattr_header_sz > vi->xattr_isize); - return -ENOATTR; - } - - it->pos = erofs_iloc(inode) + vi->inode_isize + xattr_header_sz; - it->kaddr = erofs_bread(&it->buf, erofs_blknr(it->sb, it->pos), - EROFS_KMAP); - if (IS_ERR(it->kaddr)) - return PTR_ERR(it->kaddr); - return vi->xattr_isize - xattr_header_sz; -} - /* * Regardless of success or failure, `xattr_foreach' will end up with * `pos' pointing to the next xattr item rather than an arbitrary position. @@ -333,47 +312,6 @@ static const struct xattr_iter_handlers find_xattr_handlers = { .value = xattr_copyvalue }; -static int inline_getxattr(struct inode *inode, struct erofs_xattr_iter *it) -{ - int ret; - unsigned int remaining; - - ret = inline_xattr_iter_begin(it, inode); - if (ret < 0) - return ret; - - remaining = ret; - while (remaining) { - ret = xattr_foreach(it, &find_xattr_handlers, &remaining); - if (ret != -ENOATTR) - break; - } - return ret ? ret : it->buffer_ofs; -} - -static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it) -{ - struct erofs_inode *const vi = EROFS_I(inode); - struct super_block *const sb = it->sb; - struct erofs_sb_info *sbi = EROFS_SB(sb); - unsigned int i; - int ret = -ENOATTR; - - for (i = 0; i < vi->xattr_shared_count; ++i) { - it->pos = erofs_pos(sb, sbi->xattr_blkaddr) + - vi->xattr_shared_xattrs[i] * sizeof(__le32); - it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), - EROFS_KMAP); - if (IS_ERR(it->kaddr)) - return PTR_ERR(it->kaddr); - - ret = xattr_foreach(it, &find_xattr_handlers, NULL); - if (ret != -ENOATTR) - break; - } - return ret ? ret : it->buffer_ofs; -} - static bool erofs_xattr_user_list(struct dentry *dentry) { return test_opt(&EROFS_SB(dentry->d_sb)->opt, XATTR_USER); @@ -384,39 +322,6 @@ static bool erofs_xattr_trusted_list(struct dentry *dentry) return capable(CAP_SYS_ADMIN); } -int erofs_getxattr(struct inode *inode, int index, - const char *name, - void *buffer, size_t buffer_size) -{ - int ret; - struct erofs_xattr_iter it; - - if (!name) - return -EINVAL; - - ret = erofs_init_inode_xattrs(inode); - if (ret) - return ret; - - it.index = index; - it.name = (struct qstr)QSTR_INIT(name, strlen(name)); - if (it.name.len > EROFS_NAME_LEN) - return -ERANGE; - - it.sb = inode->i_sb; - it.buf = __EROFS_BUF_INITIALIZER; - erofs_init_metabuf(&it.buf, it.sb); - it.buffer = buffer; - it.buffer_size = buffer_size; - it.buffer_ofs = 0; - - ret = inline_getxattr(inode, &it); - if (ret == -ENOATTR) - ret = shared_getxattr(inode, &it); - erofs_put_metabuf(&it.buf); - return ret; -} - static int erofs_xattr_generic_get(const struct xattr_handler *handler, struct dentry *unused, struct inode *inode, const char *name, void *buffer, size_t size) @@ -521,32 +426,49 @@ static const struct xattr_iter_handlers list_xattr_handlers = { .value = NULL }; -static int inline_listxattr(struct erofs_xattr_iter *it) +static int erofs_xattr_iter_inline(struct erofs_xattr_iter *it, + struct inode *inode, bool getxattr) { + struct erofs_inode *const vi = EROFS_I(inode); + const struct xattr_iter_handlers *op; + unsigned int xattr_header_sz, remaining; int ret; - unsigned int remaining; - ret = inline_xattr_iter_begin(it, d_inode(it->dentry)); - if (ret < 0) - return ret; + xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) + + sizeof(u32) * vi->xattr_shared_count; + if (xattr_header_sz >= vi->xattr_isize) { + DBG_BUGON(xattr_header_sz > vi->xattr_isize); + return -ENOATTR; + } + + it->pos = erofs_iloc(inode) + vi->inode_isize + xattr_header_sz; + it->kaddr = erofs_bread(&it->buf, erofs_blknr(it->sb, it->pos), + EROFS_KMAP); + if (IS_ERR(it->kaddr)) + return PTR_ERR(it->kaddr); + + remaining = vi->xattr_isize - xattr_header_sz; + op = getxattr ? &find_xattr_handlers : &list_xattr_handlers; - remaining = ret; while (remaining) { - ret = xattr_foreach(it, &list_xattr_handlers, &remaining); - if (ret) + ret = xattr_foreach(it, op, &remaining); + if ((getxattr && ret != -ENOATTR) || (!getxattr && ret)) break; } - return ret ? ret : it->buffer_ofs; + return ret; } -static int shared_listxattr(struct erofs_xattr_iter *it) +static int erofs_xattr_iter_shared(struct erofs_xattr_iter *it, + struct inode *inode, bool getxattr) { - struct inode *const inode = d_inode(it->dentry); struct erofs_inode *const vi = EROFS_I(inode); struct super_block *const sb = it->sb; struct erofs_sb_info *sbi = EROFS_SB(sb); unsigned int i; - int ret = 0; + const struct xattr_iter_handlers *op; + int ret = -ENOATTR; + + op = getxattr ? &find_xattr_handlers : &list_xattr_handlers; for (i = 0; i < vi->xattr_shared_count; ++i) { it->pos = erofs_pos(sb, sbi->xattr_blkaddr) + @@ -556,20 +478,52 @@ static int shared_listxattr(struct erofs_xattr_iter *it) if (IS_ERR(it->kaddr)) return PTR_ERR(it->kaddr); - ret = xattr_foreach(it, &list_xattr_handlers, NULL); - if (ret) + ret = xattr_foreach(it, op, NULL); + if ((getxattr && ret != -ENOATTR) || (!getxattr && ret)) break; } - return ret ? ret : it->buffer_ofs; + return ret; } -ssize_t erofs_listxattr(struct dentry *dentry, - char *buffer, size_t buffer_size) +int erofs_getxattr(struct inode *inode, int index, const char *name, + void *buffer, size_t buffer_size) { int ret; struct erofs_xattr_iter it; - ret = erofs_init_inode_xattrs(d_inode(dentry)); + if (!name) + return -EINVAL; + + ret = erofs_init_inode_xattrs(inode); + if (ret) + return ret; + + it.index = index; + it.name = (struct qstr)QSTR_INIT(name, strlen(name)); + if (it.name.len > EROFS_NAME_LEN) + return -ERANGE; + + it.sb = inode->i_sb; + it.buf = __EROFS_BUF_INITIALIZER; + erofs_init_metabuf(&it.buf, it.sb); + it.buffer = buffer; + it.buffer_size = buffer_size; + it.buffer_ofs = 0; + + ret = erofs_xattr_iter_inline(&it, inode, true); + if (ret == -ENOATTR) + ret = erofs_xattr_iter_shared(&it, inode, true); + erofs_put_metabuf(&it.buf); + return ret ? ret : it.buffer_ofs; +} + +ssize_t erofs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size) +{ + int ret; + struct erofs_xattr_iter it; + struct inode *inode = d_inode(dentry); + + ret = erofs_init_inode_xattrs(inode); if (ret == -ENOATTR) return 0; if (ret) @@ -583,11 +537,13 @@ ssize_t erofs_listxattr(struct dentry *dentry, it.buffer_size = buffer_size; it.buffer_ofs = 0; - ret = inline_listxattr(&it); - if (ret >= 0 || ret == -ENOATTR) - ret = shared_listxattr(&it); + ret = erofs_xattr_iter_inline(&it, inode, false); + if (!ret || ret == -ENOATTR) + ret = erofs_xattr_iter_shared(&it, inode, false); + if (ret == -ENOATTR) + ret = 0; erofs_put_metabuf(&it.buf); - return ret; + return ret ? ret : it.buffer_ofs; } void erofs_xattr_prefixes_cleanup(struct super_block *sb) -- cgit v1.2.3 From f02615eb6f5a1e732230784bf0e3b6543540e853 Mon Sep 17 00:00:00 2001 From: Jingbo Xu Date: Tue, 13 Jun 2023 15:41:14 +0800 Subject: erofs: use separate xattr parsers for listxattr/getxattr There's a callback styled xattr parser, i.e. xattr_foreach(), which is shared among listxattr and getxattr. Convert it to two separate xattr parsers to serve listxattr and getxattr for better readability. Signed-off-by: Jingbo Xu Reviewed-by: Gao Xiang Link: https://lore.kernel.org/r/20230613074114.120115-6-jefflexu@linux.alibaba.com Signed-off-by: Gao Xiang --- fs/erofs/xattr.c | 371 ++++++++++++++++++++----------------------------------- 1 file changed, 137 insertions(+), 234 deletions(-) (limited to 'fs') diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c index abbf8e0bcdac..40178b6e0688 100644 --- a/fs/erofs/xattr.c +++ b/fs/erofs/xattr.c @@ -123,195 +123,6 @@ out_unlock: return ret; } -/* - * the general idea for these return values is - * if 0 is returned, go on processing the current xattr; - * 1 (> 0) is returned, skip this round to process the next xattr; - * -err (< 0) is returned, an error (maybe ENOXATTR) occurred - * and need to be handled - */ -struct xattr_iter_handlers { - int (*entry)(struct erofs_xattr_iter *it, struct erofs_xattr_entry *entry); - int (*name)(struct erofs_xattr_iter *it, unsigned int processed, char *buf, - unsigned int len); - int (*alloc_buffer)(struct erofs_xattr_iter *it, unsigned int value_sz); - void (*value)(struct erofs_xattr_iter *it, unsigned int processed, char *buf, - unsigned int len); -}; - -/* - * Regardless of success or failure, `xattr_foreach' will end up with - * `pos' pointing to the next xattr item rather than an arbitrary position. - */ -static int xattr_foreach(struct erofs_xattr_iter *it, - const struct xattr_iter_handlers *op, - unsigned int *tlimit) -{ - struct erofs_xattr_entry entry; - struct super_block *sb = it->sb; - unsigned int value_sz, processed, slice; - int err; - - /* 0. fixup blkaddr, pos */ - it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), EROFS_KMAP); - if (IS_ERR(it->kaddr)) - return PTR_ERR(it->kaddr); - - /* - * 1. read xattr entry to the memory, - * since we do EROFS_XATTR_ALIGN - * therefore entry should be in the page - */ - entry = *(struct erofs_xattr_entry *) - (it->kaddr + erofs_blkoff(sb, it->pos)); - if (tlimit) { - unsigned int entry_sz = erofs_xattr_entry_size(&entry); - - /* xattr on-disk corruption: xattr entry beyond xattr_isize */ - if (*tlimit < entry_sz) { - DBG_BUGON(1); - return -EFSCORRUPTED; - } - *tlimit -= entry_sz; - } - - it->pos += sizeof(struct erofs_xattr_entry); - value_sz = le16_to_cpu(entry.e_value_size); - - /* handle entry */ - err = op->entry(it, &entry); - if (err) { - it->pos += entry.e_name_len + value_sz; - goto out; - } - - /* 2. handle xattr name (pos will finally be at the end of name) */ - processed = 0; - - while (processed < entry.e_name_len) { - it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), - EROFS_KMAP); - if (IS_ERR(it->kaddr)) { - err = PTR_ERR(it->kaddr); - goto out; - } - - slice = min_t(unsigned int, - sb->s_blocksize - erofs_blkoff(sb, it->pos), - entry.e_name_len - processed); - - /* handle name */ - err = op->name(it, processed, - it->kaddr + erofs_blkoff(sb, it->pos), slice); - if (err) { - it->pos += entry.e_name_len - processed + value_sz; - goto out; - } - - it->pos += slice; - processed += slice; - } - - /* 3. handle xattr value */ - processed = 0; - - if (op->alloc_buffer) { - err = op->alloc_buffer(it, value_sz); - if (err) { - it->pos += value_sz; - goto out; - } - } - - while (processed < value_sz) { - it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), - EROFS_KMAP); - if (IS_ERR(it->kaddr)) { - err = PTR_ERR(it->kaddr); - goto out; - } - - slice = min_t(unsigned int, - sb->s_blocksize - erofs_blkoff(sb, it->pos), - value_sz - processed); - op->value(it, processed, it->kaddr + erofs_blkoff(sb, it->pos), - slice); - it->pos += slice; - processed += slice; - } - -out: - /* xattrs should be 4-byte aligned (on-disk constraint) */ - it->pos = EROFS_XATTR_ALIGN(it->pos); - return err < 0 ? err : 0; -} - -static int erofs_xattr_long_entrymatch(struct erofs_xattr_iter *it, - struct erofs_xattr_entry *entry) -{ - struct erofs_sb_info *sbi = EROFS_SB(it->sb); - struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes + - (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK); - - if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count) - return -ENOATTR; - - if (it->index != pf->prefix->base_index || - it->name.len != entry->e_name_len + pf->infix_len) - return -ENOATTR; - - if (memcmp(it->name.name, pf->prefix->infix, pf->infix_len)) - return -ENOATTR; - - it->infix_len = pf->infix_len; - return 0; -} - -static int xattr_entrymatch(struct erofs_xattr_iter *it, - struct erofs_xattr_entry *entry) -{ - /* should also match the infix for long name prefixes */ - if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX) - return erofs_xattr_long_entrymatch(it, entry); - - if (it->index != entry->e_name_index || - it->name.len != entry->e_name_len) - return -ENOATTR; - it->infix_len = 0; - return 0; -} - -static int xattr_namematch(struct erofs_xattr_iter *it, - unsigned int processed, char *buf, unsigned int len) -{ - if (memcmp(buf, it->name.name + it->infix_len + processed, len)) - return -ENOATTR; - return 0; -} - -static int xattr_checkbuffer(struct erofs_xattr_iter *it, - unsigned int value_sz) -{ - int err = it->buffer_size < value_sz ? -ERANGE : 0; - - it->buffer_ofs = value_sz; - return !it->buffer ? 1 : err; -} - -static void xattr_copyvalue(struct erofs_xattr_iter *it, - unsigned int processed, - char *buf, unsigned int len) -{ - memcpy(it->buffer + processed, buf, len); -} - -static const struct xattr_iter_handlers find_xattr_handlers = { - .entry = xattr_entrymatch, - .name = xattr_namematch, - .alloc_buffer = xattr_checkbuffer, - .value = xattr_copyvalue -}; - static bool erofs_xattr_user_list(struct dentry *dentry) { return test_opt(&EROFS_SB(dentry->d_sb)->opt, XATTR_USER); @@ -364,20 +175,49 @@ const struct xattr_handler *erofs_xattr_handlers[] = { NULL, }; -static int xattr_entrylist(struct erofs_xattr_iter *it, - struct erofs_xattr_entry *entry) +static int erofs_xattr_copy_to_buffer(struct erofs_xattr_iter *it, + unsigned int len) +{ + unsigned int slice, processed; + struct super_block *sb = it->sb; + void *src; + + for (processed = 0; processed < len; processed += slice) { + it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), + EROFS_KMAP); + if (IS_ERR(it->kaddr)) + return PTR_ERR(it->kaddr); + + src = it->kaddr + erofs_blkoff(sb, it->pos); + slice = min_t(unsigned int, sb->s_blocksize - + erofs_blkoff(sb, it->pos), len - processed); + memcpy(it->buffer + it->buffer_ofs, src, slice); + it->buffer_ofs += slice; + it->pos += slice; + } + return 0; +} + +static int erofs_listxattr_foreach(struct erofs_xattr_iter *it) { - unsigned int base_index = entry->e_name_index; - unsigned int prefix_len, infix_len = 0; + struct erofs_xattr_entry entry; + unsigned int base_index, name_total, prefix_len, infix_len = 0; const char *prefix, *infix = NULL; + int err; - if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX) { + /* 1. handle xattr entry */ + entry = *(struct erofs_xattr_entry *) + (it->kaddr + erofs_blkoff(it->sb, it->pos)); + it->pos += sizeof(struct erofs_xattr_entry); + + base_index = entry.e_name_index; + if (entry.e_name_index & EROFS_XATTR_LONG_PREFIX) { struct erofs_sb_info *sbi = EROFS_SB(it->sb); struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes + - (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK); + (entry.e_name_index & EROFS_XATTR_LONG_PREFIX_MASK); if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count) - return 1; + return 0; infix = pf->prefix->infix; infix_len = pf->infix_len; base_index = pf->prefix->base_index; @@ -385,53 +225,102 @@ static int xattr_entrylist(struct erofs_xattr_iter *it, prefix = erofs_xattr_prefix(base_index, it->dentry); if (!prefix) - return 1; + return 0; prefix_len = strlen(prefix); + name_total = prefix_len + infix_len + entry.e_name_len + 1; if (!it->buffer) { - it->buffer_ofs += prefix_len + infix_len + - entry->e_name_len + 1; - return 1; + it->buffer_ofs += name_total; + return 0; } - if (it->buffer_ofs + prefix_len + infix_len + - + entry->e_name_len + 1 > it->buffer_size) + if (it->buffer_ofs + name_total > it->buffer_size) return -ERANGE; memcpy(it->buffer + it->buffer_ofs, prefix, prefix_len); memcpy(it->buffer + it->buffer_ofs + prefix_len, infix, infix_len); it->buffer_ofs += prefix_len + infix_len; - return 0; -} -static int xattr_namelist(struct erofs_xattr_iter *it, - unsigned int processed, char *buf, unsigned int len) -{ - memcpy(it->buffer + it->buffer_ofs, buf, len); - it->buffer_ofs += len; + /* 2. handle xattr name */ + err = erofs_xattr_copy_to_buffer(it, entry.e_name_len); + if (err) + return err; + + it->buffer[it->buffer_ofs++] = '\0'; return 0; } -static int xattr_skipvalue(struct erofs_xattr_iter *it, - unsigned int value_sz) +static int erofs_getxattr_foreach(struct erofs_xattr_iter *it) { - it->buffer[it->buffer_ofs++] = '\0'; - return 1; -} + struct super_block *sb = it->sb; + struct erofs_xattr_entry entry; + unsigned int slice, processed, value_sz; -static const struct xattr_iter_handlers list_xattr_handlers = { - .entry = xattr_entrylist, - .name = xattr_namelist, - .alloc_buffer = xattr_skipvalue, - .value = NULL -}; + /* 1. handle xattr entry */ + entry = *(struct erofs_xattr_entry *) + (it->kaddr + erofs_blkoff(sb, it->pos)); + it->pos += sizeof(struct erofs_xattr_entry); + value_sz = le16_to_cpu(entry.e_value_size); + + /* should also match the infix for long name prefixes */ + if (entry.e_name_index & EROFS_XATTR_LONG_PREFIX) { + struct erofs_sb_info *sbi = EROFS_SB(sb); + struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes + + (entry.e_name_index & EROFS_XATTR_LONG_PREFIX_MASK); + + if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count) + return -ENOATTR; + + if (it->index != pf->prefix->base_index || + it->name.len != entry.e_name_len + pf->infix_len) + return -ENOATTR; + + if (memcmp(it->name.name, pf->prefix->infix, pf->infix_len)) + return -ENOATTR; + + it->infix_len = pf->infix_len; + } else { + if (it->index != entry.e_name_index || + it->name.len != entry.e_name_len) + return -ENOATTR; + + it->infix_len = 0; + } + + /* 2. handle xattr name */ + for (processed = 0; processed < entry.e_name_len; processed += slice) { + it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), + EROFS_KMAP); + if (IS_ERR(it->kaddr)) + return PTR_ERR(it->kaddr); + + slice = min_t(unsigned int, + sb->s_blocksize - erofs_blkoff(sb, it->pos), + entry.e_name_len - processed); + if (memcmp(it->name.name + it->infix_len + processed, + it->kaddr + erofs_blkoff(sb, it->pos), slice)) + return -ENOATTR; + it->pos += slice; + } + + /* 3. handle xattr value */ + if (!it->buffer) { + it->buffer_ofs = value_sz; + return 0; + } + + if (it->buffer_size < value_sz) + return -ERANGE; + + return erofs_xattr_copy_to_buffer(it, value_sz); +} static int erofs_xattr_iter_inline(struct erofs_xattr_iter *it, struct inode *inode, bool getxattr) { struct erofs_inode *const vi = EROFS_I(inode); - const struct xattr_iter_handlers *op; - unsigned int xattr_header_sz, remaining; + unsigned int xattr_header_sz, remaining, entry_sz; + erofs_off_t next_pos; int ret; xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) + @@ -441,19 +330,33 @@ static int erofs_xattr_iter_inline(struct erofs_xattr_iter *it, return -ENOATTR; } - it->pos = erofs_iloc(inode) + vi->inode_isize + xattr_header_sz; - it->kaddr = erofs_bread(&it->buf, erofs_blknr(it->sb, it->pos), - EROFS_KMAP); - if (IS_ERR(it->kaddr)) - return PTR_ERR(it->kaddr); - remaining = vi->xattr_isize - xattr_header_sz; - op = getxattr ? &find_xattr_handlers : &list_xattr_handlers; + it->pos = erofs_iloc(inode) + vi->inode_isize + xattr_header_sz; while (remaining) { - ret = xattr_foreach(it, op, &remaining); + it->kaddr = erofs_bread(&it->buf, erofs_blknr(it->sb, it->pos), + EROFS_KMAP); + if (IS_ERR(it->kaddr)) + return PTR_ERR(it->kaddr); + + entry_sz = erofs_xattr_entry_size(it->kaddr + + erofs_blkoff(it->sb, it->pos)); + /* xattr on-disk corruption: xattr entry beyond xattr_isize */ + if (remaining < entry_sz) { + DBG_BUGON(1); + return -EFSCORRUPTED; + } + remaining -= entry_sz; + next_pos = it->pos + entry_sz; + + if (getxattr) + ret = erofs_getxattr_foreach(it); + else + ret = erofs_listxattr_foreach(it); if ((getxattr && ret != -ENOATTR) || (!getxattr && ret)) break; + + it->pos = next_pos; } return ret; } @@ -465,11 +368,8 @@ static int erofs_xattr_iter_shared(struct erofs_xattr_iter *it, struct super_block *const sb = it->sb; struct erofs_sb_info *sbi = EROFS_SB(sb); unsigned int i; - const struct xattr_iter_handlers *op; int ret = -ENOATTR; - op = getxattr ? &find_xattr_handlers : &list_xattr_handlers; - for (i = 0; i < vi->xattr_shared_count; ++i) { it->pos = erofs_pos(sb, sbi->xattr_blkaddr) + vi->xattr_shared_xattrs[i] * sizeof(__le32); @@ -478,7 +378,10 @@ static int erofs_xattr_iter_shared(struct erofs_xattr_iter *it, if (IS_ERR(it->kaddr)) return PTR_ERR(it->kaddr); - ret = xattr_foreach(it, op, NULL); + if (getxattr) + ret = erofs_getxattr_foreach(it); + else + ret = erofs_listxattr_foreach(it); if ((getxattr && ret != -ENOATTR) || (!getxattr && ret)) break; } -- cgit v1.2.3 From 12d0a24afd9ea58e581ea64d64e066f2027b28d9 Mon Sep 17 00:00:00 2001 From: Sandeep Dhavale Date: Wed, 21 Jun 2023 15:08:47 -0700 Subject: erofs: Fix detection of atomic context Current check for atomic context is not sufficient as z_erofs_decompressqueue_endio can be called under rcu lock from blk_mq_flush_plug_list(). See the stacktrace [1] In such case we should hand off the decompression work for async processing rather than trying to do sync decompression in current context. Patch fixes the detection by checking for rcu_read_lock_any_held() and while at it use more appropriate !in_task() check than in_atomic(). Background: Historically erofs would always schedule a kworker for decompression which would incur the scheduling cost regardless of the context. But z_erofs_decompressqueue_endio() may not always be in atomic context and we could actually benefit from doing the decompression in z_erofs_decompressqueue_endio() if we are in thread context, for example when running with dm-verity. This optimization was later added in patch [2] which has shown improvement in performance benchmarks. ============================================== [1] Problem stacktrace [name:core&]BUG: sleeping function called from invalid context at kernel/locking/mutex.c:291 [name:core&]in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 1615, name: CpuMonitorServi [name:core&]preempt_count: 0, expected: 0 [name:core&]RCU nest depth: 1, expected: 0 CPU: 7 PID: 1615 Comm: CpuMonitorServi Tainted: G S W OE 6.1.25-android14-5-maybe-dirty-mainline #1 Hardware name: MT6897 (DT) Call trace: dump_backtrace+0x108/0x15c show_stack+0x20/0x30 dump_stack_lvl+0x6c/0x8c dump_stack+0x20/0x48 __might_resched+0x1fc/0x308 __might_sleep+0x50/0x88 mutex_lock+0x2c/0x110 z_erofs_decompress_queue+0x11c/0xc10 z_erofs_decompress_kickoff+0x110/0x1a4 z_erofs_decompressqueue_endio+0x154/0x180 bio_endio+0x1b0/0x1d8 __dm_io_complete+0x22c/0x280 clone_endio+0xe4/0x280 bio_endio+0x1b0/0x1d8 blk_update_request+0x138/0x3a4 blk_mq_plug_issue_direct+0xd4/0x19c blk_mq_flush_plug_list+0x2b0/0x354 __blk_flush_plug+0x110/0x160 blk_finish_plug+0x30/0x4c read_pages+0x2fc/0x370 page_cache_ra_unbounded+0xa4/0x23c page_cache_ra_order+0x290/0x320 do_sync_mmap_readahead+0x108/0x2c0 filemap_fault+0x19c/0x52c __do_fault+0xc4/0x114 handle_mm_fault+0x5b4/0x1168 do_page_fault+0x338/0x4b4 do_translation_fault+0x40/0x60 do_mem_abort+0x60/0xc8 el0_da+0x4c/0xe0 el0t_64_sync_handler+0xd4/0xfc el0t_64_sync+0x1a0/0x1a4 [2] Link: https://lore.kernel.org/all/20210317035448.13921-1-huangjianan@oppo.com/ Reported-by: Will Shiu Suggested-by: Gao Xiang Signed-off-by: Sandeep Dhavale Reviewed-by: Gao Xiang Reviewed-by: Alexandre Mergnat Link: https://lore.kernel.org/r/20230621220848.3379029-1-dhavale@google.com Signed-off-by: Gao Xiang --- fs/erofs/zdata.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index 264bf553c287..5f1890e309c6 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -1447,7 +1447,7 @@ static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io, if (atomic_add_return(bios, &io->pending_bios)) return; /* Use (kthread_)work and sync decompression for atomic contexts only */ - if (in_atomic() || irqs_disabled()) { + if (!in_task() || irqs_disabled() || rcu_read_lock_any_held()) { #ifdef CONFIG_EROFS_FS_PCPU_KTHREAD struct kthread_worker *worker; -- cgit v1.2.3 From 1990595547976baa922285b999612cc3549874d6 Mon Sep 17 00:00:00 2001 From: Yangtao Li Date: Thu, 15 Jun 2023 11:45:38 +0800 Subject: erofs: remove unnecessary goto It's redundant, let's remove it. Signed-off-by: Yangtao Li Reviewed-by: Gao Xiang Reviewed-by: Jingbo Xu Link: https://lore.kernel.org/r/20230615034539.14286-1-frank.li@vivo.com Signed-off-by: Gao Xiang --- fs/erofs/super.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/erofs/super.c b/fs/erofs/super.c index c2829c91812b..ff18f6b14de5 100644 --- a/fs/erofs/super.c +++ b/fs/erofs/super.c @@ -954,10 +954,8 @@ static int __init erofs_module_init(void) sizeof(struct erofs_inode), 0, SLAB_RECLAIM_ACCOUNT, erofs_inode_init_once); - if (!erofs_inode_cachep) { - err = -ENOMEM; - goto icache_err; - } + if (!erofs_inode_cachep) + return -ENOMEM; err = erofs_init_shrinker(); if (err) @@ -992,7 +990,6 @@ lzma_err: erofs_exit_shrinker(); shrinker_err: kmem_cache_destroy(erofs_inode_cachep); -icache_err: return err; } -- cgit v1.2.3 From 8241fdd3cdfe88e31a3de09a72b5bff661e4534a Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Thu, 15 Jun 2023 14:44:21 +0800 Subject: erofs: clean up zmap.c Several trivial cleanups which aren't quite necessary to split: - Rename lcluster load functions as well as justify full indexes since they are typically used for global deduplication for compressed data; - Avoid unnecessary lines, comments for simplicity. No logic changes. Reviewed-by: Guo Xuenan Reviewed-by: Yue Hu Signed-off-by: Gao Xiang Link: https://lore.kernel.org/r/20230615064421.103178-1-hsiangkao@linux.alibaba.com --- fs/erofs/zmap.c | 69 ++++++++++++++++++++++++--------------------------------- 1 file changed, 29 insertions(+), 40 deletions(-) (limited to 'fs') diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c index 920fb4dbc731..1909ddafd9c7 100644 --- a/fs/erofs/zmap.c +++ b/fs/erofs/zmap.c @@ -22,8 +22,8 @@ struct z_erofs_maprecorder { bool partialref; }; -static int legacy_load_cluster_from_disk(struct z_erofs_maprecorder *m, - unsigned long lcn) +static int z_erofs_load_full_lcluster(struct z_erofs_maprecorder *m, + unsigned long lcn) { struct inode *const inode = m->inode; struct erofs_inode *const vi = EROFS_I(inode); @@ -226,8 +226,8 @@ static int unpack_compacted_index(struct z_erofs_maprecorder *m, return 0; } -static int compacted_load_cluster_from_disk(struct z_erofs_maprecorder *m, - unsigned long lcn, bool lookahead) +static int z_erofs_load_compact_lcluster(struct z_erofs_maprecorder *m, + unsigned long lcn, bool lookahead) { struct inode *const inode = m->inode; struct erofs_inode *const vi = EROFS_I(inode); @@ -277,23 +277,23 @@ out: return unpack_compacted_index(m, amortizedshift, pos, lookahead); } -static int z_erofs_load_cluster_from_disk(struct z_erofs_maprecorder *m, - unsigned int lcn, bool lookahead) +static int z_erofs_load_lcluster_from_disk(struct z_erofs_maprecorder *m, + unsigned int lcn, bool lookahead) { - const unsigned int datamode = EROFS_I(m->inode)->datalayout; - - if (datamode == EROFS_INODE_COMPRESSED_FULL) - return legacy_load_cluster_from_disk(m, lcn); - - if (datamode == EROFS_INODE_COMPRESSED_COMPACT) - return compacted_load_cluster_from_disk(m, lcn, lookahead); - - return -EINVAL; + switch (EROFS_I(m->inode)->datalayout) { + case EROFS_INODE_COMPRESSED_FULL: + return z_erofs_load_full_lcluster(m, lcn); + case EROFS_INODE_COMPRESSED_COMPACT: + return z_erofs_load_compact_lcluster(m, lcn, lookahead); + default: + return -EINVAL; + } } static int z_erofs_extent_lookback(struct z_erofs_maprecorder *m, unsigned int lookback_distance) { + struct super_block *sb = m->inode->i_sb; struct erofs_inode *const vi = EROFS_I(m->inode); const unsigned int lclusterbits = vi->z_logical_clusterbits; @@ -301,21 +301,15 @@ static int z_erofs_extent_lookback(struct z_erofs_maprecorder *m, unsigned long lcn = m->lcn - lookback_distance; int err; - /* load extent head logical cluster if needed */ - err = z_erofs_load_cluster_from_disk(m, lcn, false); + err = z_erofs_load_lcluster_from_disk(m, lcn, false); if (err) return err; switch (m->type) { case Z_EROFS_LCLUSTER_TYPE_NONHEAD: - if (!m->delta[0]) { - erofs_err(m->inode->i_sb, - "invalid lookback distance 0 @ nid %llu", - vi->nid); - DBG_BUGON(1); - return -EFSCORRUPTED; - } lookback_distance = m->delta[0]; + if (!lookback_distance) + goto err_bogus; continue; case Z_EROFS_LCLUSTER_TYPE_PLAIN: case Z_EROFS_LCLUSTER_TYPE_HEAD1: @@ -324,16 +318,15 @@ static int z_erofs_extent_lookback(struct z_erofs_maprecorder *m, m->map->m_la = (lcn << lclusterbits) | m->clusterofs; return 0; default: - erofs_err(m->inode->i_sb, - "unknown type %u @ lcn %lu of nid %llu", + erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu", m->type, lcn, vi->nid); DBG_BUGON(1); return -EOPNOTSUPP; } } - - erofs_err(m->inode->i_sb, "bogus lookback distance @ nid %llu", - vi->nid); +err_bogus: + erofs_err(sb, "bogus lookback distance %u @ lcn %lu of nid %llu", + lookback_distance, m->lcn, vi->nid); DBG_BUGON(1); return -EFSCORRUPTED; } @@ -365,7 +358,7 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m, if (m->compressedblks) goto out; - err = z_erofs_load_cluster_from_disk(m, lcn, false); + err = z_erofs_load_lcluster_from_disk(m, lcn, false); if (err) return err; @@ -397,9 +390,8 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m, break; fallthrough; default: - erofs_err(m->inode->i_sb, - "cannot found CBLKCNT @ lcn %lu of nid %llu", - lcn, vi->nid); + erofs_err(sb, "cannot found CBLKCNT @ lcn %lu of nid %llu", lcn, + vi->nid); DBG_BUGON(1); return -EFSCORRUPTED; } @@ -407,9 +399,7 @@ out: map->m_plen = erofs_pos(sb, m->compressedblks); return 0; err_bonus_cblkcnt: - erofs_err(m->inode->i_sb, - "bogus CBLKCNT @ lcn %lu of nid %llu", - lcn, vi->nid); + erofs_err(sb, "bogus CBLKCNT @ lcn %lu of nid %llu", lcn, vi->nid); DBG_BUGON(1); return -EFSCORRUPTED; } @@ -430,7 +420,7 @@ static int z_erofs_get_extent_decompressedlen(struct z_erofs_maprecorder *m) return 0; } - err = z_erofs_load_cluster_from_disk(m, lcn, true); + err = z_erofs_load_lcluster_from_disk(m, lcn, true); if (err) return err; @@ -477,7 +467,7 @@ static int z_erofs_do_map_blocks(struct inode *inode, initial_lcn = ofs >> lclusterbits; endoff = ofs & ((1 << lclusterbits) - 1); - err = z_erofs_load_cluster_from_disk(&m, initial_lcn, false); + err = z_erofs_load_lcluster_from_disk(&m, initial_lcn, false); if (err) goto unmap_out; @@ -535,8 +525,7 @@ static int z_erofs_do_map_blocks(struct inode *inode, if (flags & EROFS_GET_BLOCKS_FINDTAIL) { vi->z_tailextent_headlcn = m.lcn; /* for non-compact indexes, fragmentoff is 64 bits */ - if (fragment && - vi->datalayout == EROFS_INODE_COMPRESSED_FULL) + if (fragment && vi->datalayout == EROFS_INODE_COMPRESSED_FULL) vi->z_fragmentoff |= (u64)m.pblk << 32; } if (ztailpacking && m.lcn == vi->z_tailextent_headlcn) { -- cgit v1.2.3