> -----Original Message----- > From: FUJITA Tomonori [mailto:fujita.tomonori@xxxxxxxxxxxxx] > Sent: Thursday, June 14, 2007 3:21 PM > To: Ed Lin > Cc: bharrosh@xxxxxxxxxxx; fujita.tomonori@xxxxxxxxxxxxx; > g.liakhovetski@xxxxxx; linux-scsi@xxxxxxxxxxxxxxx > Subject: RE: [PATCH 3/3] Farther clean up of stex.c driver > > > From: "Ed Lin" <ed.lin@xxxxxxxxxxx> > Subject: RE: [PATCH 3/3] Farther clean up of stex.c driver > Date: Thu, 14 Jun 2007 12:29:05 -0700 > > > > > > > > -----Original Message----- > > > From: Boaz Harrosh [mailto:bharrosh@xxxxxxxxxxx] > > > Sent: Thursday, June 14, 2007 9:34 AM > > > To: FUJITA Tomonori; Ed Lin; Guennadi Liakhovetski > > > Cc: linux-scsi@xxxxxxxxxxxxxxx > > > Subject: Re: [PATCH 3/3] Farther clean up of stex.c driver > > > > > > > > > From 2b82909202cab8dc35184daef45b4b388f93112a Mon Sep 17 > 00:00:00 2001 > > > From: Boaz Harrosh <bharrosh@bh-buildlin2.(none)> > > > Date: Thu, 14 Jun 2007 19:14:40 +0300 > > > Subject: [PATCH] Farther clean up of stex.c driver > > > > > > - now that scsi-ml accessors do not allow modifying of > > > sg_count bufflen and > > > sglist. The code in question is open coded to directly > > > hack the scsi_cmnd > > > structure. When implementation changes the driver will > > > need to change with it. > > > > > > - Maintainer of this driver should please review again if we > > > absolutely need this > > > read-sense length fixing. What if less bytes are read and > > > 0 are left at the end. > > > Also no check is made if the buffer is large enough. > > > --- > > > drivers/scsi/stex.c | 2 +- > > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > > > diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c > > > index adda296..d784b58 100644 > > > --- a/drivers/scsi/stex.c > > > +++ b/drivers/scsi/stex.c > > > @@ -719,7 +719,7 @@ static void stex_ys_commands(struct > st_hba *hba, > > > > > > if (ccb->cmd->cmnd[0] == MGT_CMD && > > > resp->scsi_status != SAM_STAT_CHECK_CONDITION) { > > > - scsi_bufflen(ccb->cmd) = > > > + ccb->cmd->request_bufflen = > > > le32_to_cpu(*(__le32 *)&resp->variable[0]); > > > return; > > > } > > > -- > > > 1.5.0.4.402.g8035 > > > > > > > > > > > > > The software allocates a big enough buffer(so don't worry > about this), > > and it trims the data size based on returned length. So it needs the > > actual data length. > > What's the software? I mean that who trims the data size based on > returned length? > The management software provided by Promise. > > So this length fix is necessary if software doesn't > > change policy. The whole thing is guaranteed by both software and > > firmware, software will do a check, firmware will do a > check, so a check > > in driver is not necessary. > > > > I wonder whether the following will go upstream: > > #define scsi_sg_count(cmd) ((cmd)->sg_table ? > (cmd)->sg_table->sg_count > > : 0) > > #define scsi_sglist(cmd) ((cmd)->sg_table ? > (cmd)->sg_table->sglist : > > NULL) > > #define scsi_bufflen(cmd) ((cmd)->sg_table ? > (cmd)->sg_table->length : > > 0) > > > > For this particular case, because there is data exchange, so > > (cmd)->sg_table is not NULL, so we can have something like: > > ccb->cmd->sg_table->length = > > le32_to_cpu(*(__le32 *)&resp->variable[0]); > > Although I am not sure whether the software can work without change > > after the scsi_sgtable approach becomes upstream. But I > just wonder why > > we use some code that is known to be temporary even before its > > introduction. > > I think that it would be better to apply Boaz patchset with scsi-bidi > support (not now). Then you can avoid adding temporary hack. > Does the scsi_sgtable above belong to the scsi-bidi patchset? If it does, then this patch is just a temporary hack. The code needs to be changed again at that time. This is stated in the comment of this patch. --Ed Lin - 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
