* [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 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 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 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).