Re: [PATCH 1/3] block: Fix sysfs queue freeze and limits lock order

[Christoph Hellwig <hch@lst.de>]

On Sat, Jan 04, 2025 at 09:56:14PM +0530, Nilay Shroff wrote:

[can you please quote your mails properly, having to scroll pages of
fullquotes isn't very readable]

> > +	if (entry->store) {
> > +		blk_mq_freeze_queue(q);
> > +		mutex_lock(&q->sysfs_lock);
> > +		res = entry->store(disk, page, length);
> > +		mutex_unlock(&q->sysfs_lock);
> > +		blk_mq_unfreeze_queue(q);
> > +		return res;
> > +	}
> > +
> Can we move ->sysfs_lock before freezing queue? We follow the locking order 
> of acquiring ->sysfs_lock before freeze lock. For instance, we've these 
> call sites (elevator_disable(), elevator_switch()) where we first acquire 
> ->sysfs_lock and then freezes the queue. It's OK if you think this patchset
> fixes locking order between limits lock and queue freeze and so we may address 
> it in another patch. 

sysfs_lock as-is doesn't make much sense.  The sysfs (actually kernfs)
core already serializes ->store and ->show calls vs themselves and
attribute removal, and the limits lock protects the actual limits.

The problem is that the block code also abuses the lock for various
other things, so this is non-trivial.

What I've done in my version of the series is to drop it just for the
limits method for now.