* [PATCH 3/4] qla1280,qla2xxx: Remove display of transfersize
2007-10-18 8:46 [PATCH 1/4] scsi_cmnd: Rearrange and shrink some elements Matthew Wilcox
@ 2007-10-18 8:46 ` Matthew Wilcox
0 siblings, 0 replies; 10+ messages in thread
From: Matthew Wilcox @ 2007-10-18 8:46 UTC (permalink / raw)
To: linux-scsi; +Cc: Matthew Wilcox, Matthew Wilcox
As part of their debug routines, both of these drivers print out the
transfersize. That's just unnecessary, particularly since neither driver
actually uses it.
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
drivers/scsi/qla1280.c | 4 +---
drivers/scsi/qla2xxx/qla_dbg.c | 3 +--
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
index bf07b17..d319ae8 100644
--- a/drivers/scsi/qla1280.c
+++ b/drivers/scsi/qla1280.c
@@ -4025,11 +4025,9 @@ __qla1280_print_scsi_cmd(struct scsi_cmnd *cmd)
for (i = 0; i < cmd->cmd_len; i++) {
printk("0x%02x ", cmd->cmnd[i]);
}
- printk(" seg_cnt =%d\n", scsi_sg_count(cmd));
+ printk(" seg_cnt=%d, tag=%d\n", scsi_sg_count(cmd), cmd->tag);
printk(" request buffer=0x%p, request buffer len=0x%x\n",
scsi_sglist(cmd), scsi_bufflen(cmd));
- printk(" tag=%d, transfersize=0x%x \n",
- cmd->tag, cmd->transfersize);
printk(" Pid=%li, SP=0x%p\n", cmd->serial_number, CMD_SP(cmd));
printk(" underflow size = 0x%x, direction=0x%x\n",
cmd->underflow, cmd->sc_data_direction);
diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index eaa04da..11b437e 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -1417,8 +1417,7 @@ qla2x00_print_scsi_cmd(struct scsi_cmnd * cmd)
scsi_sg_count(cmd), cmd->allowed, cmd->retries);
printk(" request buffer=0x%p, request buffer len=0x%x\n",
scsi_sglist(cmd), scsi_bufflen(cmd));
- printk(" tag=%d, transfersize=0x%x\n",
- cmd->tag, cmd->transfersize);
+ printk(" tag=%d\n", cmd->tag);
printk(" serial_number=%lx, SP=%p\n", cmd->serial_number, sp);
printk(" data direction=%d\n", cmd->sc_data_direction);
--
1.4.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/4] scsi_cmnd: Rearrange and shrink some elements
@ 2007-10-18 16:27 Matthew Wilcox
2007-10-18 16:27 ` [PATCH 2/4] shuttle_usbat: Eliminate use of transfersize Matthew Wilcox
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Matthew Wilcox @ 2007-10-18 16:27 UTC (permalink / raw)
To: linux-scsi; +Cc: Matthew Wilcox, Matthew Wilcox
- rearrange the elements of the scsi_pointer
- shrink and move the eh_eflags element
- turn sc_data_direction into an unsigned char
- move tag
By doing this, we reduce the size of scsi_cmnd from 376 to 352 bytes
on x86-64 and from 272 to 260 bytes on i386.
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
include/scsi/scsi_cmnd.h | 17 +++++++----------
1 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 8b8759c..b9c7846 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -13,22 +13,21 @@ struct Scsi_Host;
struct scsi_device;
struct scsi_data_buffer {
+ struct scatterlist* sglist;
unsigned length;
int resid;
unsigned short sg_count;
unsigned short alloc_sg_count;
- struct scatterlist* sglist;
};
/* embedded in scsi_cmnd */
struct scsi_pointer {
char *ptr; /* data pointer */
- int this_residual; /* left in this buffer */
struct scatterlist *buffer; /* which buffer */
+ dma_addr_t dma_handle;
+ int this_residual; /* left in this buffer */
int buffers_residual; /* how many buffers left */
- dma_addr_t dma_handle;
-
volatile int Status;
volatile int Message;
volatile int have_data_in;
@@ -40,7 +39,6 @@ struct scsi_cmnd {
struct scsi_device *device;
struct list_head list; /* scsi_cmnd participates in queue lists */
struct list_head eh_entry; /* entry for the host eh_cmd_q */
- int eh_eflags; /* Used by error handlr */
/*
* A SCSI Command is assigned a nonzero serial_number before passed
@@ -64,7 +62,9 @@ struct scsi_cmnd {
int timeout_per_command;
unsigned char cmd_len;
- enum dma_data_direction sc_data_direction;
+ unsigned char eh_eflags; /* Used by error handler */
+ unsigned char sc_data_direction; /* enum dma_data_direction */
+ unsigned char tag; /* SCSI-II queued command tag */
/* These elements define the operation we are about to perform */
#define MAX_COMMAND_SIZE 16
@@ -82,8 +82,7 @@ struct scsi_cmnd {
reconnects. Probably == sector
size */
- struct request *request; /* The command we are
- working on */
+ struct request *request; /* The command we are working on */
#define SCSI_SENSE_BUFFERSIZE 96
unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE];
@@ -111,8 +110,6 @@ struct scsi_cmnd {
int result; /* Status code from lower level driver */
- unsigned char tag; /* SCSI-II queued command tag */
-
struct scsi_data_buffer sdb;
};
--
1.4.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] shuttle_usbat: Eliminate use of transfersize
2007-10-18 16:27 [PATCH 1/4] scsi_cmnd: Rearrange and shrink some elements Matthew Wilcox
@ 2007-10-18 16:27 ` Matthew Wilcox
2007-10-18 16:27 ` [PATCH 3/4] qla1280,qla2xxx: Remove display " Matthew Wilcox
2007-10-18 16:27 ` [PATCH 4/4] Replace scmd->transfersize with scsi_transfer_size() Matthew Wilcox
2 siblings, 0 replies; 10+ messages in thread
From: Matthew Wilcox @ 2007-10-18 16:27 UTC (permalink / raw)
To: linux-scsi; +Cc: Matthew Wilcox, Daniel Drake, Matthew Wilcox
It's more intuitive to go straight to the struct device for the
sector_size, when that's what we want, rather than using transfersize
as a replacement.
Also fix a bug in handling of GPCMD_READ_CD where we were pulling the
wrong bytes out of the packet to calculate the length. I checked this
against MMC5r04.
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
drivers/usb/storage/shuttle_usbat.c | 46 +++++++++++++++++++---------------
1 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/storage/shuttle_usbat.c b/drivers/usb/storage/shuttle_usbat.c
index 570c125..a172e98 100644
--- a/drivers/usb/storage/shuttle_usbat.c
+++ b/drivers/usb/storage/shuttle_usbat.c
@@ -49,6 +49,7 @@
#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_device.h>
#include "usb.h"
#include "transport.h"
@@ -1164,12 +1165,10 @@ static int usbat_hp8200e_handle_read10(struct us_data *us,
unsigned char *buffer;
unsigned int len;
unsigned int sector;
+ unsigned int sector_size;
unsigned int sg_offset = 0;
struct scatterlist *sg = NULL;
- US_DEBUGP("handle_read10: transfersize %d\n",
- srb->transfersize);
-
if (scsi_bufflen(srb) < 0x10000) {
result = usbat_hp8200e_rw_block_test(us, USBAT_ATA,
@@ -1184,25 +1183,32 @@ static int usbat_hp8200e_handle_read10(struct us_data *us,
}
/*
- * Since we're requesting more data than we can handle in
- * a single read command (max is 64k-1), we will perform
- * multiple reads, but each read must be in multiples of
- * a sector. Luckily the sector size is in srb->transfersize
- * (see linux/drivers/scsi/sr.c).
+ * Since we're requesting more data than we can handle in a
+ * single read command (max is 64k-1), we will perform multiple
+ * reads, but each read must be in multiples of a sector.
+ *
+ * For ordinary READ_10 commands, we're reading at the current
+ * sector size, so use the device's sector size. If the command
+ * is a READ_CD, we might be reading different sector sizes.
+ * Fortunately, we can calculate the sector size from the
+ * contents of the command.
*/
if (data[7+0] == GPCMD_READ_CD) {
- len = short_pack(data[7+9], data[7+8]);
- len <<= 16;
- len |= data[7+7];
+ len = data[7+8] | ((u32)data[7+7] << 8) |
+ ((u32)data[7+6] << 16);
US_DEBUGP("handle_read10: GPCMD_READ_CD: len %d\n", len);
- srb->transfersize = scsi_bufflen(srb)/len;
+ sector_size = scsi_bufflen(srb) / len;
+ } else {
+ sector_size = srb->device->sector_size;
}
- if (!srb->transfersize) {
- srb->transfersize = 2048; /* A guess */
- US_DEBUGP("handle_read10: transfersize 0, forcing %d\n",
- srb->transfersize);
+ US_DEBUGP("handle_read10: sector_size %d\n", sector_size);
+
+ if (!sector_size) {
+ sector_size = 2048; /* A guess */
+ US_DEBUGP("handle_read10: sector_size 0, forcing %d\n",
+ sector_size);
}
/*
@@ -1211,7 +1217,7 @@ static int usbat_hp8200e_handle_read10(struct us_data *us,
* bounce buffer and the actual transfer buffer.
*/
- len = (65535/srb->transfersize) * srb->transfersize;
+ len = (65535/sector_size) * sector_size;
US_DEBUGP("Max read is %d bytes\n", len);
len = min(len, scsi_bufflen(srb));
buffer = kmalloc(len, GFP_NOIO);
@@ -1238,8 +1244,8 @@ static int usbat_hp8200e_handle_read10(struct us_data *us,
data[7+5] = LSB_of(sector&0xFFFF);
if (data[7+0] == GPCMD_READ_CD)
data[7+6] = 0;
- data[7+7] = MSB_of(len / srb->transfersize); /* SCSI command */
- data[7+8] = LSB_of(len / srb->transfersize); /* num sectors */
+ data[7+7] = MSB_of(len / sector_size); /* SCSI command */
+ data[7+8] = LSB_of(len / sector_size); /* num sectors */
result = usbat_hp8200e_rw_block_test(us, USBAT_ATA,
registers, data, 19,
@@ -1259,7 +1265,7 @@ static int usbat_hp8200e_handle_read10(struct us_data *us,
/* Update the amount transferred and the sector number */
transferred += len;
- sector += len / srb->transfersize;
+ sector += len / sector_size;
} /* while transferred != scsi_bufflen(srb) */
--
1.4.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] qla1280,qla2xxx: Remove display of transfersize
2007-10-18 16:27 [PATCH 1/4] scsi_cmnd: Rearrange and shrink some elements Matthew Wilcox
2007-10-18 16:27 ` [PATCH 2/4] shuttle_usbat: Eliminate use of transfersize Matthew Wilcox
@ 2007-10-18 16:27 ` Matthew Wilcox
2007-10-18 18:57 ` Seokmann Ju
2007-10-18 16:27 ` [PATCH 4/4] Replace scmd->transfersize with scsi_transfer_size() Matthew Wilcox
2 siblings, 1 reply; 10+ messages in thread
From: Matthew Wilcox @ 2007-10-18 16:27 UTC (permalink / raw)
To: linux-scsi; +Cc: Matthew Wilcox, Matthew Wilcox
As part of their debug routines, both of these drivers print out the
transfersize. That's just unnecessary, particularly since neither driver
actually uses it.
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
drivers/scsi/qla1280.c | 4 +---
drivers/scsi/qla2xxx/qla_dbg.c | 3 +--
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
index bf07b17..d319ae8 100644
--- a/drivers/scsi/qla1280.c
+++ b/drivers/scsi/qla1280.c
@@ -4025,11 +4025,9 @@ __qla1280_print_scsi_cmd(struct scsi_cmnd *cmd)
for (i = 0; i < cmd->cmd_len; i++) {
printk("0x%02x ", cmd->cmnd[i]);
}
- printk(" seg_cnt =%d\n", scsi_sg_count(cmd));
+ printk(" seg_cnt=%d, tag=%d\n", scsi_sg_count(cmd), cmd->tag);
printk(" request buffer=0x%p, request buffer len=0x%x\n",
scsi_sglist(cmd), scsi_bufflen(cmd));
- printk(" tag=%d, transfersize=0x%x \n",
- cmd->tag, cmd->transfersize);
printk(" Pid=%li, SP=0x%p\n", cmd->serial_number, CMD_SP(cmd));
printk(" underflow size = 0x%x, direction=0x%x\n",
cmd->underflow, cmd->sc_data_direction);
diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index eaa04da..11b437e 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -1417,8 +1417,7 @@ qla2x00_print_scsi_cmd(struct scsi_cmnd * cmd)
scsi_sg_count(cmd), cmd->allowed, cmd->retries);
printk(" request buffer=0x%p, request buffer len=0x%x\n",
scsi_sglist(cmd), scsi_bufflen(cmd));
- printk(" tag=%d, transfersize=0x%x\n",
- cmd->tag, cmd->transfersize);
+ printk(" tag=%d\n", cmd->tag);
printk(" serial_number=%lx, SP=%p\n", cmd->serial_number, sp);
printk(" data direction=%d\n", cmd->sc_data_direction);
--
1.4.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] Replace scmd->transfersize with scsi_transfer_size()
2007-10-18 16:27 [PATCH 1/4] scsi_cmnd: Rearrange and shrink some elements Matthew Wilcox
2007-10-18 16:27 ` [PATCH 2/4] shuttle_usbat: Eliminate use of transfersize Matthew Wilcox
2007-10-18 16:27 ` [PATCH 3/4] qla1280,qla2xxx: Remove display " Matthew Wilcox
@ 2007-10-18 16:27 ` Matthew Wilcox
2007-10-19 17:45 ` Stefan Richter
2 siblings, 1 reply; 10+ messages in thread
From: Matthew Wilcox @ 2007-10-18 16:27 UTC (permalink / raw)
To: linux-scsi; +Cc: Matthew Wilcox, Matthew Wilcox
Only two drivers remain that use scmd->transfersize, but they both
look like legitimate uses. So add a new function that returns what
that element would have been, and we can remove the element. Shuffling
'result' into its place saves us 8 bytes on x86-64 (down to 344 bytes)
and 4 bytes on i386 (down to 256).
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
drivers/scsi/NCR5380.c | 2 +-
drivers/scsi/arm/fas216.c | 2 +-
drivers/scsi/scsi_lib.c | 16 +++++++++++++++-
drivers/scsi/sd.c | 6 ------
drivers/scsi/sr.c | 6 ------
include/scsi/scsi_cmnd.h | 11 +++--------
6 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 47ee320..84642bf 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -2168,7 +2168,7 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) {
#ifdef NCR5380_dma_xfer_len
if (!cmd->device->borken && !(hostdata->flags & FLAG_NO_PSEUDO_DMA) && (transfersize = NCR5380_dma_xfer_len(instance, cmd)) != 0) {
#else
- transfersize = cmd->transfersize;
+ transfersize = scsi_transfer_size(cmd);
#ifdef LIMIT_TRANSFERSIZE /* If we have problems with interrupt service */
if (transfersize > 512)
diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index a715632..5fc3646 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -801,7 +801,7 @@ static void fas216_transfer(FAS216_Info *info)
fas216_log(info, LOG_BUFFER, "pseudo transfer");
fas216_cmd(info, CMD_TRANSFERINFO | CMD_WITHDMA);
info->dma.pseudo(info->host, &info->scsi.SCp,
- direction, info->SCpnt->transfersize);
+ direction, scsi_transfer_size(info->SCpnt));
break;
case fasdma_real_block:
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ce59cfe..d14baa1 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1189,7 +1189,6 @@ int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
else
cmd->sc_data_direction = DMA_FROM_DEVICE;
- cmd->transfersize = req->data_len;
cmd->allowed = req->retries;
cmd->timeout_per_command = req->timeout;
return BLKPREP_OK;
@@ -2355,3 +2354,18 @@ void scsi_kunmap_atomic_sg(void *virt)
kunmap_atomic(virt, KM_BIO_SRC_IRQ);
}
EXPORT_SYMBOL(scsi_kunmap_atomic_sg);
+
+/*
+ * Returns the minimum size that must be transferred in one go. For
+ * normal commands, this is the sector size of the device. For special
+ * commands, the whole command must be transferred at once. If a driver
+ * cannot transfer at least this much, it should fail the command.
+ */
+unsigned int scsi_transfer_size(struct scsi_cmnd *cmd)
+{
+ struct request *req = cmd->request;
+ if (req->cmd_type == REQ_TYPE_BLOCK_PC)
+ return req->data_len;
+ return cmd->device->sector_size;
+}
+EXPORT_SYMBOL(scsi_transfer_size);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index c241f7e..00b05f6 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -509,12 +509,6 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
}
SCpnt->sdb.length = this_count * sdp->sector_size;
- /*
- * We shouldn't disconnect in the middle of a sector, so with a dumb
- * host adapter, it's safe to assume that we can at least transfer
- * this many bytes between each connect / disconnect.
- */
- SCpnt->transfersize = sdp->sector_size;
SCpnt->underflow = this_count << 9;
SCpnt->allowed = SD_MAX_RETRIES;
SCpnt->timeout_per_command = timeout;
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 0375ad2..2e48dc0 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -412,12 +412,6 @@ static int sr_prep_fn(struct request_queue *q, struct request *rq)
SCpnt->cmnd[7] = (unsigned char) (this_count >> 8) & 0xff;
SCpnt->cmnd[8] = (unsigned char) this_count & 0xff;
- /*
- * We shouldn't disconnect in the middle of a sector, so with a dumb
- * host adapter, it's safe to assume that we can at least transfer
- * this many bytes between each connect / disconnect.
- */
- SCpnt->transfersize = cd->device->sector_size;
SCpnt->underflow = this_count << 9;
SCpnt->allowed = MAX_RETRIES;
SCpnt->timeout_per_command = timeout;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index b9c7846..30b0be0 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -72,15 +72,10 @@ struct scsi_cmnd {
struct timer_list eh_timeout; /* Used to time out the command. */
- /* These elements define the operation we ultimately want to perform */
unsigned underflow; /* Return error if less than
this amount is transferred */
- unsigned transfersize; /* How much we are guaranteed to
- transfer with each SCSI transfer
- (ie, between disconnect /
- reconnects. Probably == sector
- size */
+ int result; /* Status code from lower level driver */
struct request *request; /* The command we are working on */
@@ -108,8 +103,6 @@ struct scsi_cmnd {
* obtained by scsi_malloc is guaranteed
* to be at an address < 16Mb). */
- int result; /* Status code from lower level driver */
-
struct scsi_data_buffer sdb;
};
@@ -141,6 +134,8 @@ static inline struct scatterlist *scsi_sglist(struct scsi_cmnd *cmd)
return cmd->sdb.sglist;
}
+extern unsigned int scsi_transfer_size(struct scsi_cmnd *cmd);
+
static inline unsigned scsi_bufflen(struct scsi_cmnd *cmd)
{
return cmd->sdb.length;
--
1.4.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] qla1280,qla2xxx: Remove display of transfersize
2007-10-18 16:27 ` [PATCH 3/4] qla1280,qla2xxx: Remove display " Matthew Wilcox
@ 2007-10-18 18:57 ` Seokmann Ju
2007-10-18 20:10 ` Matthew Wilcox
0 siblings, 1 reply; 10+ messages in thread
From: Seokmann Ju @ 2007-10-18 18:57 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: linux-scsi, Matthew Wilcox
Matthew Wilcox wrote:
> As part of their debug routines, both of these drivers print out the
> transfersize. That's just unnecessary, particularly since neither driver
> actually uses it.
NACK for qla2xxx - I would rather change it to 'cmd->device->sector_size' instead of removing it.
A patch will be followed by.
Thank you,
Seokmann
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] qla1280,qla2xxx: Remove display of transfersize
2007-10-18 18:57 ` Seokmann Ju
@ 2007-10-18 20:10 ` Matthew Wilcox
2007-10-18 21:05 ` Seokmann Ju
0 siblings, 1 reply; 10+ messages in thread
From: Matthew Wilcox @ 2007-10-18 20:10 UTC (permalink / raw)
To: Seokmann Ju; +Cc: linux-scsi, Matthew Wilcox
On Thu, Oct 18, 2007 at 11:57:33AM -0700, Seokmann Ju wrote:
> Matthew Wilcox wrote:
> > As part of their debug routines, both of these drivers print out the
> > transfersize. That's just unnecessary, particularly since neither driver
> > actually uses it.
> NACK for qla2xxx - I would rather change it to 'cmd->device->sector_size' instead of removing it.
> A patch will be followed by.
I don't understand why you care to print it at this point. If you know
the device, you know the sector size, right?
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] qla1280,qla2xxx: Remove display of transfersize
2007-10-18 20:10 ` Matthew Wilcox
@ 2007-10-18 21:05 ` Seokmann Ju
2007-10-18 21:40 ` Matthew Wilcox
0 siblings, 1 reply; 10+ messages in thread
From: Seokmann Ju @ 2007-10-18 21:05 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: linux-scsi, Matthew Wilcox
Matthew Wilcox wrote:
> On Thu, Oct 18, 2007 at 11:57:33AM -0700, Seokmann Ju wrote:
>> Matthew Wilcox wrote:
>>> As part of their debug routines, both of these drivers print out the
>>> transfersize. That's just unnecessary, particularly since neither driver
>>> actually uses it.
>> NACK for qla2xxx - I would rather change it to 'cmd->device->sector_size' instead of removing it.
>> A patch will be followed by.
>
> I don't understand why you care to print it at this point. If you know
> the device, you know the sector size, right?
As the debug message doesn't get displayed in normal situation, just wanted to keep as many information in the function as possible.
Acked-by: Seokmann Ju <seokmann.ju@qlogic.com>
Thank you,
Seokmann
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] qla1280,qla2xxx: Remove display of transfersize
2007-10-18 21:05 ` Seokmann Ju
@ 2007-10-18 21:40 ` Matthew Wilcox
0 siblings, 0 replies; 10+ messages in thread
From: Matthew Wilcox @ 2007-10-18 21:40 UTC (permalink / raw)
To: Seokmann Ju; +Cc: linux-scsi, Matthew Wilcox
On Thu, Oct 18, 2007 at 02:05:21PM -0700, Seokmann Ju wrote:
> Matthew Wilcox wrote:
> > I don't understand why you care to print it at this point. If you know
> > the device, you know the sector size, right?
> As the debug message doesn't get displayed in normal situation, just wanted to keep as many information in the function as possible.
Oh -- fair enough -- I was hoping you could tell me that it helps you
track down a bug in some circumstances.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] Replace scmd->transfersize with scsi_transfer_size()
2007-10-18 16:27 ` [PATCH 4/4] Replace scmd->transfersize with scsi_transfer_size() Matthew Wilcox
@ 2007-10-19 17:45 ` Stefan Richter
0 siblings, 0 replies; 10+ messages in thread
From: Stefan Richter @ 2007-10-19 17:45 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: linux-scsi, Matthew Wilcox
On 18 Oct, Matthew Wilcox wrote:
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
...
> @@ -2355,3 +2354,18 @@ void scsi_kunmap_atomic_sg(void *virt)
> kunmap_atomic(virt, KM_BIO_SRC_IRQ);
> }
> EXPORT_SYMBOL(scsi_kunmap_atomic_sg);
> +
> +/*
> + * Returns the minimum size that must be transferred in one go. For
> + * normal commands, this is the sector size of the device. For special
> + * commands, the whole command must be transferred at once. If a driver
> + * cannot transfer at least this much, it should fail the command.
> + */
As it is an exported symbol, kerneldoc format for the description is
appropriate:
/**
* scsi_transfer_size - returns the minimum size that must be transferred in one go
* @cmd: command for which to get the transfer size
*
* For normal commands, this is the sector size of the device. For special
* commands, the whole command must be transferred at once. If a driver
* cannot transfer at least this much, it should fail the command.
*/
IMO the last paragraph could be condensed to
* If a driver cannot transfer at least this much, it should fail the command.
*/
> +unsigned int scsi_transfer_size(struct scsi_cmnd *cmd)
> +{
> + struct request *req = cmd->request;
Whitespace police says: Here goes a blank line.
> + if (req->cmd_type == REQ_TYPE_BLOCK_PC)
> + return req->data_len;
> + return cmd->device->sector_size;
> +}
> +EXPORT_SYMBOL(scsi_transfer_size);
--
Stefan Richter
-=====-=-=== =-=- =--==
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2007-10-19 17:46 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-18 16:27 [PATCH 1/4] scsi_cmnd: Rearrange and shrink some elements Matthew Wilcox
2007-10-18 16:27 ` [PATCH 2/4] shuttle_usbat: Eliminate use of transfersize Matthew Wilcox
2007-10-18 16:27 ` [PATCH 3/4] qla1280,qla2xxx: Remove display " Matthew Wilcox
2007-10-18 18:57 ` Seokmann Ju
2007-10-18 20:10 ` Matthew Wilcox
2007-10-18 21:05 ` Seokmann Ju
2007-10-18 21:40 ` Matthew Wilcox
2007-10-18 16:27 ` [PATCH 4/4] Replace scmd->transfersize with scsi_transfer_size() Matthew Wilcox
2007-10-19 17:45 ` Stefan Richter
-- strict thread matches above, loose matches on Subject: below --
2007-10-18 8:46 [PATCH 1/4] scsi_cmnd: Rearrange and shrink some elements Matthew Wilcox
2007-10-18 8:46 ` [PATCH 3/4] qla1280,qla2xxx: Remove display of transfersize Matthew Wilcox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).