calling context of scsi_end_request() always hard IRQ or sometimes different?

calling context of scsi_end_request() always hard IRQ or sometimes different?

[Jason A. Donenfeld]

Hey James, Martin,

I'm in the process of fixing a few issues with the RNG and one thing
that surprised me is that scsi_end_request() appears to be called from
hard IRQ context rather than some worker or soft IRQ as I assumed it
would be. That's fine, and I can deal with it, but what I haven't yet
been able to figure out is whether it's _always_ called from hard IRQ,
or whether it's sometimes from hard IRQ and sometimes not, and so I
should handle both cases in the thing I'm working on?

And if the answer turns out to be, "I don't know; that's really
complicated and..." just say so, and I'll just try to work out the whole
function graph.

Jason

Re: calling context of scsi_end_request() always hard IRQ or sometimes different?

[James Bottomley]

On Fri, 2022-05-06 at 18:57 +0200, Jason A. Donenfeld wrote:
> Hey James, Martin,
> 
> I'm in the process of fixing a few issues with the RNG and one thing
> that surprised me is that scsi_end_request() appears to be called
> from hard IRQ context rather than some worker or soft IRQ as I
> assumed it would be. That's fine, and I can deal with it, but what I
> haven't yet been able to figure out is whether it's _always_ called
> from hard IRQ, or whether it's sometimes from hard IRQ and sometimes
> not, and so I should handle both cases in the thing I'm working on?
> 
> And if the answer turns out to be, "I don't know; that's really
> complicated and..." just say so, and I'll just try to work out the
> whole function graph.

Are you sure you mean scsi_end_request()?  It's static to scsi_lib.c so
its call graph is tiny  it basically goes from the blk-mq complete
function (softirq) through scsi_complete->scsi_finish_command-
>scsi_io_completion->scsi_end_request

However, I didn't think it was ever called from hard IRQ context,
that's usually scsi_done() (which can also be called from other
contexts).

James

Re: calling context of scsi_end_request() always hard IRQ or sometimes different?

[Jason A. Donenfeld]

Hi James,

On Fri, May 06, 2022 at 02:19:43PM -0400, James Bottomley wrote:
> On Fri, 2022-05-06 at 18:57 +0200, Jason A. Donenfeld wrote:
> > Hey James, Martin,
> > 
> > I'm in the process of fixing a few issues with the RNG and one thing
> > that surprised me is that scsi_end_request() appears to be called
> > from hard IRQ context rather than some worker or soft IRQ as I
> > assumed it would be. That's fine, and I can deal with it, but what I
> > haven't yet been able to figure out is whether it's _always_ called
> > from hard IRQ, or whether it's sometimes from hard IRQ and sometimes
> > not, and so I should handle both cases in the thing I'm working on?
> > 
> > And if the answer turns out to be, "I don't know; that's really
> > complicated and..." just say so, and I'll just try to work out the
> > whole function graph.
> 
> Are you sure you mean scsi_end_request()?  It's static to scsi_lib.c so
> its call graph is tiny  it basically goes from the blk-mq complete
> function (softirq) through scsi_complete->scsi_finish_command-
> >scsi_io_completion->scsi_end_request
> 
> However, I didn't think it was ever called from hard IRQ context,
> that's usually scsi_done() (which can also be called from other
> contexts).

Really what I'm interested in is add_disk_randomness(), and the only
caller of that is scsi_end_request(), so I think my question is the
right one.

Interestingly, I _am_ seeing it from hardirq context (if
`in_interrupt()` is to be believed):

[    2.108954]  add_timer_randomness.cold+0x5/0x3a
[    2.110514]  scsi_end_request+0x136/0x1a0
[    2.111903]  scsi_io_completion+0x2e/0x710
[    2.113314]  virtscsi_req_done+0x59/0xa0
[    2.114705]  vring_interrupt+0x46/0x70
[    2.116002]  __handle_irq_event_percpu+0x32/0xb0
[    2.117591]  handle_irq_event+0x2f/0x70
[    2.118929]  handle_edge_irq+0x7c/0x210
[    2.120249]  __common_interrupt+0x33/0x90
[    2.121641]  common_interrupt+0x7b/0xa0
 
And it sounds like you're saying that this is really a softirq function.
So is it correct for me to conclude that the right answer here is that
it can be called from both/multiple contexts, and that's fine and
normal?

Jason

Re: calling context of scsi_end_request() always hard IRQ or sometimes different?

[James Bottomley]

On Fri, 2022-05-06 at 21:22 +0200, Jason A. Donenfeld wrote:
> Hi James,
> 
> On Fri, May 06, 2022 at 02:19:43PM -0400, James Bottomley wrote:
> > On Fri, 2022-05-06 at 18:57 +0200, Jason A. Donenfeld wrote:
> > > Hey James, Martin,
> > > 
> > > I'm in the process of fixing a few issues with the RNG and one
> > > thing
> > > that surprised me is that scsi_end_request() appears to be called
> > > from hard IRQ context rather than some worker or soft IRQ as I
> > > assumed it would be. That's fine, and I can deal with it, but
> > > what I
> > > haven't yet been able to figure out is whether it's _always_
> > > called
> > > from hard IRQ, or whether it's sometimes from hard IRQ and
> > > sometimes
> > > not, and so I should handle both cases in the thing I'm working
> > > on?
> > > 
> > > And if the answer turns out to be, "I don't know; that's really
> > > complicated and..." just say so, and I'll just try to work out
> > > the
> > > whole function graph.
> > 
> > Are you sure you mean scsi_end_request()?  It's static to
> > scsi_lib.c so
> > its call graph is tiny  it basically goes from the blk-mq complete
> > function (softirq) through scsi_complete->scsi_finish_command-
> > > scsi_io_completion->scsi_end_request
> > 
> > However, I didn't think it was ever called from hard IRQ context,
> > that's usually scsi_done() (which can also be called from other
> > contexts).
> 
> Really what I'm interested in is add_disk_randomness(), and the only
> caller of that is scsi_end_request(), so I think my question is the
> right one.
> 
> Interestingly, I _am_ seeing it from hardirq context (if
> `in_interrupt()` is to be believed):
> 
> [    2.108954]  add_timer_randomness.cold+0x5/0x3a
> [    2.110514]  scsi_end_request+0x136/0x1a0
> [    2.111903]  scsi_io_completion+0x2e/0x710

The call trace looks broken here.  After virtscsi_req_done it should
invoke scsi_done and blk_mq_complete_req, which usually goes via the
block softirq (but which may complete in the hardirq under some
circumstances), before it gets back into scsi_io_completion.

> [    2.113314]  virtscsi_req_done+0x59/0xa0
> [    2.114705]  vring_interrupt+0x46/0x70
> [    2.116002]  __handle_irq_event_percpu+0x32/0xb0
> [    2.117591]  handle_irq_event+0x2f/0x70
> [    2.118929]  handle_edge_irq+0x7c/0x210
> [    2.120249]  __common_interrupt+0x33/0x90
> [    2.121641]  common_interrupt+0x7b/0xa0
>  
> And it sounds like you're saying that this is really a softirq
> function. So is it correct for me to conclude that the right answer
> here is that it can be called from both/multiple contexts, and that's
> fine and normal?

Pretty much, yes.

James