On Tue, 2014-05-27 at 10:06 +0200, Bart Van Assche wrote: > On 05/26/14 17:23, Paolo Bonzini wrote: > > Il 26/05/2014 17:14, Bart Van Assche ha scritto: > >> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c > >> index 88d46fe..c972eab 100644 > >> --- a/drivers/scsi/scsi.c > >> +++ b/drivers/scsi/scsi.c > >> @@ -334,7 +334,7 @@ void scsi_put_command(struct scsi_cmnd *cmd) > >> list_del_init(&cmd->list); > >> spin_unlock_irqrestore(&cmd->device->list_lock, flags); > >> > >> - cancel_delayed_work(&cmd->abort_work); > >> + WARN_ON_ONCE(delayed_work_pending(&cmd->abort_work)); > >> > >> __scsi_put_command(cmd->device->host, cmd); > >> } > >> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c > >> index f17aa7a..5232583 100644 > >> --- a/drivers/scsi/scsi_error.c > >> +++ b/drivers/scsi/scsi_error.c > >> @@ -193,7 +193,7 @@ scsi_abort_command(struct scsi_cmnd *scmd) > >> SCSI_LOG_ERROR_RECOVERY(3, > >> scmd_printk(KERN_INFO, scmd, > >> "scmd %p previous abort failed\n", scmd)); > >> - cancel_delayed_work(&scmd->abort_work); > >> + WARN_ON_ONCE(delayed_work_pending(&scmd->abort_work)); > >> return FAILED; > >> } > >> > >> > > > > I still prefer a BUG_ON in scsi_put_command, but anyway: series > > > > Reviewed-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> > > Hello Paolo, > > As you probably know scsi_put_command() can get called from softirq > context. A BUG_ON() in that context might make it unnecessary hard for a > user to collect call traces. Why? The messages dumped are the same, the trace just starts from the IRQ context ... I don't see what the problem is. The question isn't ease of gathering the data, it's correctness. The point is that if the assert fails we have a free of an in-use command leading to a nasty use after free ... the machine state is hosed at that point. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
