* [PATCH 09/10] Enable clustering and large transfers
@ 2006-03-28 16:03 Matthew Wilcox
2006-04-01 14:35 ` Kai Makisara
0 siblings, 1 reply; 6+ messages in thread
From: Matthew Wilcox @ 2006-03-28 16:03 UTC (permalink / raw)
To: linux-scsi; +Cc: Matthew Wilcox, Kai Makisara
This patch enables clustering and sets max_sectors to 0xffff to enable
reading and writing of large blocks with tapes (and large transfers with
sg). This change is needed after the sg and st drivers started using
chained bios through scsi_request_async() in 2.6.16.
Signed-off-by: Kai Makisara <kai.makisara@kolumbus.fi>
Signed-off-by: Matthew Wilcox <matthew@wil.cx>
---
drivers/scsi/sym53c8xx_2/sym_glue.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
1c573ec4648b4b4ddadcbd1945a9a608977df513
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index e48409e..2c4e5f1 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -1870,7 +1870,8 @@ static struct scsi_host_template sym2_te
.eh_bus_reset_handler = sym53c8xx_eh_bus_reset_handler,
.eh_host_reset_handler = sym53c8xx_eh_host_reset_handler,
.this_id = 7,
- .use_clustering = DISABLE_CLUSTERING,
+ .use_clustering = ENABLE_CLUSTERING,
+ .max_sectors = 0xFFFF,
#ifdef SYM_LINUX_PROC_INFO_SUPPORT
.proc_info = sym53c8xx_proc_info,
.proc_name = NAME53C8XX,
--
1.2.4.g375a5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 09/10] Enable clustering and large transfers
2006-03-28 16:03 [PATCH 09/10] Enable clustering and large transfers Matthew Wilcox
@ 2006-04-01 14:35 ` Kai Makisara
2006-04-01 17:03 ` Christoph Hellwig
2006-04-01 20:22 ` Douglas Gilbert
0 siblings, 2 replies; 6+ messages in thread
From: Kai Makisara @ 2006-04-01 14:35 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: linux-scsi, Jens Axboe
On Tue, 28 Mar 2006, Matthew Wilcox wrote:
> This patch enables clustering and sets max_sectors to 0xffff to enable
> reading and writing of large blocks with tapes (and large transfers with
> sg). This change is needed after the sg and st drivers started using
> chained bios through scsi_request_async() in 2.6.16.
>
> Signed-off-by: Kai Makisara <kai.makisara@kolumbus.fi>
> Signed-off-by: Matthew Wilcox <matthew@wil.cx>
>
> ---
>
> drivers/scsi/sym53c8xx_2/sym_glue.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> 1c573ec4648b4b4ddadcbd1945a9a608977df513
> diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
> index e48409e..2c4e5f1 100644
> --- a/drivers/scsi/sym53c8xx_2/sym_glue.c
> +++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
> @@ -1870,7 +1870,8 @@ static struct scsi_host_template sym2_te
> .eh_bus_reset_handler = sym53c8xx_eh_bus_reset_handler,
> .eh_host_reset_handler = sym53c8xx_eh_host_reset_handler,
> .this_id = 7,
> - .use_clustering = DISABLE_CLUSTERING,
> + .use_clustering = ENABLE_CLUSTERING,
> + .max_sectors = 0xFFFF,
> #ifdef SYM_LINUX_PROC_INFO_SUPPORT
> .proc_info = sym53c8xx_proc_info,
> .proc_name = NAME53C8XX,
>
Did you approve this patch or just forgot to remove it from the queue
after the discussion initiated by Jens Axboe?
I have done some more investigation based on the message from Gerard
Roudier that Jens found. I fetched the FreeBSD driver. It had a
work-around for the 896 rev. 1 chips but I did not find that in the Linux
version. The work-around does build two device s/g elements for any
original s/g segment that crosses a 16 MB boundary.
In Linux we can do that by setting the .dma_boundary field in the
scsi_host_template. Would it be an acceptable solution to set the 16 MB
limit for all sym53c8xx adapters? It would take care of the known errata
and Gerards suspicions about possible problems with other chips.
The patch at the end does compile and it is running in this system but has
not had any more testing.
--
Kai
--- linux-2.6.16/drivers/scsi/sym53c8xx_2/sym_glue.c 2006-03-20 23:13:18.000000000 +0200
+++ linux-2.6.16-k1/drivers/scsi/sym53c8xx_2/sym_glue.c 2006-04-01 17:21:34.000000000 +0300
@@ -1978,7 +1978,11 @@ static struct scsi_host_template sym2_te
.eh_bus_reset_handler = sym53c8xx_eh_bus_reset_handler,
.eh_host_reset_handler = sym53c8xx_eh_host_reset_handler,
.this_id = 7,
- .use_clustering = DISABLE_CLUSTERING,
+ .use_clustering = ENABLE_CLUSTERING,
+ .max_sectors = 0xFFFF,
+ /* Due to errata on the 896 rev. 1 chip, don't allow s/g segments to
+ cross 16 MB boundary */
+ .dma_boundary = 0xFFFFFF,
#ifdef SYM_LINUX_PROC_INFO_SUPPORT
.proc_info = sym53c8xx_proc_info,
.proc_name = NAME53C8XX,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 09/10] Enable clustering and large transfers
2006-04-01 14:35 ` Kai Makisara
@ 2006-04-01 17:03 ` Christoph Hellwig
2006-04-01 20:26 ` Kai Makisara
2006-04-01 20:22 ` Douglas Gilbert
1 sibling, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2006-04-01 17:03 UTC (permalink / raw)
To: Kai Makisara; +Cc: Matthew Wilcox, linux-scsi, Jens Axboe
On Sat, Apr 01, 2006 at 05:35:13PM +0300, Kai Makisara wrote:
> I have done some more investigation based on the message from Gerard
> Roudier that Jens found. I fetched the FreeBSD driver. It had a
> work-around for the 896 rev. 1 chips but I did not find that in the Linux
> version. The work-around does build two device s/g elements for any
> original s/g segment that crosses a 16 MB boundary.
>
> In Linux we can do that by setting the .dma_boundary field in the
> scsi_host_template. Would it be an acceptable solution to set the 16 MB
> limit for all sym53c8xx adapters? It would take care of the known errata
> and Gerards suspicions about possible problems with other chips.
We can also set the flag in the scsi_host so not all adapters are punished.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 09/10] Enable clustering and large transfers
2006-04-01 14:35 ` Kai Makisara
2006-04-01 17:03 ` Christoph Hellwig
@ 2006-04-01 20:22 ` Douglas Gilbert
2006-04-02 14:18 ` Kai Makisara
1 sibling, 1 reply; 6+ messages in thread
From: Douglas Gilbert @ 2006-04-01 20:22 UTC (permalink / raw)
To: Kai Makisara; +Cc: Matthew Wilcox, linux-scsi, Jens Axboe
Kai Makisara wrote:
> On Tue, 28 Mar 2006, Matthew Wilcox wrote:
>
>
>>This patch enables clustering and sets max_sectors to 0xffff to enable
>>reading and writing of large blocks with tapes (and large transfers with
>>sg). This change is needed after the sg and st drivers started using
>>chained bios through scsi_request_async() in 2.6.16.
>>
>>Signed-off-by: Kai Makisara <kai.makisara@kolumbus.fi>
>>Signed-off-by: Matthew Wilcox <matthew@wil.cx>
>>
>>---
>>
>> drivers/scsi/sym53c8xx_2/sym_glue.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>>1c573ec4648b4b4ddadcbd1945a9a608977df513
>>diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
>>index e48409e..2c4e5f1 100644
>>--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
>>+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
>>@@ -1870,7 +1870,8 @@ static struct scsi_host_template sym2_te
>> .eh_bus_reset_handler = sym53c8xx_eh_bus_reset_handler,
>> .eh_host_reset_handler = sym53c8xx_eh_host_reset_handler,
>> .this_id = 7,
>>- .use_clustering = DISABLE_CLUSTERING,
>>+ .use_clustering = ENABLE_CLUSTERING,
>>+ .max_sectors = 0xFFFF,
>> #ifdef SYM_LINUX_PROC_INFO_SUPPORT
>> .proc_info = sym53c8xx_proc_info,
>> .proc_name = NAME53C8XX,
>>
>
> Did you approve this patch or just forgot to remove it from the queue
> after the discussion initiated by Jens Axboe?
>
> I have done some more investigation based on the message from Gerard
> Roudier that Jens found. I fetched the FreeBSD driver. It had a
> work-around for the 896 rev. 1 chips but I did not find that in the Linux
> version. The work-around does build two device s/g elements for any
> original s/g segment that crosses a 16 MB boundary.
Kai + Matthew,
In my testing with lk 2.6.16 the 16 MB boundary is a
hard limit, after several other things are tweaked.
The sym53c8xx driver won't even come close due to:
#define SYM_CONF_MAX_SG (96)
...
instance->sg_tablesize = SYM_CONF_MAX_SG;
I assume that the st driver (like sg) limits on .sg_tablesize .
Matthew, if you change this please set it at 256 (not SG_ALL
which is 255).
I have attempted to capture the current (overly complex)
constraints on maximum transfer size at:
http://www.torque.net/sg/sg_io.html
in the section on "Maximum transfer size per command".
> In Linux we can do that by setting the .dma_boundary field in the
> scsi_host_template. Would it be an acceptable solution to set the 16 MB
> limit for all sym53c8xx adapters? It would take care of the known errata
> and Gerards suspicions about possible problems with other chips.
>
> The patch at the end does compile and it is running in this system but has
> not had any more testing.
Doug Gilbert
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 09/10] Enable clustering and large transfers
2006-04-01 17:03 ` Christoph Hellwig
@ 2006-04-01 20:26 ` Kai Makisara
0 siblings, 0 replies; 6+ messages in thread
From: Kai Makisara @ 2006-04-01 20:26 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Matthew Wilcox, linux-scsi, Jens Axboe
On Sat, 1 Apr 2006, Christoph Hellwig wrote:
> On Sat, Apr 01, 2006 at 05:35:13PM +0300, Kai Makisara wrote:
> > I have done some more investigation based on the message from Gerard
> > Roudier that Jens found. I fetched the FreeBSD driver. It had a
> > work-around for the 896 rev. 1 chips but I did not find that in the Linux
> > version. The work-around does build two device s/g elements for any
> > original s/g segment that crosses a 16 MB boundary.
> >
> > In Linux we can do that by setting the .dma_boundary field in the
> > scsi_host_template. Would it be an acceptable solution to set the 16 MB
> > limit for all sym53c8xx adapters? It would take care of the known errata
> > and Gerards suspicions about possible problems with other chips.
>
> We can also set the flag in the scsi_host so not all adapters are punished.
>
Yes, but the question is, do we want to be conservative and set the mask
for all adapters (including the ones possibly having errata we don't
know), or less conservative and set it for the ones we know are buggy.
For those who make decisions, below is an attempt at limiting dma only for
the chip we know is having a problem. Builds and boots (with 53c1010).
Kai
--- linux-2.6.16/drivers/scsi/sym53c8xx_2/sym_glue.c 2006-03-20 23:13:18.000000000 +0200
+++ linux-2.6.16-k1/drivers/scsi/sym53c8xx_2/sym_glue.c 2006-04-01 23:16:04.000000000 +0300
@@ -1576,6 +1576,7 @@ static struct Scsi_Host * __devinit sym_
struct host_data *host_data;
struct sym_hcb *np = NULL;
struct Scsi_Host *instance = NULL;
+ struct sym_chip *chip = &dev->chip;
struct pci_dev *pdev = dev->pdev;
unsigned long flags;
struct sym_fw *fw;
@@ -1712,6 +1713,13 @@ static struct Scsi_Host * __devinit sym_
BUG_ON(sym2_transport_template == NULL);
instance->transportt = sym2_transport_template;
+ /*
+ * Due to errata on the 896 rev. 1 chip, don't allow s/g segments to
+ * cross 16 MB boundary
+ */
+ if (pdev->device == PCI_DEVICE_ID_NCR_53C896 && chip->revision_id < 2)
+ instance->dma_boundary = 0xFFFFFF;
+
spin_unlock_irqrestore(instance->host_lock, flags);
return instance;
@@ -1978,7 +1986,8 @@ static struct scsi_host_template sym2_te
.eh_bus_reset_handler = sym53c8xx_eh_bus_reset_handler,
.eh_host_reset_handler = sym53c8xx_eh_host_reset_handler,
.this_id = 7,
- .use_clustering = DISABLE_CLUSTERING,
+ .use_clustering = ENABLE_CLUSTERING,
+ .max_sectors = 0xFFFF,
#ifdef SYM_LINUX_PROC_INFO_SUPPORT
.proc_info = sym53c8xx_proc_info,
.proc_name = NAME53C8XX,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 09/10] Enable clustering and large transfers
2006-04-01 20:22 ` Douglas Gilbert
@ 2006-04-02 14:18 ` Kai Makisara
0 siblings, 0 replies; 6+ messages in thread
From: Kai Makisara @ 2006-04-02 14:18 UTC (permalink / raw)
To: Douglas Gilbert; +Cc: Matthew Wilcox, linux-scsi, Jens Axboe
On Sat, 1 Apr 2006, Douglas Gilbert wrote:
> Kai Makisara wrote:
> > On Tue, 28 Mar 2006, Matthew Wilcox wrote:
> >
> >
> >>This patch enables clustering and sets max_sectors to 0xffff to enable
> >>reading and writing of large blocks with tapes (and large transfers with
> >>sg). This change is needed after the sg and st drivers started using
> >>chained bios through scsi_request_async() in 2.6.16.
> >>
> >>Signed-off-by: Kai Makisara <kai.makisara@kolumbus.fi>
> >>Signed-off-by: Matthew Wilcox <matthew@wil.cx>
> >>
> >>---
> >>
> >> drivers/scsi/sym53c8xx_2/sym_glue.c | 3 ++-
> >> 1 files changed, 2 insertions(+), 1 deletions(-)
> >>
> >>1c573ec4648b4b4ddadcbd1945a9a608977df513
> >>diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
> >>index e48409e..2c4e5f1 100644
> >>--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
> >>+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
> >>@@ -1870,7 +1870,8 @@ static struct scsi_host_template sym2_te
> >> .eh_bus_reset_handler = sym53c8xx_eh_bus_reset_handler,
> >> .eh_host_reset_handler = sym53c8xx_eh_host_reset_handler,
> >> .this_id = 7,
> >>- .use_clustering = DISABLE_CLUSTERING,
> >>+ .use_clustering = ENABLE_CLUSTERING,
> >>+ .max_sectors = 0xFFFF,
> >> #ifdef SYM_LINUX_PROC_INFO_SUPPORT
> >> .proc_info = sym53c8xx_proc_info,
> >> .proc_name = NAME53C8XX,
> >>
...
> Kai + Matthew,
> In my testing with lk 2.6.16 the 16 MB boundary is a
> hard limit, after several other things are tweaked.
>
> The sym53c8xx driver won't even come close due to:
> #define SYM_CONF_MAX_SG (96)
> ...
> instance->sg_tablesize = SYM_CONF_MAX_SG;
>
I was able to read 65535 blocks (32 MB - 512 B) for each SCSI command from
my disk using the sym53c8xx_2 driver and SG_IO through the sg driverv. I
did not change SYM_CONF_MAX but I did add
blk_queue_max_segment_size(sdev->request_queue, 256 * 1024);
to sym53c8xx_slave_configure to increase the maximum segment size for the
queue. Did you do also this?
> I assume that the st driver (like sg) limits on .sg_tablesize .
> Matthew, if you change this please set it at 256 (not SG_ALL
> which is 255).
>
> I have attempted to capture the current (overly complex)
> constraints on maximum transfer size at:
> http://www.torque.net/sg/sg_io.html
> in the section on "Maximum transfer size per command".
>
The maximum segments size limitation is mentioned on your page. Maybe you
emphasize that without changing this you can't get beyond 16 MB with only
128 segments.
--
Kai
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-04-02 14:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-28 16:03 [PATCH 09/10] Enable clustering and large transfers Matthew Wilcox
2006-04-01 14:35 ` Kai Makisara
2006-04-01 17:03 ` Christoph Hellwig
2006-04-01 20:26 ` Kai Makisara
2006-04-01 20:22 ` Douglas Gilbert
2006-04-02 14:18 ` Kai Makisara
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).