On Tue, Mar 18, 2025 at 09:03:18AM +0800, Ming Lei wrote: > If vfs_flush() is called with queue frozen, the queue freeze lock may be > connected with FS internal lock, and lockdep warning can be triggered > because the queue freeze lock is connected with too many global or > sub-system locks. > > Fix the warning by moving vfs_fsync() out of loop_update_dio(): > > - vfs_fsync() is only needed when switching to dio > > - only loop_change_fd() and loop_configure() may switch from buffered > IO to direct IO, so call vfs_fsync() directly here. This way is safe > because either loop is in unbound, or new file isn't attached > > - for the other two cases of set_status and set_block_size, direct IO > can only become off, so no need to call vfs_fsync() > > Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx> > Reported-by: Kun Hu <huk23@xxxxxxxxxxxxxx> > Reported-by: Jiaji Qin <jjtan24@xxxxxxxxxxxxxx> > Closes: https://lore.kernel.org/linux-block/359BC288-B0B1-4815-9F01-3A349B12E816@xxxxxxxxxxxxxx/T/#u > Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx> > --- > V2: > - update comment(Christoph) > > drivers/block/loop.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c > index 8ca092303794..7ddb3cbc20fe 100644 > --- a/drivers/block/loop.c > +++ b/drivers/block/loop.c > @@ -189,18 +189,12 @@ static bool lo_can_use_dio(struct loop_device *lo) > */ > static inline void loop_update_dio(struct loop_device *lo) > { > - bool dio_in_use = lo->lo_flags & LO_FLAGS_DIRECT_IO; > - > lockdep_assert_held(&lo->lo_mutex); > WARN_ON_ONCE(lo->lo_state == Lo_bound && > lo->lo_queue->mq_freeze_depth == 0); > > if ((lo->lo_flags & LO_FLAGS_DIRECT_IO) && !lo_can_use_dio(lo)) > lo->lo_flags &= ~LO_FLAGS_DIRECT_IO; > - > - /* flush dirty pages before starting to issue direct I/O */ > - if ((lo->lo_flags & LO_FLAGS_DIRECT_IO) && !dio_in_use) > - vfs_fsync(lo->lo_backing_file, 0); > } > > /** > @@ -637,6 +631,9 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev, > if (get_loop_size(lo, file) != get_loop_size(lo, old_file)) > goto out_err; > > + /* may work in dio, so flush page cache before issuing dio */ > + vfs_fsync(file, 0); What does "may work" here mean? I think you mean something like: /* * We might switch to direct I/O mode for the loop device, write back * all dirty data the page cache now that so that the individual I/O * operations don't have to do that. */ ?
