Re: [PATCH 0/2] Fix race between do_invalidatepage and init_page_buffers
From: Jan Kara <jack@suse.cz>
Date: 2020-08-25 08:41:39
On Tue 25-08-20 10:11:29, yebin wrote:
Your patch certainly can fix the problem with my testcases, but I don't think it's a good way. There are other paths that can call do_invalidatepage , for instance block ioctl to discard and zero_range.
OK, good point! So my patch is a cleanup that stands on its own and we should do it regardless. But I agree we need more to completely fix this. I don't quite like the callback you've added just for this special case (furthermore it grows size of every buffer_head and there can be lots of those). But I agree with the general idea that we shouldn't discard buffers that the filesystem is working with. In fact I believe that fallocate(2) and zeroout/discard ioctls should return EBUSY if they are run against a mounted device because with 99% probability something went wrong and you're accidentally discarding the wrong device. But maybe I'm wrong. I'll run this idea through other fs developers. Honza
On 2020/8/24 23:51, Jan Kara wrote:quoted
On Sat 22-08-20 16:22:16, Ye Bin wrote:quoted
Ye Bin (2): ext4: Add comment to BUFFER_FLAGS_DISCARD for search code jbd2: Fix race between do_invalidatepage and init_page_buffers fs/buffer.c | 12 +++++++++++- fs/jbd2/journal.c | 7 +++++++ include/linux/buffer_head.h | 2 ++ 3 files changed, 20 insertions(+), 1 deletion(-)Thanks for the good description of the problem and the analysis. I could now easily understand what was really happening on your system. I think the problem should be fixed differently through - it is a problem of block_write_full_page() that it invalidates buffers while JBD2 is working with them. Attached patch should also fix the problem. Can you please test whether it fixes your testcase as well? Thanks! Honza
-- Jan Kara [off-list ref] SUSE Labs, CR