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
