Re: [PATCH] loop: don't call vfs_flush() with queue frozen

[Ming Lei <ming.lei@redhat.com>]

On Mon, Jan 13, 2025 at 11:07:42PM +0800, Ming Lei wrote:
> On Mon, Jan 13, 2025 at 05:49:43AM -0800, Christoph Hellwig wrote:
> > On Mon, Jan 13, 2025 at 05:24:46PM +0800, Ming Lei wrote:
> > > > Please state the locks.  Nothing fs internal here, that report is
> > > > about i_rwsem.  And a false positive because it is about ordering
> > > > of i_rwsem on the upper file system sitting on the loop device vs the
> > > > one on the lower file systems sitting below the block device.  These
> > > > obviously can't deadlock, we just need to tell lockdep about that fact.
> > > 
> > > How can you guarantee that some code won't submit IO by grabbing the
> > > i_rwsem?
> > 
> > ?  A lot of the I/O will grab i_rwsem on the underlying device.
> > Basically all writes, and for many file systems also on reads.  But
> > that is an entirely different i_rwsem as the one held the bio submitter
> > as that is in different file system.  There is no way the top file
> > system can lock i_rwsem on the lower file system except through the
> > loop driver, and that always sits below the freeze protection.

Actually some FSs may call kmalloc(GFP_KERNEL) with i_rwsem grabbed,
which could call into real deadlock if IO on the loop disk is caused by
the kmalloc(GFP_KERNEL).

So it is not one false positive.

> > 
> > > As I explained, it is fine to move out vfs_fsync() out of freeze queue.
> > > 
> > > Actually any lock which depends on freeze queue needs to take a careful
> > > look, because freeze queue connects too many global/sub-system locks.
> > 
> > For block layer locks: absolutely.  For file systems lock: not at all,
> > because we're talking about different file systems instances.  The only
> > exception would be file systems taking global locks in the I/O path,
> > but I sincerely hope no one does that.
>  
> Didn't you see the report on fs_reclaim and sysfs root lock?
> 
> https://lore.kernel.org/linux-block/197b07435a736825ab40dab8d91db031c7fce37e.camel@linux.intel.com/

There are more, such as mm->mmap_lock[1], hfs SB 'cat_tree' lock[2]...


[1] https://lore.kernel.org/linux-block/67863050.050a0220.216c54.006f.GAE@google.com/
[2] https://lore.kernel.org/linux-block/67582202.050a0220.a30f1.01cb.GAE@google.com/

Thanks,
Ming