Re: ISP1760 driver crashes

Re: ISP1760 driver crashes

[Alan Stern]

On Wed, 19 Nov 2008, Sebastian Andrzej Siewior wrote:

> Hommel, Thomas (GE EntSol, Intelligent Platforms) wrote:
> 
> > I indeed have HIGHMEM enabled in my configuration.
> > I recompiled the kernel without HIGHMEM and it works. I don't think that
> > this a satisfying solution for a board with up to 2GB of RAM and
> > considerable amount of VMALLOC space, but at least this works.
> > If you have any more ideas how to circumvent this, please let me know.
> Sure, this is not a sollution but atleast now I know what happens:
> - The kernel allocates memory for transfer
> - the memory is highmem and not in kernel so the buffer is NULL
> - we don't have a dma-mask and therefore the dma address is 0
> - boom
> 
> The sollution would be probably to prevent the usb-storage core to 
> allocate memory from HIGHMEM.

usb-storage doesn't allocate the memory.  The memory is allocated by 
the block layer or the filesystem.

> Now I don't if there is a flag for something 
> like that and I am not using that. On the other hand this may be broken 
> for a long time and you are the first one which has that much memory with 
> no DMA-capable USB controller.

Jens, is there any way to tell the kernel that a device uses PIO and 
therefore its buffers shouldn't be allocated in high memory?  For 
example, shouldn't a NULL dma_mask do this?

If not, are there standard routines to set up bounce buffers for such 
devices?

Alan Stern

Re: ISP1760 driver crashes

[Jens Axboe]

On Wed, Nov 19 2008, Alan Stern wrote:
> On Wed, 19 Nov 2008, Sebastian Andrzej Siewior wrote:
> 
> > Hommel, Thomas (GE EntSol, Intelligent Platforms) wrote:
> > 
> > > I indeed have HIGHMEM enabled in my configuration.
> > > I recompiled the kernel without HIGHMEM and it works. I don't think that
> > > this a satisfying solution for a board with up to 2GB of RAM and
> > > considerable amount of VMALLOC space, but at least this works.
> > > If you have any more ideas how to circumvent this, please let me know.
> > Sure, this is not a sollution but atleast now I know what happens:
> > - The kernel allocates memory for transfer
> > - the memory is highmem and not in kernel so the buffer is NULL
> > - we don't have a dma-mask and therefore the dma address is 0
> > - boom
> > 
> > The sollution would be probably to prevent the usb-storage core to 
> > allocate memory from HIGHMEM.
> 
> usb-storage doesn't allocate the memory.  The memory is allocated by 
> the block layer or the filesystem.
> 
> > Now I don't if there is a flag for something 
> > like that and I am not using that. On the other hand this may be broken 
> > for a long time and you are the first one which has that much memory with 
> > no DMA-capable USB controller.
> 
> Jens, is there any way to tell the kernel that a device uses PIO and 
> therefore its buffers shouldn't be allocated in high memory?  For 
> example, shouldn't a NULL dma_mask do this?

Sure, just use blk_queue_bounce_limit(q, BLK_BOUNCE_HIGH), then you are
certain that you will always have a virtual mapping for the IO you
receive.

Or you can use the bio kmap/kunmap helpers to get such a mapping
temporarily if you wish. But if your pio condition is permanent, you may
as well just use bouncing.

-- 
Jens Axboe

Re: ISP1760 driver crashes

[Alan Stern]

On Wed, 19 Nov 2008, Jens Axboe wrote:

