Re: [PATCH] block: model nowait as trylock for non-blocking queue enter

[Christian Borntraeger <borntraeger@linux.ibm.com>]

> Will this actually fix the lockdep splat for REQ_NOWAIT bios?
> Looking at the fast-path for bio submissions, bio_queue_enter() in
> block/blk.h still unconditionally passes 0 for the trylock parameter:
> block/blk.h:bio_queue_enter() {
> 	struct request_queue *q = bdev_get_queue(bio->bi_bdev);
> 	if (blk_try_enter_queue(q, false)) {
> 		rwsem_acquire_read(&q->io_lockdep_map, 0, 0, _RET_IP_);
> 		rwsem_release(&q->io_lockdep_map, _RET_IP_);
> 		return 0;
> 	}
> 	return __bio_queue_enter(q, bio);
> }
> Any non-blocking bio submission to an unfrozen queue (the vast majority of
> submissions) will hit this fast-path, causing lockdep to still falsely
> record a blocking dependency and potentially trigger the splat.
> Furthermore, is the updated annotation in __bio_queue_enter() ever
> reachable for a NOWAIT bio? If the queue is frozen, blk_try_enter_queue()
> fails, and the preceding loop immediately returns -EAGAIN for NOWAIT:
> 		if (nowait) {
> 			if (test_bit(GD_DEAD, &disk->state))
> 				goto dead;
> 			bio_wouldblock_error(bio);
> 			return -EAGAIN;
> 		}
> Does this make the nowait parameter in rwsem_acquire_read() here
> essentially dead code for bios?


Looks like sashiko is right on both counts. bio_queue_enter() in block/blk.h is
the only caller of __bio_queue_enter() and carries its own trylock=0 annotation
on the ▎ fast path, which is where practically every REQ_NOWAIT bio goes — so the
__bio_queue_enter() hunk alone fixes nothing for bios.

We need to annotate the fast path as well. Will do a v2.


And yes, the __bio_queue_enter() annotation is near-unreachable for a NOWAIT
bio: we only get there after the fast-path tryget already failed, and a second
failure returns -EAGAIN. It is reachable only if the queue becomes enterable
between the two attempts.

For consistency we might want to keep it?

Note the reported splat itself is on q_usage_counter(queue), via
blk_queue_enter() from dm-mpath's blk_mq_alloc_request(BLK_MQ_REQ_NOWAIT).
That path has no inline fast path so the fix should fix the report.