bi_end_io() is called with lock held: bug or feature?

bi_end_io() is called with lock held: bug or feature?

[Maxim Patlasov]

Hi Jens,

While experimenting with some third-party driver, I noticed that
sometimes (at least for empty flush requests) bi_end_io callback is
called with IRQs locally disabled. The cause turned out to be in
blk_end_bidi_request():

>	spin_lock_irqsave(q->queue_lock, flags);
>	blk_finish_request(rq, error);
>	spin_unlock_irqrestore(q->queue_lock, flags);

A relevant part of stack trace is:

> bio_endio+0x18/0x30
> dec_pending+0x172/0x2b0
> clone_endio+0x99/0xd0
> bio_endio+0x18/0x30
> req_bio_endio+0x83/0xc0
> blk_update_request+0xff/0x470
> blk_update_bidi_request+0x22/0xa0
> __blk_end_bidi_request+0x1c/0x40
> __blk_end_request_all+0x1a/0x30
> blk_flush_complete_seq+0x258/0x280
> flush_end_io+0xe5/0x170
> blk_finish_request+0x86/0x280
> blk_end_bidi_request+0x4f/0x80

At first glance it looks a bit weird that in most cases bi_end_io
callback is called w/o any locks held but under some conditions (e.g.
completing empty REQ_FLUSH bio) - with q->queue_lock acquired. Is it a
bug or expected behaviour?

Thanks in advance,
Maxim

Re: bi_end_io() is called with lock held: bug or feature?

[Jens Axboe]

On 2011-07-04 17:56, Maxim Patlasov wrote:
> Hi Jens,
> 
> While experimenting with some third-party driver, I noticed that
> sometimes (at least for empty flush requests) bi_end_io callback is
> called with IRQs locally disabled. The cause turned out to be in
> blk_end_bidi_request():
> 
>> 	spin_lock_irqsave(q->queue_lock, flags);
>> 	blk_finish_request(rq, error);
>> 	spin_unlock_irqrestore(q->queue_lock, flags);
> 
> A relevant part of stack trace is:
> 
>> bio_endio+0x18/0x30
>> dec_pending+0x172/0x2b0
>> clone_endio+0x99/0xd0
>> bio_endio+0x18/0x30
>> req_bio_endio+0x83/0xc0
>> blk_update_request+0xff/0x470
>> blk_update_bidi_request+0x22/0xa0
>> __blk_end_bidi_request+0x1c/0x40
>> __blk_end_request_all+0x1a/0x30
>> blk_flush_complete_seq+0x258/0x280
>> flush_end_io+0xe5/0x170
>> blk_finish_request+0x86/0x280
>> blk_end_bidi_request+0x4f/0x80
> 
> At first glance it looks a bit weird that in most cases bi_end_io
> callback is called w/o any locks held but under some conditions (e.g.
> completing empty REQ_FLUSH bio) - with q->queue_lock acquired. Is it a
> bug or expected behaviour?

There's generally not any guarentees as to whether bi_end_io is called
with the queue lock held (and ints disabled) or not. It largely depends
on the driver.

IOW, you cannot rely on process context. That is also why eg
bio_set_pages_dirty() is punted to a work queue.

-- 
Jens Axboe