When we enable MMP in ext4_multi_mount_protect() during mount or
remount, we end up calling sb_start_write() from write_mmp_block(). This
triggers lockdep warning because freeze protection ranks above s_umount
semaphore we are holding during mount / remount. The problem is harmless
because we are guaranteed the filesystem is not frozen during mount /
remount but still let's fix the warning by not grabbing freeze
protection from ext4_multi_mount_protect().
Reported-by: [email protected]
Signed-off-by: Jan Kara <[email protected]>
---
fs/ext4/mmp.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index 4681fff6665f..46735ce315b5 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -39,28 +39,36 @@ static void ext4_mmp_csum_set(struct super_block *sb, struct mmp_struct *mmp)
* Write the MMP block using REQ_SYNC to try to get the block on-disk
* faster.
*/
-static int write_mmp_block(struct super_block *sb, struct buffer_head *bh)
+static int write_mmp_block_thawed(struct super_block *sb,
+ struct buffer_head *bh)
{
struct mmp_struct *mmp = (struct mmp_struct *)(bh->b_data);
- /*
- * We protect against freezing so that we don't create dirty buffers
- * on frozen filesystem.
- */
- sb_start_write(sb);
ext4_mmp_csum_set(sb, mmp);
lock_buffer(bh);
bh->b_end_io = end_buffer_write_sync;
get_bh(bh);
submit_bh(REQ_OP_WRITE | REQ_SYNC | REQ_META | REQ_PRIO, bh);
wait_on_buffer(bh);
- sb_end_write(sb);
if (unlikely(!buffer_uptodate(bh)))
return -EIO;
-
return 0;
}
+static int write_mmp_block(struct super_block *sb, struct buffer_head *bh)
+{
+ int err;
+
+ /*
+ * We protect against freezing so that we don't create dirty buffers
+ * on frozen filesystem.
+ */
+ sb_start_write(sb);
+ err = write_mmp_block_thawed(sb, bh);
+ sb_end_write(sb);
+ return err;
+}
+
/*
* Read the MMP block. It _must_ be read from disk and hence we clear the
* uptodate flag on the buffer.
@@ -340,7 +348,11 @@ int ext4_multi_mount_protect(struct super_block *sb,
seq = mmp_new_seq();
mmp->mmp_seq = cpu_to_le32(seq);
- retval = write_mmp_block(sb, bh);
+ /*
+ * On mount / remount we are protected against fs freezing (by s_umount
+ * semaphore) and grabbing freeze protection upsets lockdep
+ */
+ retval = write_mmp_block_thawed(sb, bh);
if (retval)
goto failed;
--
2.35.3
On Tue, Apr 11, 2023 at 02:10:19PM +0200, Jan Kara wrote:
> When we enable MMP in ext4_multi_mount_protect() during mount or
> remount, we end up calling sb_start_write() from write_mmp_block(). This
> triggers lockdep warning because freeze protection ranks above s_umount
> semaphore we are holding during mount / remount. The problem is harmless
> because we are guaranteed the filesystem is not frozen during mount /
> remount but still let's fix the warning by not grabbing freeze
> protection from ext4_multi_mount_protect().
>
> Reported-by: [email protected]
> Signed-off-by: Jan Kara <[email protected]>
> ---
Looks good to me,
Reviewed-by: Christian Brauner <[email protected]>
On Sun, Apr 30, 2023 at 12:34:00PM -0400, Theodore Ts'o wrote:
> On Tue, Apr 11, 2023 at 02:10:19PM +0200, Jan Kara wrote:
> > When we enable MMP in ext4_multi_mount_protect() during mount or
> > remount, we end up calling sb_start_write() from write_mmp_block(). This
> > triggers lockdep warning because freeze protection ranks above s_umount
> > semaphore we are holding during mount / remount. The problem is harmless
> > because we are guaranteed the filesystem is not frozen during mount /
> > remount but still let's fix the warning by not grabbing freeze
> > protection from ext4_multi_mount_protect().
> >
> > Reported-by: [email protected]
>
> I believe this is the wrong Reported-by. The correct one looks like
> it should be: ...
By the way, I noticed because I was browsing the syzbot dashboard for
the ext4 subsystem, and the Syzbot page for "possible deadlock in
sys_quotactl_fd"[1], I saw a discussion link to lore for the patch
"[PATCH] ext4: Fix lockdep warning when enabling MMP", and said,
"Hmm.... that looks wrong." :-)
[1] https://syzkaller.appspot.com/bug?id=1680b22e0e5eb9245a6faff10221ed76b8c5eb81
Anyway, thanks to the Syzbot team for adding Disscusion links to the
Syzbot pages. It makes it a lot easier to find discussions related to
a particular issue.
Note: you can also manually add a thread to the discussions section by
simply adding a CC to the Reported-by link for the particular issue (for
example, "Cc: [email protected]").
Cheers,
- Ted
On Tue, Apr 11, 2023 at 02:10:19PM +0200, Jan Kara wrote:
> When we enable MMP in ext4_multi_mount_protect() during mount or
> remount, we end up calling sb_start_write() from write_mmp_block(). This
> triggers lockdep warning because freeze protection ranks above s_umount
> semaphore we are holding during mount / remount. The problem is harmless
> because we are guaranteed the filesystem is not frozen during mount /
> remount but still let's fix the warning by not grabbing freeze
> protection from ext4_multi_mount_protect().
>
> Reported-by: [email protected]
I believe this is the wrong Reported-by. The correct one looks like
it should be:
Reported-by: [email protected]
Link: https://syzkaller.appspot.com/bug?id=ab7e5b6f400b7778d46f01841422e5718fb81843
It's also helpful to add a Link line to the Syzkaller dashboard to
make it easier to find the relevant Syzbot report.
I'll put this on my todo list to grab for the next batch of ext4
fixups.
- Ted
On Sun 30-04-23 12:34:00, Theodore Ts'o wrote:
> On Tue, Apr 11, 2023 at 02:10:19PM +0200, Jan Kara wrote:
> > When we enable MMP in ext4_multi_mount_protect() during mount or
> > remount, we end up calling sb_start_write() from write_mmp_block(). This
> > triggers lockdep warning because freeze protection ranks above s_umount
> > semaphore we are holding during mount / remount. The problem is harmless
> > because we are guaranteed the filesystem is not frozen during mount /
> > remount but still let's fix the warning by not grabbing freeze
> > protection from ext4_multi_mount_protect().
> >
> > Reported-by: [email protected]
>
> I believe this is the wrong Reported-by. The correct one looks like
> it should be:
>
> Reported-by: [email protected]
> Link: https://syzkaller.appspot.com/bug?id=ab7e5b6f400b7778d46f01841422e5718fb81843
Well, one of the reports of this problem has also the ID I have referenced
(https://syzkaller.appspot.com/bug?extid=6b7df7d5506b32467149). There are
apparently multiple ones...
> It's also helpful to add a Link line to the Syzkaller dashboard to
> make it easier to find the relevant Syzbot report.
Right, I'll do that next time. Thanks for the idea.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Tue, 11 Apr 2023 14:10:19 +0200, Jan Kara wrote:
> When we enable MMP in ext4_multi_mount_protect() during mount or
> remount, we end up calling sb_start_write() from write_mmp_block(). This
> triggers lockdep warning because freeze protection ranks above s_umount
> semaphore we are holding during mount / remount. The problem is harmless
> because we are guaranteed the filesystem is not frozen during mount /
> remount but still let's fix the warning by not grabbing freeze
> protection from ext4_multi_mount_protect().
>
> [...]
Applied, thanks!
[1/1] ext4: Fix lockdep warning when enabling MMP
commit: 949f95ff39bf188e594e7ecd8e29b82eb108f5bf
Best regards,
--
Theodore Ts'o <[email protected]>