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

[James Bottomley <jejb@linux.ibm.com>]

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