On Tue, Dec 08, 2020 at 04:44:12PM +0800, Chao Yu wrote: > Hi Xiang, ...
> > > > I discussed this case in the original thread, > > https://lore.kernel.org/r/20200519100612.GA3687@hsiangkao-HP-ZHAN-66-Pro-G1 > > > > The previous conclusion is that for EROFS case (see Matthew's reply) this > > pair won't have too much usage. since EROFS pattern saves extra page > > reference count (- and +) by cases. > > Alright, I see. Yeah, yet in order for further confusion (or questions from others), let me update to use this pair as much as possible in the next version :( (If someone breaks in the future, I may need to remind him in time though...) > ... > > Reviewed-by: Chao Yu <yuch...@huawei.com> Thanks for the review! Thanks, Gao Xiang > > Thanks, > > > > > Thanks, > > Gao Xiang > > > > > > > > Thanks, > > > > > > > } > > > > @@ -648,12 +649,12 @@ static int z_erofs_do_read_page(struct > > > > z_erofs_decompress_frontend *fe, > > > > retry: > > > > err = z_erofs_attach_page(clt, page, page_type); > > > > - /* should allocate an additional staging page for pagevec */ > > > > + /* should allocate an additional short-lived page for pagevec */ > > > > if (err == -EAGAIN) { > > > > struct page *const newpage = > > > > alloc_page(GFP_NOFS | __GFP_NOFAIL); > > > > - newpage->mapping = Z_EROFS_MAPPING_STAGING; > > > > + set_page_private(newpage, Z_EROFS_SHORTLIVED_PAGE); > > > > err = z_erofs_attach_page(clt, newpage, > > > > Z_EROFS_PAGE_TYPE_EXCLUSIVE); > > > > if (!err) > > > > @@ -710,6 +711,11 @@ static void z_erofs_decompress_kickoff(struct > > > > z_erofs_decompressqueue *io, > > > > queue_work(z_erofs_workqueue, &io->u.work); > > > > } > > > > +static bool z_erofs_page_is_invalidated(struct page *page) > > > > +{ > > > > + return !page->mapping && !z_erofs_is_shortlived_page(page); > > > > +} > > > > + > > > > static void z_erofs_decompressqueue_endio(struct bio *bio) > > > > { > > > > tagptr1_t t = tagptr_init(tagptr1_t, bio->bi_private); > > > > @@ -722,7 +728,7 @@ static void z_erofs_decompressqueue_endio(struct > > > > bio *bio) > > > > struct page *page = bvec->bv_page; > > > > DBG_BUGON(PageUptodate(page)); > > > > - DBG_BUGON(!page->mapping); > > > > + DBG_BUGON(z_erofs_page_is_invalidated(page)); > > > > if (err) > > > > SetPageError(page); > > > > @@ -795,9 +801,9 @@ static int z_erofs_decompress_pcluster(struct > > > > super_block *sb, > > > > /* all pages in pagevec ought to be valid */ > > > > DBG_BUGON(!page); > > > > - DBG_BUGON(!page->mapping); > > > > + DBG_BUGON(z_erofs_page_is_invalidated(page)); > > > > - if (z_erofs_put_stagingpage(pagepool, page)) > > > > + if (z_erofs_put_shortlivedpage(pagepool, page)) > > > > continue; > > > > if (page_type == Z_EROFS_VLE_PAGE_TYPE_HEAD) > > > > @@ -831,9 +837,9 @@ static int z_erofs_decompress_pcluster(struct > > > > super_block *sb, > > > > /* all compressed pages ought to be valid */ > > > > DBG_BUGON(!page); > > > > - DBG_BUGON(!page->mapping); > > > > + DBG_BUGON(z_erofs_page_is_invalidated(page)); > > > > - if (!z_erofs_page_is_staging(page)) { > > > > + if (!z_erofs_is_shortlived_page(page)) { > > > > if (erofs_page_is_managed(sbi, page)) { > > > > if (!PageUptodate(page)) > > > > err = -EIO; > > > > @@ -858,7 +864,7 @@ static int z_erofs_decompress_pcluster(struct > > > > super_block *sb, > > > > overlapped = true; > > > > } > > > > - /* PG_error needs checking for inplaced and staging > > > > pages */ > > > > + /* PG_error needs checking for all non-managed pages */ > > > > if (PageError(page)) { > > > > DBG_BUGON(PageUptodate(page)); > > > > err = -EIO; > > > > @@ -897,8 +903,8 @@ static int z_erofs_decompress_pcluster(struct > > > > super_block *sb, > > > > if (erofs_page_is_managed(sbi, page)) > > > > continue; > > > > - /* recycle all individual staging pages */ > > > > - (void)z_erofs_put_stagingpage(pagepool, page); > > > > + /* recycle all individual short-lived pages */ > > > > + (void)z_erofs_put_shortlivedpage(pagepool, page); > > > > WRITE_ONCE(compressed_pages[i], NULL); > > > > } > > > > @@ -908,10 +914,10 @@ static int z_erofs_decompress_pcluster(struct > > > > super_block *sb, > > > > if (!page) > > > > continue; > > > > - DBG_BUGON(!page->mapping); > > > > + DBG_BUGON(z_erofs_page_is_invalidated(page)); > > > > - /* recycle all individual staging pages */ > > > > - if (z_erofs_put_stagingpage(pagepool, page)) > > > > + /* recycle all individual short-lived pages */ > > > > + if (z_erofs_put_shortlivedpage(pagepool, page)) > > > > continue; > > > > if (err < 0) > > > > @@ -1011,13 +1017,17 @@ static struct page > > > > *pickup_page_for_submission(struct z_erofs_pcluster *pcl, > > > > mapping = READ_ONCE(page->mapping); > > > > /* > > > > - * unmanaged (file) pages are all locked solidly, > > > > + * file-backed online pages in plcuster are all locked steady, > > > > * therefore it is impossible for `mapping' to be NULL. > > > > */ > > > > if (mapping && mapping != mc) > > > > /* ought to be unmanaged pages */ > > > > goto out; > > > > + /* directly return for shortlived page as well */ > > > > + if (z_erofs_is_shortlived_page(page)) > > > > + goto out; > > > > + > > > > lock_page(page); > > > > /* only true if page reclaim goes wrong, should never happen */ > > > > @@ -1062,8 +1072,8 @@ static struct page > > > > *pickup_page_for_submission(struct z_erofs_pcluster *pcl, > > > > out_allocpage: > > > > page = erofs_allocpage(pagepool, gfp | __GFP_NOFAIL); > > > > if (!tocache || add_to_page_cache_lru(page, mc, index + nr, > > > > gfp)) { > > > > - /* non-LRU / non-movable temporary page is needed */ > > > > - page->mapping = Z_EROFS_MAPPING_STAGING; > > > > + /* turn into temporary page if fails */ > > > > + set_page_private(page, Z_EROFS_SHORTLIVED_PAGE); > > > > tocache = false; > > > > } > > > > diff --git a/fs/erofs/zdata.h b/fs/erofs/zdata.h > > > > index 68c9b29fc0ca..b503b353d4ab 100644 > > > > --- a/fs/erofs/zdata.h > > > > +++ b/fs/erofs/zdata.h > > > > @@ -173,6 +173,7 @@ static inline void z_erofs_onlinepage_endio(struct > > > > page *page) > > > > v = atomic_dec_return(u.o); > > > > if (!(v & Z_EROFS_ONLINEPAGE_COUNT_MASK)) { > > > > + set_page_private(page, 0); > > > > ClearPagePrivate(page); > > > > if (!PageError(page)) > > > > SetPageUptodate(page); > > > > > > > > > > > . > > >
