[PATCH] fcoe: Reduce max_sectors to 1024

[PATCH] fcoe: Reduce max_sectors to 1024

[Hannes Reinecke]

The DDP offload on ixgbe is only capable of handling requests up
to 1024 sectors. So we should be exposing this to avoid spurious
messages about 'not enough user buffers'.

Cc: Vasu Dev <vasu.dev@linux.intel.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/fcoe/fcoe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 00ee0ed..602bcc9 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -287,7 +287,7 @@ static struct scsi_host_template fcoe_shost_template = {
 	.can_queue = FCOE_MAX_OUTSTANDING_COMMANDS,
 	.use_clustering = ENABLE_CLUSTERING,
 	.sg_tablesize = SG_ALL,
-	.max_sectors = 0xffff,
+	.max_sectors = 1024,
 };
 
 /**
-- 
1.7.12.4

Re: [PATCH] fcoe: Reduce max_sectors to 1024

[Christoph Hellwig]

On Wed, Jul 09, 2014 at 08:16:21AM +0200, Hannes Reinecke wrote:
> The DDP offload on ixgbe is only capable of handling requests up
> to 1024 sectors. So we should be exposing this to avoid spurious
> messages about 'not enough user buffers'.

Shouldn't the network driver be able to communicate it's offload
capabilities to the fcoe core?  I'd be happy to take this as an iterim
fix if there's a comment explaining it, but long-term it should be
driver specific.

Re: [PATCH] fcoe: Reduce max_sectors to 1024

[Hannes Reinecke]

On 07/09/2014 08:18 AM, Christoph Hellwig wrote:
> On Wed, Jul 09, 2014 at 08:16:21AM +0200, Hannes Reinecke wrote:
>> The DDP offload on ixgbe is only capable of handling requests up
>> to 1024 sectors. So we should be exposing this to avoid spurious
>> messages about 'not enough user buffers'.
>
> Shouldn't the network driver be able to communicate it's offload
> capabilities to the fcoe core?  I'd be happy to take this as an iterim
> fix if there's a comment explaining it, but long-term it should be
> driver specific.
>
ixgbe can handle up to 256 sgl elements, but the maximum single I/O 
size is 256 pages. This translates into 1024 sectors.

And seeing that the 'fcoe' driver is effectively tied to ixgbe ATM I 
doubt it's worth investing in yet another callout here.
Intel, can you clarify on i40e? Is it FCoE capable?
If so it might be worth adding a configuration knob here.
Unless i40e has the same limitations as ixgbe ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] fcoe: Reduce max_sectors to 1024

[Vasu Dev]

On Wed, 2014-07-09 at 08:40 +0200, Hannes Reinecke wrote:
> On 07/09/2014 08:18 AM, Christoph Hellwig wrote:
> > On Wed, Jul 09, 2014 at 08:16:21AM +0200, Hannes Reinecke wrote:
> >> The DDP offload on ixgbe is only capable of handling requests up
> >> to 1024 sectors. So we should be exposing this to avoid spurious
> >> messages about 'not enough user buffers'.
> >
> > Shouldn't the network driver be able to communicate it's offload
> > capabilities to the fcoe core? 

Yes network driver can and that is the best way to go to allow HW/driver
specific max sectors limit but that would require new fcoe netdev
capability exposed since currently none to indicate fcoe IO size limit.

>  I'd be happy to take this as an iterim
> > fix if there's a comment explaining it, but long-term it should be
> > driver specific.
> >

I agree with all and for now comments make sense until long term fix
implemented as suggested by Christoph and that would require new netdev
capability.

> ixgbe can handle up to 256 sgl elements, but the maximum single I/O 
> size is 256 pages. This translates into 1024 sectors.
> 
> And seeing that the 'fcoe' driver is effectively tied to ixgbe ATM I 
> doubt it's worth investing in yet another callout here.
> Intel, can you clarify on i40e? Is it FCoE capable?
> If so it might be worth adding a configuration knob here.
> Unless i40e has the same limitations as ixgbe ...

Well within ixgbe some silicon could do lot more elements than 256 and
even first DDP silicon it could do more 256 elements with each could be
either 4K, 8K, 16K or 64K in size, it means largest IO size as 256*64K.
However current implementation limited to 512K IO size max (1024 max
sector) and this wasn't issue in most typically work loads with the
exception in some case SuSe work loads issuing larger than 512K IO size
and due to that SuSe already had it reduced to max_sector = 1024 with
this patch https://bugzilla.novell.com/show_bug.cgi?id=695898#c11 and
I'm fine with doing same limit for all in upsteam kernel in case IO
merging could lead to larger than 512K IO size for all fcoe network
drivers.

Does it limit IO size for pass through IO not through block layer ? 
 
Thanks,
Vasu

> 
> Cheers,
> 
> Hannes

Re: [PATCH] fcoe: Reduce max_sectors to 1024

[Christoph Hellwig]

Hannes, can you respin the patch with a comment explaining the limit?

> Does it limit IO size for pass through IO not through block layer ? 

The max_sectors limit also limits the size of passthrough requests.