From: Vladimir Zapolskiy <vladimir@tuxera.com> To: Gao Xiang <hsiangkao@redhat.com> Cc: <linux-erofs@lists.ozlabs.org>, Gao Xiang <hsiangkao@aol.com>, <stable@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Date: Fri, 30 Oct 2020 15:32:55 +0200 [thread overview] Message-ID: <02427b81-7854-1d97-662f-ab2d2b868514@tuxera.com> (raw) In-Reply-To: <20201030124745.GB133455@xiangao.remote.csb> 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
WARNING: multiple messages have this Message-ID (diff)
From: Vladimir Zapolskiy <vladimir@tuxera.com> To: Gao Xiang <hsiangkao@redhat.com> Cc: linux-erofs@lists.ozlabs.org, LKML <linux-kernel@vger.kernel.org>, stable@vger.kernel.org Subject: Re: [PATCH 1/4] erofs: fix setting up pcluster for temporary pages Date: Fri, 30 Oct 2020 15:32:55 +0200 [thread overview] Message-ID: <02427b81-7854-1d97-662f-ab2d2b868514@tuxera.com> (raw) In-Reply-To: <20201030124745.GB133455@xiangao.remote.csb> 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
next prev parent reply other threads:[~2020-10-30 13:33 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top [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 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 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 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 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-10-30 12:20 ` Vladimir Zapolskiy 2020-10-30 12:47 ` Gao Xiang 2020-10-30 12:47 ` Gao Xiang 2020-10-30 13:32 ` Vladimir Zapolskiy [this message] 2020-10-30 13:32 ` Vladimir Zapolskiy 2020-10-30 14:10 ` Gao Xiang 2020-10-30 14:10 ` Gao Xiang 2020-11-04 1:05 ` Chao Yu 2020-11-04 1:05 ` Chao Yu 2020-11-04 1:11 ` Gao Xiang 2020-11-04 1:11 ` Gao Xiang 2020-11-04 1:44 ` Chao Yu 2020-11-04 1:44 ` Chao Yu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=02427b81-7854-1d97-662f-ab2d2b868514@tuxera.com \ --to=vladimir@tuxera.com \ --cc=hsiangkao@aol.com \ --cc=hsiangkao@redhat.com \ --cc=linux-erofs@lists.ozlabs.org \ --cc=linux-kernel@vger.kernel.org \ --cc=stable@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.