> On Wed, Nov 19 2008, Alan Stern wrote:
> > On Wed, 19 Nov 2008, Sebastian Andrzej Siewior wrote:
> > 
> > > Hommel, Thomas (GE EntSol, Intelligent Platforms) wrote:
> > > 
> > > > I indeed have HIGHMEM enabled in my configuration.
> > > > I recompiled the kernel without HIGHMEM and it works. I don't think that
> > > > this a satisfying solution for a board with up to 2GB of RAM and
> > > > considerable amount of VMALLOC space, but at least this works.
> > > > If you have any more ideas how to circumvent this, please let me know.
> > > Sure, this is not a sollution but atleast now I know what happens:
> > > - The kernel allocates memory for transfer
> > > - the memory is highmem and not in kernel so the buffer is NULL
> > > - we don't have a dma-mask and therefore the dma address is 0
> > > - boom
> > > 
> > > The sollution would be probably to prevent the usb-storage core to 
> > > allocate memory from HIGHMEM.
> > 
> > usb-storage doesn't allocate the memory.  The memory is allocated by 
> > the block layer or the filesystem.
> > 
> > > Now I don't if there is a flag for something 
> > > like that and I am not using that. On the other hand this may be broken 
> > > for a long time and you are the first one which has that much memory with 
> > > no DMA-capable USB controller.
> > 
> > Jens, is there any way to tell the kernel that a device uses PIO and 
> > therefore its buffers shouldn't be allocated in high memory?  For 
> > example, shouldn't a NULL dma_mask do this?
> 
> Sure, just use blk_queue_bounce_limit(q, BLK_BOUNCE_HIGH), then you are
> certain that you will always have a virtual mapping for the IO you
> receive.
> 
> Or you can use the bio kmap/kunmap helpers to get such a mapping
> temporarily if you wish. But if your pio condition is permanent, you may
> as well just use bouncing.

Thank you.

Thomas, the blk_queue_bounce_limit() routine is called in 
drivers/scsi/scsi_lib.c:__scsi_alloc_queue().  The value it passes is 
computed by scsi_calculate_bounce_limit(), and in that routine 
host_dev->dma_mask should be NULL (since isp1760-hcd sets the mask to 
NULL).  Therefore the bounce limit should be 0xffffffff.

Now maybe this value isn't correct.  You can try the patch below to see 
if it helps.  If it doesn't, add a printk in __scsi_alloc_queue() to 
see what bounce limit value is getting used.

Alan Stern



