Re: [PATCH scsi] Short the path length of scsi_cmd_to_driver()

2012-09-28 Thread Li Zhong 
On Thu, 2012-09-27 at 13:43 -0400, Martin K. Petersen wrote:
  Li == Li Zhong zh...@linux.vnet.ibm.com writes:
 
  @@ -845,8 +844,11 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, 
  unsigned char *cmnd,
  
  scsi_eh_restore_cmnd(scmd, ses);
  
 -if (sdrv  sdrv-eh_action)
 -rtn = sdrv-eh_action(scmd, cmnd, cmnd_size, rtn);
 +if (scmd-request-cmd_type == REQ_TYPE_FS) {
 +struct scsi_driver *sdrv = scsi_cmd_to_driver(scmd);
 +if (sdrv-eh_action)
 +rtn = sdrv-eh_action(scmd, cmnd, cmnd_size, rtn);
 +}
  
  return rtn;
  }
 
 My only concern is whether our device lifetime rules guarantee that the
 ULD is always attached when we service an error handling command?

Thank you, Martin, for the review. 

I don't know much about scsi, it might take me some more time to have an
answer to the above question. 

For now, if I understand correctly, maybe we could only do the
not-consistent behaviours bug fix?  Or could we provide two versions of
scsi_cmd_to_driver(), one with NULL checking for scsi_send_eh_cmnd(),
one without the checking for scsi_finish_command()? 

Thanks, Zhong

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH scsi] Short the path length of scsi_cmd_to_driver()

2012-09-28 Thread James Bottomley 
On Thu, 2012-09-27 at 13:43 -0400, Martin K. Petersen wrote:
  Li == Li Zhong zh...@linux.vnet.ibm.com writes:
 
  @@ -845,8 +844,11 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, 
  unsigned char *cmnd,
  
  scsi_eh_restore_cmnd(scmd, ses);
  
 -if (sdrv  sdrv-eh_action)
 -rtn = sdrv-eh_action(scmd, cmnd, cmnd_size, rtn);
 +if (scmd-request-cmd_type == REQ_TYPE_FS) {
 +struct scsi_driver *sdrv = scsi_cmd_to_driver(scmd);
 +if (sdrv-eh_action)
 +rtn = sdrv-eh_action(scmd, cmnd, cmnd_size, rtn);
 +}
  
  return rtn;
  }
 
 My only concern is whether our device lifetime rules guarantee that the
 ULD is always attached when we service an error handling command?

Yes, they are.  We can only get REQ_TYPE_FS through a filesystem, which
must be mounted on a block device, which is provided by the ULD.  You
can't unmount with outstanding I/O (which people complain about when it
goes into error handling, I'll admit), so the ULD has to stay bound.

James



--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH scsi] Short the path length of scsi_cmd_to_driver()

2012-09-27 Thread Martin K. Petersen 
 Li == Li Zhong zh...@linux.vnet.ibm.com writes:

 @@ -845,8 +844,11 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, 
 unsigned char *cmnd,
 
   scsi_eh_restore_cmnd(scmd, ses);
 
-  if (sdrv  sdrv-eh_action)
-  rtn = sdrv-eh_action(scmd, cmnd, cmnd_size, rtn);
+  if (scmd-request-cmd_type == REQ_TYPE_FS) {
+  struct scsi_driver *sdrv = scsi_cmd_to_driver(scmd);
+  if (sdrv-eh_action)
+  rtn = sdrv-eh_action(scmd, cmnd, cmnd_size, rtn);
+  }
 
   return rtn;
 }

My only concern is whether our device lifetime rules guarantee that the
ULD is always attached when we service an error handling command?

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html