Re: [PATCH 1/2] block: avoid to hold q->limits_lock across APIs for atomic update queue limits

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

On Tue, Dec 17, 2024 at 08:19:28AM +0100, Christoph Hellwig wrote:
> On Tue, Dec 17, 2024 at 03:05:48PM +0800, Ming Lei wrote:
> > On Tue, Dec 17, 2024 at 05:40:56AM +0100, Christoph Hellwig wrote:
> > > On Tue, Dec 17, 2024 at 09:52:51AM +0800, Ming Lei wrote:
> > > > The local copy can be updated in any way with any data, so does another
> > > > concurrent update on q->limits really matter?
> > > 
> > > Yes, because that means one of the updates get lost even if it is
> > > for entirely separate fields.
> > 
> > Right, but the limits are still valid anytime.
> > 
> > Any suggestion for fixing this deadlock?
> 
> What is "this deadlock"?

The commit log provides two reports:

- lockdep warning

https://lore.kernel.org/linux-block/Z1A8fai9_fQFhs1s@hovoldconsulting.com/

- real deadlock report

https://lore.kernel.org/linux-scsi/ZxG38G9BuFdBpBHZ@fedora/

It is actually one simple ABBA lock:

1) queue_attr_store()

      blk_mq_freeze_queue(q);					//queue freeze lock
      res = entry->store(disk, page, length);
	  			queue_limits_start_update		//->limits_lock
				...
				queue_limits_commit_update
      blk_mq_unfreeze_queue(q);

2) sd_revalidate_disk()

queue_limits_start_update					//->limits_lock
	sd_read_capacity()
		scsi_execute_cmd
			scsi_alloc_request
				blk_queue_enter					//queue freeze lock


Thanks,
Ming