Index: usb-2.6/drivers/scsi/scsi_lib.c
===================================================================
--- usb-2.6.orig/drivers/scsi/scsi_lib.c
+++ usb-2.6/drivers/scsi/scsi_lib.c
@@ -1684,7 +1684,7 @@ static void scsi_request_fn(struct reque
 u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
 {
 	struct device *host_dev;
-	u64 bounce_limit = 0xffffffff;
+	u64 bounce_limit = BLK_BOUNCE_HIGH;
 
 	if (shost->unchecked_isa_dma)
 		return BLK_BOUNCE_ISA;

Re: ISP1760 driver crashes

[Jens Axboe]

On Wed, Nov 19 2008, Alan Stern wrote:
> On Wed, 19 Nov 2008, Jens Axboe wrote:
> 
> > On Wed, Nov 19 2008, Alan Stern wrote:
> > > On Wed, 19 Nov 2008, Sebastian Andrzej Siewior wrote:
> > > 
> > > > Hommel, Thomas (GE EntSol, Intelligent Platforms) wrote:
> > > > 
> > > > > I indeed have HIGHMEM enabled in my configuration.
> > > > > I recompiled the kernel without HIGHMEM and it works. I don't think that
> > > > > this a satisfying solution for a board with up to 2GB of RAM and
> > > > > considerable amount of VMALLOC space, but at least this works.
> > > > > If you have any more ideas how to circumvent this, please let me know.
> > > > Sure, this is not a sollution but atleast now I know what happens:
> > > > - The kernel allocates memory for transfer
> > > > - the memory is highmem and not in kernel so the buffer is NULL
> > > > - we don't have a dma-mask and therefore the dma address is 0
> > > > - boom
> > > > 
> > > > The sollution would be probably to prevent the usb-storage core to 
> > > > allocate memory from HIGHMEM.
> > > 
> > > usb-storage doesn't allocate the memory.  The memory is allocated by 
> > > the block layer or the filesystem.
> > > 
> > > > Now I don't if there is a flag for something 
> > > > like that and I am not using that. On the other hand this may be broken 
> > > > for a long time and you are the first one which has that much memory with 
> > > > no DMA-capable USB controller.
> > > 
> > > Jens, is there any way to tell the kernel that a device uses PIO and 
> > > therefore its buffers shouldn't be allocated in high memory?  For 
> > > example, shouldn't a NULL dma_mask do this?
> > 
> > Sure, just use blk_queue_bounce_limit(q, BLK_BOUNCE_HIGH), then you are
> > certain that you will always have a virtual mapping for the IO you
> > receive.
> > 
> > Or you can use the bio kmap/kunmap helpers to get such a mapping
> > temporarily if you wish. But if your pio condition is permanent, you may
> > as well just use bouncing.
> 
> Thank you.
> 
> Thomas, the blk_queue_bounce_limit() routine is called in 
> drivers/scsi/scsi_lib.c:__scsi_alloc_queue().  The value it passes is 
> computed by scsi_calculate_bounce_limit(), and in that routine 
> host_dev->dma_mask should be NULL (since isp1760-hcd sets the mask to 
> NULL).  Therefore the bounce limit should be 0xffffffff.
> 
> Now maybe this value isn't correct.  You can try the patch below to see 
> if it helps.  If it doesn't, add a printk in __scsi_alloc_queue() to 
> see what bounce limit value is getting used.
> 
> Alan Stern
> 
> 
> 
> Index: usb-2.6/drivers/scsi/scsi_lib.c
> ===================================================================
> --- usb-2.6.orig/drivers/scsi/scsi_lib.c
> +++ usb-2.6/drivers/scsi/scsi_lib.c
> @@ -1684,7 +1684,7 @@ static void scsi_request_fn(struct reque
>  u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
>  {
>  	struct device *host_dev;
> -	u64 bounce_limit = 0xffffffff;
> +	u64 bounce_limit = BLK_BOUNCE_HIGH;
>  
>  	if (shost->unchecked_isa_dma)
>  		return BLK_BOUNCE_ISA;
> 

The best solution is probably to either provide a "doesn't do highmem"
in the scsi host template, or provide an appropriate DMA mask for the
pci device to indicate it through that setting instead.

-- 
Jens Axboe

RE: ISP1760 driver crashes

[Hommel, Thomas (GE EntSol, Intelligent Platforms)]

> 
> On Wed, 19 Nov 2008, Jens Axboe wrote:
> 
> > On Wed, Nov 19 2008, Alan Stern wrote:
> > > On Wed, 19 Nov 2008, Sebastian Andrzej Siewior wrote:
> > > 
> > > > Hommel, Thomas (GE EntSol, Intelligent Platforms) wrote:
> > > > 
> > > > > I indeed have HIGHMEM enabled in my configuration.
> > > > > I recompiled the kernel without HIGHMEM and it works. I don't 
> > > > > think that this a satisfying solution for a board 
> with up to 2GB 
> > > > > of RAM and considerable amount of VMALLOC space, but 
> at least this works.
> > > > > If you have any more ideas how to circumvent this, 
> please let me know.
> > > > Sure, this is not a sollution but atleast now I know 
> what happens:
> > > > - The kernel allocates memory for transfer
> > > > - the memory is highmem and not in kernel so the buffer is NULL
> > > > - we don't have a dma-mask and therefore the dma address is 0
> > > > - boom
> > > > 
> > > > The sollution would be probably to prevent the 
> usb-storage core to 
> > > > allocate memory from HIGHMEM.
> > > 
> > > usb-storage doesn't allocate the memory.  The memory is 
> allocated by 
> > > the block layer or the filesystem.
> > > 
> > > > Now I don't if there is a flag for something like that and I am 
> > > > not using that. On the other hand this may be broken for a long 
> > > > time and you are the first one which has that much 
> memory with no 
> > > > DMA-capable USB controller.
> > > 
> > > Jens, is there any way to tell the kernel that a device 
> uses PIO and 
> > > therefore its buffers shouldn't be allocated in high memory?  For 
> > > example, shouldn't a NULL dma_mask do this?
> > 
> > Sure, just use blk_queue_bounce_limit(q, BLK_BOUNCE_HIGH), then you 
> > are certain that you will always have a virtual mapping for 
> the IO you 
> > receive.
> > 
> > Or you can use the bio kmap/kunmap helpers to get such a mapping 
> > temporarily if you wish. But if your pio condition is 
> permanent, you 
> > may as well just use bouncing.
> 
> Thank you.
> 
> Thomas, the blk_queue_bounce_limit() routine is called in 
> drivers/scsi/scsi_lib.c:__scsi_alloc_queue().  The value it 
> passes is computed by scsi_calculate_bounce_limit(), and in 
> that routine host_dev->dma_mask should be NULL (since 
> isp1760-hcd sets the mask to NULL).  Therefore the bounce 
> limit should be 0xffffffff.
> 
> Now maybe this value isn't correct.  You can try the patch 
> below to see if it helps.  If it doesn't, add a printk in 
> __scsi_alloc_queue() to see what bounce limit value is getting used.
> 
> Alan Stern
> 
Thanks Alan,
This seems to fix the problem.

Thomas
> 
> 
> Index: usb-2.6/drivers/scsi/scsi_lib.c
> ===================================================================
> --- usb-2.6.orig/drivers/scsi/scsi_lib.c
> +++ usb-2.6/drivers/scsi/scsi_lib.c
> @@ -1684,7 +1684,7 @@ static void scsi_request_fn(struct reque
>  u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)  {
>  	struct device *host_dev;
> -	u64 bounce_limit = 0xffffffff;
> +	u64 bounce_limit = BLK_BOUNCE_HIGH;
>  
>  	if (shost->unchecked_isa_dma)
>  		return BLK_BOUNCE_ISA;
> 
> 

Re: ISP1760 driver crashes

[Alan Stern]

On Wed, 19 Nov 2008, Jens Axboe wrote:

> > --- usb-2.6.orig/drivers/scsi/scsi_lib.c
> > +++ usb-2.6/drivers/scsi/scsi_lib.c
> > @@ -1684,7 +1684,7 @@ static void scsi_request_fn(struct reque
> >  u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
> >  {
> >  	struct device *host_dev;
> > -	u64 bounce_limit = 0xffffffff;
> > +	u64 bounce_limit = BLK_BOUNCE_HIGH;
> >  
> >  	if (shost->unchecked_isa_dma)
> >  		return BLK_BOUNCE_ISA;
> > 
> 
> The best solution is probably to either provide a "doesn't do highmem"
> in the scsi host template, or provide an appropriate DMA mask for the
> pci device to indicate it through that setting instead.

The DMA mask is currently set to NULL.  Is that not appropriate for a 
device that can't do DMA?  If not, then what would be appropriate?

Also, is the patch above not correct?

Alan Stern

Re: ISP1760 driver crashes

[Jens Axboe]

On Wed, Nov 19 2008, Alan Stern wrote:
> On Wed, 19 Nov 2008, Jens Axboe wrote:
> 
> > > --- usb-2.6.orig/drivers/scsi/scsi_lib.c
> > > +++ usb-2.6/drivers/scsi/scsi_lib.c
> > > @@ -1684,7 +1684,7 @@ static void scsi_request_fn(struct reque
> > >  u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
> > >  {
> > >  	struct device *host_dev;
> > > -	u64 bounce_limit = 0xffffffff;
> > > +	u64 bounce_limit = BLK_BOUNCE_HIGH;
> > >  
> > >  	if (shost->unchecked_isa_dma)
> > >  		return BLK_BOUNCE_ISA;
> > > 
> > 
> > The best solution is probably to either provide a "doesn't do highmem"
> > in the scsi host template, or provide an appropriate DMA mask for the
> > pci device to indicate it through that setting instead.
> 
> The DMA mask is currently set to NULL.  Is that not appropriate for a 
> device that can't do DMA?  If not, then what would be appropriate?

It's changing behaviour. There's no current rule that says if you don't
have a dma mask set, we only do PIO (even if such a rule DOES make
sense). Additionally, you don't HAVE to bounce for PIO. As I wrote
earlier, it's perfectly feasible to use bio kmap'ings to do the
transfer.

> Also, is the patch above not correct?

It'll certainly work in the sense that if you don't have a dma_mask set,
you only get lowmem pages. Whether the new behaviour is something we
want, not sure. Check with James what he thinks, it's his domain.

-- 
Jens Axboe

Re: ISP1760 driver crashes

[FUJITA Tomonori]

On Wed, 19 Nov 2008 18:21:25 +0100
Jens Axboe <jens.axboe@oracle.com> wrote:

> On Wed, Nov 19 2008, Alan Stern wrote:
> > On Wed, 19 Nov 2008, Jens Axboe wrote:
> > 
> > > > --- usb-2.6.orig/drivers/scsi/scsi_lib.c
> > > > +++ usb-2.6/drivers/scsi/scsi_lib.c
> > > > @@ -1684,7 +1684,7 @@ static void scsi_request_fn(struct reque
> > > >  u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
> > > >  {
> > > >  	struct device *host_dev;
> > > > -	u64 bounce_limit = 0xffffffff;
> > > > +	u64 bounce_limit = BLK_BOUNCE_HIGH;
> > > >  
> > > >  	if (shost->unchecked_isa_dma)
> > > >  		return BLK_BOUNCE_ISA;
> > > > 
> > > 
> > > The best solution is probably to either provide a "doesn't do highmem"
> > > in the scsi host template, or provide an appropriate DMA mask for the
> > > pci device to indicate it through that setting instead.
> > 
> > The DMA mask is currently set to NULL.  Is that not appropriate for a 
> > device that can't do DMA?  If not, then what would be appropriate?
> 
> It's changing behaviour. There's no current rule that says if you don't
> have a dma mask set, we only do PIO (even if such a rule DOES make
> sense). Additionally, you don't HAVE to bounce for PIO. As I wrote
> earlier, it's perfectly feasible to use bio kmap'ings to do the
> transfer.
> 
> > Also, is the patch above not correct?
> 
> It'll certainly work in the sense that if you don't have a dma_mask set,
> you only get lowmem pages. Whether the new behaviour is something we
> want, not sure. Check with James what he thinks, it's his domain.

We have been used 4GB for long time if dma_mask is zero (I guess we
use 4GB as kinda the default dma address limit at several places). The
majority of drivers (such as pci) sets properly dev->dma_mask so the
patch might not change anything but suddenly changing the
long-standing rule in an odd way (use BLK_BOUNCE_HIGH if dma_mask is
zero) doesn't sound a good idea to me.

Why not calling blk_queue_bounce_limit() in the slave_configure hook?
I think that it's the common way for SCSI LLDs with odd bounce limit.

Re: ISP1760 driver crashes

[Jens Axboe]

On Thu, Nov 20 2008, FUJITA Tomonori wrote:
> On Wed, 19 Nov 2008 18:21:25 +0100
> Jens Axboe <jens.axboe@oracle.com> wrote:
> 
> > On Wed, Nov 19 2008, Alan Stern wrote:
> > > On Wed, 19 Nov 2008, Jens Axboe wrote:
> > > 
> > > > > --- usb-2.6.orig/drivers/scsi/scsi_lib.c
> > > > > +++ usb-2.6/drivers/scsi/scsi_lib.c
> > > > > @@ -1684,7 +1684,7 @@ static void scsi_request_fn(struct reque
> > > > >  u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
> > > > >  {
> > > > >  	struct device *host_dev;
> > > > > -	u64 bounce_limit = 0xffffffff;
> > > > > +	u64 bounce_limit = BLK_BOUNCE_HIGH;
> > > > >  
> > > > >  	if (shost->unchecked_isa_dma)
> > > > >  		return BLK_BOUNCE_ISA;
> > > > > 
> > > > 
> > > > The best solution is probably to either provide a "doesn't do highmem"
> > > > in the scsi host template, or provide an appropriate DMA mask for the
> > > > pci device to indicate it through that setting instead.
> > > 
> > > The DMA mask is currently set to NULL.  Is that not appropriate for a 
> > > device that can't do DMA?  If not, then what would be appropriate?
> > 
> > It's changing behaviour. There's no current rule that says if you don't
> > have a dma mask set, we only do PIO (even if such a rule DOES make
> > sense). Additionally, you don't HAVE to bounce for PIO. As I wrote
> > earlier, it's perfectly feasible to use bio kmap'ings to do the
> > transfer.
> > 
> > > Also, is the patch above not correct?
> > 
> > It'll certainly work in the sense that if you don't have a dma_mask set,
> > you only get lowmem pages. Whether the new behaviour is something we
> > want, not sure. Check with James what he thinks, it's his domain.
> 
> We have been used 4GB for long time if dma_mask is zero (I guess we
> use 4GB as kinda the default dma address limit at several places). The
> majority of drivers (such as pci) sets properly dev->dma_mask so the
> patch might not change anything but suddenly changing the
> long-standing rule in an odd way (use BLK_BOUNCE_HIGH if dma_mask is
> zero) doesn't sound a good idea to me.
> 
> Why not calling blk_queue_bounce_limit() in the slave_configure hook?
> I think that it's the common way for SCSI LLDs with odd bounce limit.

Yep, that should indeed work just fine and is preferable to changing
this logic.

-- 
Jens Axboe

Re: ISP1760 driver crashes

[Alan Stern]

On Thu, 20 Nov 2008, FUJITA Tomonori wrote:

> We have been used 4GB for long time if dma_mask is zero (I guess we
> use 4GB as kinda the default dma address limit at several places). The
> majority of drivers (such as pci) sets properly dev->dma_mask so the
> patch might not change anything but suddenly changing the
> long-standing rule in an odd way (use BLK_BOUNCE_HIGH if dma_mask is
> zero) doesn't sound a good idea to me.
> 
> Why not calling blk_queue_bounce_limit() in the slave_configure hook?
> I think that it's the common way for SCSI LLDs with odd bounce limit.

Thomas, here's a patch to do what Tomonori suggests.  Try replacing the 
old patch with this one.

Alan Stern


Index: usb-2.6/drivers/usb/storage/scsiglue.c
===================================================================
--- usb-2.6.orig/drivers/usb/storage/scsiglue.c
+++ usb-2.6/drivers/usb/storage/scsiglue.c
@@ -129,6 +129,14 @@ static int slave_configure(struct scsi_d
 					      max_sectors);
 	}
 
+	/* Some USB host controllers can't do DMA; they have to use PIO.
+	 * They indicate this by setting their dma_mask to NULL.  For
+	 * such controllers we need to make sure the block layer sets
+	 * up bounce buffers in addressable memory.
+	 */
+	if (!us->pusb_dev->bus->controller->dma_mask)
+		blk_queue_bounce_limit(sdev->request_queue, BLK_BOUNCE_HIGH);
+
 	/* We can't put these settings in slave_alloc() because that gets
 	 * called before the device type is known.  Consequently these
 	 * settings can't be overridden via the scsi devinfo mechanism. */

Re: ISP1760 driver crashes

[Jens Axboe]

On Thu, Nov 20 2008, Alan Stern wrote:
> On Thu, 20 Nov 2008, FUJITA Tomonori wrote:
> 
> > We have been used 4GB for long time if dma_mask is zero (I guess we
> > use 4GB as kinda the default dma address limit at several places). The
> > majority of drivers (such as pci) sets properly dev->dma_mask so the
> > patch might not change anything but suddenly changing the
> > long-standing rule in an odd way (use BLK_BOUNCE_HIGH if dma_mask is
> > zero) doesn't sound a good idea to me.
> > 
> > Why not calling blk_queue_bounce_limit() in the slave_configure hook?
> > I think that it's the common way for SCSI LLDs with odd bounce limit.
> 
> Thomas, here's a patch to do what Tomonori suggests.  Try replacing the 
> old patch with this one.
> 
> Alan Stern
> 
> 
> Index: usb-2.6/drivers/usb/storage/scsiglue.c
> ===================================================================
> --- usb-2.6.orig/drivers/usb/storage/scsiglue.c
> +++ usb-2.6/drivers/usb/storage/scsiglue.c
> @@ -129,6 +129,14 @@ static int slave_configure(struct scsi_d
>  					      max_sectors);
>  	}
>  
> +	/* Some USB host controllers can't do DMA; they have to use PIO.
> +	 * They indicate this by setting their dma_mask to NULL.  For
> +	 * such controllers we need to make sure the block layer sets
> +	 * up bounce buffers in addressable memory.
> +	 */
> +	if (!us->pusb_dev->bus->controller->dma_mask)
> +		blk_queue_bounce_limit(sdev->request_queue, BLK_BOUNCE_HIGH);
> +
>  	/* We can't put these settings in slave_alloc() because that gets
>  	 * called before the device type is known.  Consequently these
>  	 * settings can't be overridden via the scsi devinfo mechanism. */
> 

That looks like a good fix. You can add my Acked-by: to that, if you
wish.

-- 
Jens Axboe

RE: ISP1760 driver crashes

[Hommel, Thomas (GE EntSol, Intelligent Platforms)]

> Alan Stern wrote:
> 
> On Thu, 20 Nov 2008, FUJITA Tomonori wrote:
> 
> > We have been used 4GB for long time if dma_mask is zero (I guess we 
> > use 4GB as kinda the default dma address limit at several 
> places). The 
> > majority of drivers (such as pci) sets properly 
> dev->dma_mask so the 
> > patch might not change anything but suddenly changing the 
> > long-standing rule in an odd way (use BLK_BOUNCE_HIGH if dma_mask is
> > zero) doesn't sound a good idea to me.
> > 
> > Why not calling blk_queue_bounce_limit() in the 
> slave_configure hook?
> > I think that it's the common way for SCSI LLDs with odd 
> bounce limit.
> 
> Thomas, here's a patch to do what Tomonori suggests.  Try 
> replacing the old patch with this one.
> 
Thanks, I've tried the new patch and everything works ok.
Thomas

> Alan Stern
> 
> 
> Index: usb-2.6/drivers/usb/storage/scsiglue.c
> ===================================================================
> --- usb-2.6.orig/drivers/usb/storage/scsiglue.c
> +++ usb-2.6/drivers/usb/storage/scsiglue.c
> @@ -129,6 +129,14 @@ static int slave_configure(struct scsi_d
>  					      max_sectors);
>  	}
>  
> +	/* Some USB host controllers can't do DMA; they have to use PIO.
> +	 * They indicate this by setting their dma_mask to NULL.  For
> +	 * such controllers we need to make sure the block layer sets
> +	 * up bounce buffers in addressable memory.
> +	 */
> +	if (!us->pusb_dev->bus->controller->dma_mask)
> +		blk_queue_bounce_limit(sdev->request_queue, 
> BLK_BOUNCE_HIGH);
> +
>  	/* We can't put these settings in slave_alloc() because 
> that gets
>  	 * called before the device type is known.  Consequently these
>  	 * settings can't be overridden via the scsi devinfo 
> mechanism. */
> 
>