* [PATCH 1/4] erofs: fix setting up pcluster for temporary pages [not found] <20201022145724.27284-1-hsiangkao.ref@aol.com> @ 2020-10-22 14:57 ` Gao Xiang via Linux-erofs 2020-10-22 14:57 ` [PATCH 2/4] erofs: get rid of magical Z_EROFS_MAPPING_STAGING Gao Xiang via Linux-erofs ` (4 more replies) 0 siblings, 5 replies; 11+ messages in thread From: Gao Xiang via Linux-erofs @ 2020-10-22 14:57 UTC (permalink / raw) To: linux-erofs; +Cc: stable, LKML From: Gao Xiang <hsiangkao@redhat.com> pcluster should be only set up for all managed pages instead of temporary pages. Since it currently uses page->mapping to identify, the impact is minor for now. Fixes: 5ddcee1f3a1c ("erofs: get rid of __stagingpage_alloc helper") Cc: <stable@vger.kernel.org> # 5.5+ Signed-off-by: Gao Xiang <hsiangkao@redhat.com> --- fs/erofs/zdata.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index 50912a5420b4..86fd3bf62af6 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -1078,8 +1078,11 @@ static struct page *pickup_page_for_submission(struct z_erofs_pcluster *pcl, cond_resched(); goto repeat; } - set_page_private(page, (unsigned long)pcl); - SetPagePrivate(page); + + if (tocache) { + set_page_private(page, (unsigned long)pcl); + SetPagePrivate(page); + } out: /* the only exit (for tracing and debugging) */ return page; } -- 2.24.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] erofs: get rid of magical Z_EROFS_MAPPING_STAGING 2020-10-22 14:57 ` [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Gao Xiang via Linux-erofs @ 2020-10-22 14:57 ` Gao Xiang via Linux-erofs 2020-10-22 14:57 ` [PATCH 3/4] erofs: insert to managed cache after adding to pcl Gao Xiang via Linux-erofs ` (3 subsequent siblings) 4 siblings, 0 replies; 11+ messages in thread From: Gao Xiang via Linux-erofs @ 2020-10-22 14:57 UTC (permalink / raw) To: linux-erofs; +Cc: LKML From: Gao Xiang <hsiangkao@redhat.com> Previously, we played around with magical page->mapping for short-lived temporary pages since we need to identify different types of pages in the same pcluster but both invalidated and short-lived temporary pages can have page->mapping == NULL. It was considered as safe because temporary pages are all non-LRU / non-movable pages. This patch tends to use specific page->private to identify short-lived pages instead so it won't rely on page->mapping anymore. Details are described in "compress.h" as well. Signed-off-by: Gao Xiang <hsiangkao@redhat.com> --- prelimitary test is done. will do stress test then. fs/erofs/compress.h | 50 ++++++++++++++++++++++++++++++----------- fs/erofs/decompressor.c | 2 +- fs/erofs/zdata.c | 42 +++++++++++++++++++++------------- fs/erofs/zdata.h | 1 + 4 files changed, 65 insertions(+), 30 deletions(-) diff --git a/fs/erofs/compress.h b/fs/erofs/compress.h index 3d452443c545..2bbf47f353ef 100644 --- a/fs/erofs/compress.h +++ b/fs/erofs/compress.h @@ -26,30 +26,54 @@ struct z_erofs_decompress_req { bool inplace_io, partial_decoding; }; +#define Z_EROFS_SHORTLIVED_PAGE (-1UL << 2) + /* - * - 0x5A110C8D ('sallocated', Z_EROFS_MAPPING_STAGING) - - * used to mark temporary allocated pages from other - * file/cached pages and NULL mapping pages. + * For all pages in a pcluster, page->private should be one of + * Type Last 2bits page->private + * short-lived page 00 Z_EROFS_SHORTLIVED_PAGE + * cached/managed page 00 pointer to z_erofs_pcluster + * online page (file-backed, 01/10/11 sub-index << 2 | count + * some pages can be used for inplace I/O) + * + * page->mapping should be one of + * Type page->mapping + * short-lived page NULL + * cached/managed page non-NULL or NULL (invalidated/truncated page) + * online page non-NULL + * + * For all managed pages, PG_private should be set with 1 extra refcount, + * which is used for page reclaim / migration. */ -#define Z_EROFS_MAPPING_STAGING ((void *)0x5A110C8D) -/* check if a page is marked as staging */ -static inline bool z_erofs_page_is_staging(struct page *page) +/* + * short-lived pages are pages directly from buddy system with specific + * page->private (no need to set PagePrivate since these are non-LRU / + * non-movable pages and bypass reclaim / migration code). + */ +static inline bool z_erofs_is_shortlived_page(struct page *page) { - return page->mapping == Z_EROFS_MAPPING_STAGING; + if (page->private != Z_EROFS_SHORTLIVED_PAGE) + return false; + + DBG_BUGON(page->mapping); + return true; } -static inline bool z_erofs_put_stagingpage(struct list_head *pagepool, - struct page *page) +static inline bool z_erofs_put_shortlivedpage(struct list_head *pagepool, + struct page *page) { - if (!z_erofs_page_is_staging(page)) + if (!z_erofs_is_shortlived_page(page)) return false; - /* staging pages should not be used by others at the same time */ - if (page_ref_count(page) > 1) + /* short-lived pages should not be used by others at the same time */ + if (page_ref_count(page) > 1) { put_page(page); - else + } else { + /* follow the pcluster rule above. */ + set_page_private(page, 0); list_add(&page->lru, pagepool); + } return true; } diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c index cbadbf55c6c2..1cb1ffd10569 100644 --- a/fs/erofs/decompressor.c +++ b/fs/erofs/decompressor.c @@ -76,7 +76,7 @@ static int z_erofs_lz4_prepare_destpages(struct z_erofs_decompress_req *rq, victim = erofs_allocpage(pagepool, GFP_KERNEL); if (!victim) return -ENOMEM; - victim->mapping = Z_EROFS_MAPPING_STAGING; + set_page_private(victim, Z_EROFS_SHORTLIVED_PAGE); } rq->out[i] = victim; } diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index 86fd3bf62af6..afeadf413c2c 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -255,6 +255,7 @@ int erofs_try_to_free_cached_page(struct address_space *mapping, erofs_workgroup_unfreeze(&pcl->obj, 1); if (ret) { + set_page_private(page, 0); ClearPagePrivate(page); put_page(page); } @@ -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); -- 2.24.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] erofs: insert to managed cache after adding to pcl 2020-10-22 14:57 ` [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Gao Xiang via Linux-erofs 2020-10-22 14:57 ` [PATCH 2/4] erofs: get rid of magical Z_EROFS_MAPPING_STAGING Gao Xiang via Linux-erofs @ 2020-10-22 14:57 ` Gao Xiang via Linux-erofs 2020-10-22 14:57 ` [PATCH 4/4] erofs: complete a missing case for inplace I/O Gao Xiang via Linux-erofs ` (2 subsequent siblings) 4 siblings, 0 replies; 11+ messages in thread From: Gao Xiang via Linux-erofs @ 2020-10-22 14:57 UTC (permalink / raw) To: linux-erofs; +Cc: LKML From: Gao Xiang <hsiangkao@redhat.com> Previously, it could be some concern to call add_to_page_cache_lru() with page->mapping == Z_EROFS_MAPPING_STAGING (!= NULL). In contrast, page->private is used instead now, so partially revert commit 5ddcee1f3a1c ("erofs: get rid of __stagingpage_alloc helper") with some adaption for simplicity. Signed-off-by: Gao Xiang <hsiangkao@redhat.com> --- fs/erofs/zdata.c | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index afeadf413c2c..edd7325570e1 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -1071,28 +1071,19 @@ static struct page *pickup_page_for_submission(struct z_erofs_pcluster *pcl, put_page(page); out_allocpage: page = erofs_allocpage(pagepool, gfp | __GFP_NOFAIL); - if (!tocache || add_to_page_cache_lru(page, mc, index + nr, gfp)) { - /* turn into temporary page if fails */ - set_page_private(page, Z_EROFS_SHORTLIVED_PAGE); - tocache = false; - } - if (oldpage != cmpxchg(&pcl->compressed_pages[nr], oldpage, page)) { - if (tocache) { - /* since it added to managed cache successfully */ - unlock_page(page); - put_page(page); - } else { - list_add(&page->lru, pagepool); - } + list_add(&page->lru, pagepool); cond_resched(); goto repeat; } - if (tocache) { - set_page_private(page, (unsigned long)pcl); - SetPagePrivate(page); + if (!tocache || add_to_page_cache_lru(page, mc, index + nr, gfp)) { + /* turn into temporary page if fails */ + set_page_private(page, Z_EROFS_SHORTLIVED_PAGE); + goto out; } + set_page_private(page, (unsigned long)pcl); + SetPagePrivate(page); out: /* the only exit (for tracing and debugging) */ return page; } -- 2.24.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] erofs: complete a missing case for inplace I/O 2020-10-22 14:57 ` [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Gao Xiang via Linux-erofs 2020-10-22 14:57 ` [PATCH 2/4] erofs: get rid of magical Z_EROFS_MAPPING_STAGING Gao Xiang via Linux-erofs 2020-10-22 14:57 ` [PATCH 3/4] erofs: insert to managed cache after adding to pcl Gao Xiang via Linux-erofs @ 2020-10-22 14:57 ` Gao Xiang via Linux-erofs 2020-10-30 12:20 ` [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Vladimir Zapolskiy 2020-11-04 1:05 ` Chao Yu 4 siblings, 0 replies; 11+ messages in thread From: Gao Xiang via Linux-erofs @ 2020-10-22 14:57 UTC (permalink / raw) To: linux-erofs; +Cc: LKML From: Gao Xiang <hsiangkao@redhat.com> Add a missing case which could cause unnecessary page allocation but not directly use inplace I/O instead, which increases runtime extra memory footprint. The detail is, considering a file-backed page, the right half of the page is chosen to be cached (e.g. the end page) and some of its data doesn't exist in managed cache, so the pcluster will be kept in the submission chain. (In other words, it cannot be decompressed in advance, for example, due to the bypass chain). Currently, the pcluster for this case is downgraded as NOINPLACE, and stop the left half of the page from doing inplace I/O (so an extra page allocation is needed then.) Let's avoid such unnecessary downgrade instead. Signed-off-by: Gao Xiang <hsiangkao@redhat.com> --- fs/erofs/zdata.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index edd7325570e1..ef12275f7fcc 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -160,7 +160,8 @@ static void preload_compressed_pages(struct z_erofs_collector *clt, const unsigned int clusterpages = BIT(pcl->clusterbits); struct page **pages = clt->compressedpages; pgoff_t index = pcl->obj.index + (pages - pcl->compressed_pages); - bool standalone = true; + bool io_cant_bypass = false; + bool updated = false; if (clt->mode < COLLECT_PRIMARY_FOLLOWED) return; @@ -180,20 +181,31 @@ static void preload_compressed_pages(struct z_erofs_collector *clt, } else if (type == DELAYEDALLOC) { t = tagptr_init(compressed_page_t, PAGE_UNALLOCATED); } else { /* DONTALLOC */ - if (standalone) + /* update to first inplace I/O page */ + if (!updated) { clt->compressedpages = pages; - standalone = false; + updated = true; + } + io_cant_bypass = true; continue; } - if (!cmpxchg_relaxed(pages, NULL, tagptr_cast_ptr(t))) + if (!cmpxchg_relaxed(pages, NULL, tagptr_cast_ptr(t))) { + if (type == DELAYEDALLOC) + io_cant_bypass = true; continue; + } if (page) put_page(page); } - if (standalone) /* downgrade to PRIMARY_FOLLOWED_NOINPLACE */ + /* + * for COLLECT_PRIMARY_FOLLOWED pcluster, if all managed pages have + * been found (which means it can be handled without submittng I/O) + * it's unsafe to do inplace I/O. let's downgrade to NOINPLACE instead. + */ + if (!io_cant_bypass) clt->mode = COLLECT_PRIMARY_FOLLOWED_NOINPLACE; } -- 2.24.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] erofs: fix setting up pcluster for temporary pages 2020-10-22 14:57 ` [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Gao Xiang via Linux-erofs ` (2 preceding siblings ...) 2020-10-22 14:57 ` [PATCH 4/4] erofs: complete a missing case for inplace I/O Gao Xiang via Linux-erofs @ 2020-10-30 12:20 ` Vladimir Zapolskiy 2020-10-30 12:47 ` Gao Xiang 2020-11-04 1:05 ` Chao Yu 4 siblings, 1 reply; 11+ messages in thread From: Vladimir Zapolskiy @ 2020-10-30 12:20 UTC (permalink / raw) To: linux-erofs, Gao Xiang; +Cc: LKML, stable Hello Gao Xiang, On 10/22/20 5:57 PM, Gao Xiang via Linux-erofs wrote: > From: Gao Xiang <hsiangkao@redhat.com> > > pcluster should be only set up for all managed pages instead of > temporary pages. Since it currently uses page->mapping to identify, > the impact is minor for now. > > Fixes: 5ddcee1f3a1c ("erofs: get rid of __stagingpage_alloc helper") > Cc: <stable@vger.kernel.org> # 5.5+ > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> I was looking exactly at this problem recently, my change is one-to-one to your fix, thus I can provide a tag: Tested-by: Vladimir Zapolskiy <vladimir@tuxera.com> The fixed problem is minor, but the kernel log becomes polluted, if a page allocation debug option is enabled: % md5sum ~/erofs/testfile BUG: Bad page state in process kworker/u9:0 pfn:687de page:0000000057b8bcb4 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x687de flags: 0x4000000000002000(private) raw: 4000000000002000 dead000000000100 dead000000000122 0000000000000000 raw: 0000000000000000 ffff888066758690 00000000ffffffff 0000000000000000 page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set Modules linked in: CPU: 1 PID: 602 Comm: kworker/u9:0 Not tainted 5.9.1 #2 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1 04/01/2014 Workqueue: erofs_unzipd z_erofs_decompressqueue_work Call Trace: dump_stack+0x84/0xba bad_page.cold+0xac/0xb1 check_free_page_bad+0xb0/0xc0 free_pcp_prepare+0x2c8/0x2d0 free_unref_page+0x18/0xf0 put_pages_list+0x11a/0x120 z_erofs_decompressqueue_work+0xc9/0x110 ? z_erofs_decompress_pcluster.isra.0+0xf10/0xf10 ? read_word_at_a_time+0x12/0x20 ? strscpy+0xc7/0x1a0 process_one_work+0x30c/0x730 worker_thread+0x91/0x640 ? __kasan_check_read+0x11/0x20 ? rescuer_thread+0x8a0/0x8a0 kthread+0x1dd/0x200 ? kthread_unpark+0xa0/0xa0 ret_from_fork+0x1f/0x30 Disabling lock debugging due to kernel taint -- Best wishes, Vladimir ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] erofs: fix setting up pcluster for temporary pages 2020-10-30 12:20 ` [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Vladimir Zapolskiy @ 2020-10-30 12:47 ` Gao Xiang 2020-10-30 13:32 ` Vladimir Zapolskiy 0 siblings, 1 reply; 11+ messages in thread From: Gao Xiang @ 2020-10-30 12:47 UTC (permalink / raw) To: Vladimir Zapolskiy; +Cc: linux-erofs, LKML, stable Hi Vladimir, On Fri, Oct 30, 2020 at 02:20:31PM +0200, Vladimir Zapolskiy wrote: > Hello Gao Xiang, > > On 10/22/20 5:57 PM, Gao Xiang via Linux-erofs wrote: > > From: Gao Xiang <hsiangkao@redhat.com> > > > > pcluster should be only set up for all managed pages instead of > > temporary pages. Since it currently uses page->mapping to identify, > > the impact is minor for now. > > > > Fixes: 5ddcee1f3a1c ("erofs: get rid of __stagingpage_alloc helper") > > Cc: <stable@vger.kernel.org> # 5.5+ > > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> > > I was looking exactly at this problem recently, my change is one-to-one > to your fix, thus I can provide a tag: > > Tested-by: Vladimir Zapolskiy <vladimir@tuxera.com> Many thanks for confirming this! I found this when I was killing magical stagingpage page->mapping, it's somewhat late :-) > > > The fixed problem is minor, but the kernel log becomes polluted, if > a page allocation debug option is enabled: > > % md5sum ~/erofs/testfile > BUG: Bad page state in process kworker/u9:0 pfn:687de > page:0000000057b8bcb4 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x687de > flags: 0x4000000000002000(private) > raw: 4000000000002000 dead000000000100 dead000000000122 0000000000000000 > raw: 0000000000000000 ffff888066758690 00000000ffffffff 0000000000000000 > page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set > Modules linked in: > CPU: 1 PID: 602 Comm: kworker/u9:0 Not tainted 5.9.1 #2 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1 04/01/2014 > Workqueue: erofs_unzipd z_erofs_decompressqueue_work > Call Trace: > dump_stack+0x84/0xba > bad_page.cold+0xac/0xb1 > check_free_page_bad+0xb0/0xc0 > free_pcp_prepare+0x2c8/0x2d0 > free_unref_page+0x18/0xf0 > put_pages_list+0x11a/0x120 > z_erofs_decompressqueue_work+0xc9/0x110 > ? z_erofs_decompress_pcluster.isra.0+0xf10/0xf10 > ? read_word_at_a_time+0x12/0x20 > ? strscpy+0xc7/0x1a0 > process_one_work+0x30c/0x730 > worker_thread+0x91/0x640 > ? __kasan_check_read+0x11/0x20 > ? rescuer_thread+0x8a0/0x8a0 > kthread+0x1dd/0x200 > ? kthread_unpark+0xa0/0xa0 > ret_from_fork+0x1f/0x30 > Disabling lock debugging due to kernel taint Yeah, I can make a pull-request to Linus if you need this to be in master now, or I can post it for v5.11-rc1 since 5.4 LTS isn't effected (and it would be only a print problem with debugging option.) Thanks, Gao Xiang > > -- > Best wishes, > Vladimir > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] erofs: fix setting up pcluster for temporary pages 2020-10-30 12:47 ` Gao Xiang @ 2020-10-30 13:32 ` Vladimir Zapolskiy 2020-10-30 14:10 ` Gao Xiang 0 siblings, 1 reply; 11+ messages in thread From: Vladimir Zapolskiy @ 2020-10-30 13:32 UTC (permalink / raw) To: Gao Xiang; +Cc: linux-erofs, LKML, stable Hi Gao Xiang, On 10/30/20 2:47 PM, Gao Xiang wrote: > Hi Vladimir, > > On Fri, Oct 30, 2020 at 02:20:31PM +0200, Vladimir Zapolskiy wrote: >> Hello Gao Xiang, >> >> On 10/22/20 5:57 PM, Gao Xiang via Linux-erofs wrote: >>> From: Gao Xiang <hsiangkao@redhat.com> >>> >>> pcluster should be only set up for all managed pages instead of >>> temporary pages. Since it currently uses page->mapping to identify, >>> the impact is minor for now. >>> >>> Fixes: 5ddcee1f3a1c ("erofs: get rid of __stagingpage_alloc helper") >>> Cc: <stable@vger.kernel.org> # 5.5+ >>> Signed-off-by: Gao Xiang <hsiangkao@redhat.com> >> >> I was looking exactly at this problem recently, my change is one-to-one >> to your fix, thus I can provide a tag: >> >> Tested-by: Vladimir Zapolskiy <vladimir@tuxera.com> > > Many thanks for confirming this! > I found this when I was killing magical stagingpage page->mapping, > it's somewhat late :-) > sure, for me it was an exciting immersion into the filesystem code :) >> >> >> The fixed problem is minor, but the kernel log becomes polluted, if >> a page allocation debug option is enabled: >> >> % md5sum ~/erofs/testfile >> BUG: Bad page state in process kworker/u9:0 pfn:687de >> page:0000000057b8bcb4 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x687de >> flags: 0x4000000000002000(private) >> raw: 4000000000002000 dead000000000100 dead000000000122 0000000000000000 >> raw: 0000000000000000 ffff888066758690 00000000ffffffff 0000000000000000 >> page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set >> Modules linked in: >> CPU: 1 PID: 602 Comm: kworker/u9:0 Not tainted 5.9.1 #2 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1 04/01/2014 >> Workqueue: erofs_unzipd z_erofs_decompressqueue_work >> Call Trace: >> dump_stack+0x84/0xba >> bad_page.cold+0xac/0xb1 >> check_free_page_bad+0xb0/0xc0 >> free_pcp_prepare+0x2c8/0x2d0 >> free_unref_page+0x18/0xf0 >> put_pages_list+0x11a/0x120 >> z_erofs_decompressqueue_work+0xc9/0x110 >> ? z_erofs_decompress_pcluster.isra.0+0xf10/0xf10 >> ? read_word_at_a_time+0x12/0x20 >> ? strscpy+0xc7/0x1a0 >> process_one_work+0x30c/0x730 >> worker_thread+0x91/0x640 >> ? __kasan_check_read+0x11/0x20 >> ? rescuer_thread+0x8a0/0x8a0 >> kthread+0x1dd/0x200 >> ? kthread_unpark+0xa0/0xa0 >> ret_from_fork+0x1f/0x30 >> Disabling lock debugging due to kernel taint > > Yeah, I can make a pull-request to Linus if you need this to be in master > now, or I can post it for v5.11-rc1 since 5.4 LTS isn't effected (and it > would be only a print problem with debugging option.) > As for myself I don't utterly need this fix on the master branch ASAP, however it might be reasonable to get it included right into the next v5.10 release, because I believe it'll be an LTS. Eventually it's up to you to make a decision, from my side I won't urge you, the fixed issue is obviously a non-critical one. Thank you for the original fix and taking my opinion into consideration :) -- Best wishes, Vladimir ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] erofs: fix setting up pcluster for temporary pages 2020-10-30 13:32 ` Vladimir Zapolskiy @ 2020-10-30 14:10 ` Gao Xiang 0 siblings, 0 replies; 11+ messages in thread From: Gao Xiang @ 2020-10-30 14:10 UTC (permalink / raw) To: Vladimir Zapolskiy; +Cc: linux-erofs, LKML, stable On Fri, Oct 30, 2020 at 03:32:55PM +0200, Vladimir Zapolskiy wrote: > Hi Gao Xiang, > > On 10/30/20 2:47 PM, Gao Xiang wrote: > > Hi Vladimir, > > > > On Fri, Oct 30, 2020 at 02:20:31PM +0200, Vladimir Zapolskiy wrote: > > > Hello Gao Xiang, > > > > > > On 10/22/20 5:57 PM, Gao Xiang via Linux-erofs wrote: > > > > From: Gao Xiang <hsiangkao@redhat.com> > > > > > > > > pcluster should be only set up for all managed pages instead of > > > > temporary pages. Since it currently uses page->mapping to identify, > > > > the impact is minor for now. > > > > > > > > Fixes: 5ddcee1f3a1c ("erofs: get rid of __stagingpage_alloc helper") > > > > Cc: <stable@vger.kernel.org> # 5.5+ > > > > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> > > > > > > I was looking exactly at this problem recently, my change is one-to-one > > > to your fix, thus I can provide a tag: > > > > > > Tested-by: Vladimir Zapolskiy <vladimir@tuxera.com> > > > > Many thanks for confirming this! > > I found this when I was killing magical stagingpage page->mapping, > > it's somewhat late :-) > > > > sure, for me it was an exciting immersion into the filesystem code :) Thanks for your effort on this! You could also post related kernel message in advance and I will definitly look into that as well. :) > > > > > > > > > > The fixed problem is minor, but the kernel log becomes polluted, if > > > a page allocation debug option is enabled: > > > > > > % md5sum ~/erofs/testfile > > > BUG: Bad page state in process kworker/u9:0 pfn:687de > > > page:0000000057b8bcb4 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x687de > > > flags: 0x4000000000002000(private) > > > raw: 4000000000002000 dead000000000100 dead000000000122 0000000000000000 > > > raw: 0000000000000000 ffff888066758690 00000000ffffffff 0000000000000000 > > > page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set > > > Modules linked in: > > > CPU: 1 PID: 602 Comm: kworker/u9:0 Not tainted 5.9.1 #2 > > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1 04/01/2014 > > > Workqueue: erofs_unzipd z_erofs_decompressqueue_work > > > Call Trace: > > > dump_stack+0x84/0xba > > > bad_page.cold+0xac/0xb1 > > > check_free_page_bad+0xb0/0xc0 > > > free_pcp_prepare+0x2c8/0x2d0 > > > free_unref_page+0x18/0xf0 > > > put_pages_list+0x11a/0x120 > > > z_erofs_decompressqueue_work+0xc9/0x110 > > > ? z_erofs_decompress_pcluster.isra.0+0xf10/0xf10 > > > ? read_word_at_a_time+0x12/0x20 > > > ? strscpy+0xc7/0x1a0 > > > process_one_work+0x30c/0x730 > > > worker_thread+0x91/0x640 > > > ? __kasan_check_read+0x11/0x20 > > > ? rescuer_thread+0x8a0/0x8a0 > > > kthread+0x1dd/0x200 > > > ? kthread_unpark+0xa0/0xa0 > > > ret_from_fork+0x1f/0x30 > > > Disabling lock debugging due to kernel taint > > > > Yeah, I can make a pull-request to Linus if you need this to be in master > > now, or I can post it for v5.11-rc1 since 5.4 LTS isn't effected (and it > > would be only a print problem with debugging option.) > > > > As for myself I don't utterly need this fix on the master branch ASAP, however > it might be reasonable to get it included right into the next v5.10 release, > because I believe it'll be an LTS. Eventually it's up to you to make a decision, > from my side I won't urge you, the fixed issue is obviously a non-critical one. > > Thank you for the original fix and taking my opinion into consideration :) Yeah, v5.10 is a LTS version, and you are right, I will try to make a pull-request after I get Chao's RVB. Thanks, Gao Xiang > > -- > Best wishes, > Vladimir > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] erofs: fix setting up pcluster for temporary pages 2020-10-22 14:57 ` [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Gao Xiang via Linux-erofs ` (3 preceding siblings ...) 2020-10-30 12:20 ` [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Vladimir Zapolskiy @ 2020-11-04 1:05 ` Chao Yu 2020-11-04 1:11 ` Gao Xiang 4 siblings, 1 reply; 11+ messages in thread From: Chao Yu @ 2020-11-04 1:05 UTC (permalink / raw) To: Gao Xiang, linux-erofs; +Cc: LKML, stable On 2020/10/22 22:57, Gao Xiang wrote: > From: Gao Xiang <hsiangkao@redhat.com> > > pcluster should be only set up for all managed pages instead of > temporary pages. Since it currently uses page->mapping to identify, > the impact is minor for now. > > Fixes: 5ddcee1f3a1c ("erofs: get rid of __stagingpage_alloc helper") > Cc: <stable@vger.kernel.org> # 5.5+ > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] erofs: fix setting up pcluster for temporary pages 2020-11-04 1:05 ` Chao Yu @ 2020-11-04 1:11 ` Gao Xiang 2020-11-04 1:44 ` Chao Yu 0 siblings, 1 reply; 11+ messages in thread From: Gao Xiang @ 2020-11-04 1:11 UTC (permalink / raw) To: Chao Yu; +Cc: stable, linux-erofs, LKML On Wed, Nov 04, 2020 at 09:05:56AM +0800, Chao Yu wrote: > On 2020/10/22 22:57, Gao Xiang wrote: > > From: Gao Xiang <hsiangkao@redhat.com> > > > > pcluster should be only set up for all managed pages instead of > > temporary pages. Since it currently uses page->mapping to identify, > > the impact is minor for now. > > > > Fixes: 5ddcee1f3a1c ("erofs: get rid of __stagingpage_alloc helper") > > Cc: <stable@vger.kernel.org> # 5.5+ > > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> > > Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, I've also added a note to the commit message like this, " [ Update: Vladimir reported the kernel log becomes polluted because PAGE_FLAGS_CHECK_AT_FREE flag(s) set if the page allocation debug option is enabled. ] " Will apply all of this to -fixes branch. Thanks, Gao Xiang > > Thanks, > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] erofs: fix setting up pcluster for temporary pages 2020-11-04 1:11 ` Gao Xiang @ 2020-11-04 1:44 ` Chao Yu 0 siblings, 0 replies; 11+ messages in thread From: Chao Yu @ 2020-11-04 1:44 UTC (permalink / raw) To: Gao Xiang; +Cc: stable, linux-erofs, LKML On 2020/11/4 9:11, Gao Xiang wrote: > On Wed, Nov 04, 2020 at 09:05:56AM +0800, Chao Yu wrote: >> On 2020/10/22 22:57, Gao Xiang wrote: >>> From: Gao Xiang <hsiangkao@redhat.com> >>> >>> pcluster should be only set up for all managed pages instead of >>> temporary pages. Since it currently uses page->mapping to identify, >>> the impact is minor for now. >>> >>> Fixes: 5ddcee1f3a1c ("erofs: get rid of __stagingpage_alloc helper") >>> Cc: <stable@vger.kernel.org> # 5.5+ >>> Signed-off-by: Gao Xiang <hsiangkao@redhat.com> >> >> Reviewed-by: Chao Yu <yuchao0@huawei.com> > > Thanks, I've also added a note to the commit message like this, > " > [ Update: Vladimir reported the kernel log becomes polluted > because PAGE_FLAGS_CHECK_AT_FREE flag(s) set if the page > allocation debug option is enabled. ] > " > Will apply all of this to -fixes branch. Thanks for noticing that, looks fine to me. Thanks, > > Thanks, > Gao Xiang > >> >> Thanks, >> > > . > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-11-04 1:45 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20201022145724.27284-1-hsiangkao.ref@aol.com> 2020-10-22 14:57 ` [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Gao Xiang via Linux-erofs 2020-10-22 14:57 ` [PATCH 2/4] erofs: get rid of magical Z_EROFS_MAPPING_STAGING Gao Xiang via Linux-erofs 2020-10-22 14:57 ` [PATCH 3/4] erofs: insert to managed cache after adding to pcl Gao Xiang via Linux-erofs 2020-10-22 14:57 ` [PATCH 4/4] erofs: complete a missing case for inplace I/O Gao Xiang via Linux-erofs 2020-10-30 12:20 ` [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Vladimir Zapolskiy 2020-10-30 12:47 ` Gao Xiang 2020-10-30 13:32 ` Vladimir Zapolskiy 2020-10-30 14:10 ` Gao Xiang 2020-11-04 1:05 ` Chao Yu 2020-11-04 1:11 ` Gao Xiang 2020-11-04 1:44 ` Chao Yu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).