Adding Cc: LSML
On Jul 23 Jia-Ju Bai wrote:
sbp2_scsi_queuecommand() is only set to .queuecommand ofAs far as I remember, scsi_host_template::queuecommand() can be invoked
"struct scsi_host_template", and this function pointer is never called
in atomic context.
from either process context or tasklet context, predominantly the latter.
I haven't followed recent developments of the block and scsi stack, hence
don't know if this has changed fundamentally.
But even if it is purely process context now and no spinlocks held, the
memory allocation must be done so that the kernel does not go into memory
reclaim. Otherwise this could deadlock.
sbp2_scsi_queuecommand() calls kzalloc() with GFP_ATOMIC,I doubt that static analysis, even if very sophisticated, can detect
which is not necessary.
GFP_ATOMIC can be replaced with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
deadlock scenarios such as I noted.
I also manually check the kernel code before reporting it.What does it mean? Did you run-time test it, for which actual SBP-2
hardware is required? (Such a test could detect GFP-KERNEL use in atomic
context, but would not reliably detect memory reclaim related deadlocks.)
Signed-off-by: Jia-Ju Bai <baijiaju1990@xxxxxxxxx>NACK for GFP_KERNEL, but I am curious whether a weaker GFP set than ATOMIC
---
drivers/firewire/sbp2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c
index 6bac03999fd4..a7cd9d87eb02 100644
--- a/drivers/firewire/sbp2.c
+++ b/drivers/firewire/sbp2.c
@@ -1463,7 +1463,7 @@ static int sbp2_scsi_queuecommand(struct Scsi_Host *shost,
struct sbp2_command_orb *orb;
int generation, retval = SCSI_MLQUEUE_HOST_BUSY;
- orb = kzalloc(sizeof(*orb), GFP_ATOMIC);
+ orb = kzalloc(sizeof(*orb), GFP_KERNEL);
if (orb == NULL)
return SCSI_MLQUEUE_HOST_BUSY;
is possible in scsi_host_template::queuecommand.