On 22/08/2023 16:25, Bart Van Assche wrote:
On 8/22/23 02:04, John Garry wrote:
If it's on the first list, then there is not much point in checking
this list.
It might be even worth checking list_empty(&cmd->eh_entry) initially
also to
save the bother.
Having said all this, adding those checks will add lots of unpleasant
indentation...
Since the above code is only used to export information through debugfs
I don't
think that it should be optimized heavily?
To me it's not really a matter of having the code optimized, but more
about having clear and efficient code.
Anyway, how about combining the
(untested) patch below with the above patch?
diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
index a9bc5f7ce745..f795848b316c 100644
--- a/drivers/scsi/scsi_debugfs.c
+++ b/drivers/scsi/scsi_debugfs.c
@@ -45,15 +45,16 @@ void scsi_show_rq(struct seq_file *m, struct request
*rq)
list_for_each_entry(cmd2, &shost->eh_abort_list, eh_entry) {
if (cmd == cmd2) {
list_info = "on eh_abort_list";
- break;
+ goto unlock;
}
}
list_for_each_entry(cmd2, &shost->eh_cmd_q, eh_entry) {
if (cmd == cmd2) {
list_info = "on eh_cmd_q";
- break;
+ goto unlock;
}
}
I think that list_info could be set to NULL here, rather than when declared.
+unlock:
spin_unlock_irq(shost->host_lock);
__scsi_format_command(buf, sizeof(buf), cmd->cmnd, cmd->cmd_len);
Regardless of comment, above:
Reviewed-by: John Garry <john.g.garry@xxxxxxxxxx>
thanks
