[PATCH] block: do not artificially constrain max_sectors for stacking drivers

[PATCH] block: do not artificially constrain max_sectors for stacking drivers

[Mike Snitzer]

blk_set_stacking_limits is intended to allow stacking drivers to build
up the limits of the stacked device based on the underlying devices'
limits.  But in the case of 'max_sectors' the default of
BLK_DEF_MAX_SECTORS (1024) doesn't allow the stacking driver to inherit
a max_sectors larger than 1024.

It is now clear that this artificial limit is getting in the way so
change blk_set_stacking_limits's max_sectors to UINT_MAX (which allows
stacking drivers like dm-multipath to inherit 'max_sectors' from the
underlying paths).

Reported-by: Vijay Chauhan <vijay.chauhan@netapp.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
---
 block/blk-settings.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index d3234fc..565a678 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -143,8 +143,7 @@ void blk_set_stacking_limits(struct queue_limits *lim)
 	lim->discard_zeroes_data = 1;
 	lim->max_segments = USHRT_MAX;
 	lim->max_hw_sectors = UINT_MAX;
-
-	lim->max_sectors = BLK_DEF_MAX_SECTORS;
+	lim->max_sectors = UINT_MAX;
 }
 EXPORT_SYMBOL(blk_set_stacking_limits);
 

[PATCH v2] block: do not artificially constrain max_sectors for stacking drivers

[Mike Snitzer]

blk_set_stacking_limits() is intended to allow stacking drivers to build
up the limits of the stacked device based on the underlying devices'
limits.  But in the case of 'max_sectors' the default of
BLK_DEF_MAX_SECTORS (1024) doesn't allow the stacking driver to inherit
a max_sectors larger than 1024.

It is now clear that this artificial limit is getting in the way so
change blk_set_stacking_limits's max_sectors to UINT_MAX (which allows
stacking drivers like dm-multipath to inherit 'max_sectors' from the
underlying paths).

blk_limits_max_hw_sectors() must allow stacking drivers to not have
max_sectors set to BLK_DEF_MAX_SECTORS as a side-effect.  Move that
historic constraint to blk_queue_max_hw_sectors().

Reported-by: Vijay Chauhan <vijay.chauhan@netapp.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
---
 block/blk-settings.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

v2: tweak blk_limits_max_hw_sectors and blk_queue_max_hw_sectors

diff --git a/block/blk-settings.c b/block/blk-settings.c
index d3234fc..b808dfd 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -143,8 +143,7 @@ void blk_set_stacking_limits(struct queue_limits *lim)
 	lim->discard_zeroes_data = 1;
 	lim->max_segments = USHRT_MAX;
 	lim->max_hw_sectors = UINT_MAX;
-
-	lim->max_sectors = BLK_DEF_MAX_SECTORS;
+	lim->max_sectors = UINT_MAX;
 }
 EXPORT_SYMBOL(blk_set_stacking_limits);
 
@@ -255,8 +254,7 @@ void blk_limits_max_hw_sectors(struct queue_limits *limits, unsigned int max_hw_
 	}
 
 	limits->max_hw_sectors = max_hw_sectors;
-	limits->max_sectors = min_t(unsigned int, max_hw_sectors,
-				    BLK_DEF_MAX_SECTORS);
+	limits->max_sectors = min_not_zero(limits->max_sectors, max_hw_sectors);
 }
 EXPORT_SYMBOL(blk_limits_max_hw_sectors);
 
@@ -271,6 +269,8 @@ EXPORT_SYMBOL(blk_limits_max_hw_sectors);
 void blk_queue_max_hw_sectors(struct request_queue *q, unsigned int max_hw_sectors)
 {
 	blk_limits_max_hw_sectors(&q->limits, max_hw_sectors);
+	q->limits.max_sectors = min_t(unsigned int, max_hw_sectors,
+				      BLK_DEF_MAX_SECTORS);
 }
 EXPORT_SYMBOL(blk_queue_max_hw_sectors);
 

Re: [PATCH v2] block: do not artificially constrain max_sectors for stacking drivers

[Mike Snitzer]

On Mon, Jul 09 2012 at 10:57am -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> blk_set_stacking_limits() is intended to allow stacking drivers to build
> up the limits of the stacked device based on the underlying devices'
> limits.  But in the case of 'max_sectors' the default of
> BLK_DEF_MAX_SECTORS (1024) doesn't allow the stacking driver to inherit
> a max_sectors larger than 1024.
> 
> It is now clear that this artificial limit is getting in the way so
> change blk_set_stacking_limits's max_sectors to UINT_MAX (which allows
> stacking drivers like dm-multipath to inherit 'max_sectors' from the
> underlying paths).
> 
> blk_limits_max_hw_sectors() must allow stacking drivers to not have
> max_sectors set to BLK_DEF_MAX_SECTORS as a side-effect.  Move that
> historic constraint to blk_queue_max_hw_sectors().
> 
> Reported-by: Vijay Chauhan <vijay.chauhan@netapp.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> ---
>  block/blk-settings.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> v2: tweak blk_limits_max_hw_sectors and blk_queue_max_hw_sectors

As it happens, v2's changes to blk_limits_max_hw_sectors and
blk_queue_max_hw_sectors are not strictly required in order for existing
stacking drivers to have have an unconstrained max_sectors.  Dropping
those changes also allows for consistency across both block functions.

So I'd be happy if v1 were to be staged for 3.6.  NetApp: it would be
great if you could confirm that v1 does in fact address the max_sectors
issue you reported.

Thanks,
Mike

RE: [PATCH v2] block: do not artificially constrain max_sectors for stacking drivers

[Chauhan, Vijay]

On Tuesday, July 10, 2012 4:27 AM, Mike Wrote:
>As it happens, v2's changes to blk_limits_max_hw_sectors and
>blk_queue_max_hw_sectors are not strictly required in order for existing
>stacking drivers to have have an unconstrained max_sectors.  Dropping
>those changes also allows for consistency across both block functions.
>
>So I'd be happy if v1 were to be staged for 3.6.  NetApp: it would be
>great if you could confirm that v1 does in fact address the max_sectors
>issue you reported.
>
>Thanks,
>Mike

Mike, Thanks for the quick fix. I verified with Patch v1 and it resolves this issue.

Thanks,
Vijay

Re: [PATCH v2] block: do not artificially constrain max_sectors for stacking drivers

[Mike Snitzer]

On Tue, Jul 10 2012 at  3:10pm -0400,
Chauhan, Vijay <Vijay.Chauhan@netapp.com> wrote:

> On Tuesday, July 10, 2012 4:27 AM, Mike Wrote:
> >As it happens, v2's changes to blk_limits_max_hw_sectors and
> >blk_queue_max_hw_sectors are not strictly required in order for existing
> >stacking drivers to have have an unconstrained max_sectors.  Dropping
> >those changes also allows for consistency across both block functions.
> >
> >So I'd be happy if v1 were to be staged for 3.6.  NetApp: it would be
> >great if you could confirm that v1 does in fact address the max_sectors
> >issue you reported.
> >
> >Thanks,
> >Mike
> 
> Mike, Thanks for the quick fix. I verified with Patch v1 and it resolves this issue.

Great, thanks for testing.  I assume Jens will be OK with staging v1 of
this patch for 3.6 once he gets back from vacation.

Jens please feel free to add the following to v1's patch header:

Tested-by: Vijay Chauhan <vijay.chauhan@netapp.com>

[RESEND PATCH] block: do not artificially constrain max_sectors for stacking drivers

[Mike Snitzer]

blk_set_stacking_limits is intended to allow stacking drivers to build
up the limits of the stacked device based on the underlying devices'
limits.  But defaulting 'max_sectors' to BLK_DEF_MAX_SECTORS (1024)
doesn't allow the stacking driver to inherit a max_sectors larger than
1024 -- due to blk_stack_limits' use of min_not_zero.

It is now clear that this artificial limit is getting in the way so
change blk_set_stacking_limits's max_sectors to UINT_MAX (which allows
stacking drivers like dm-multipath to inherit 'max_sectors' from the
underlying paths).

Reported-by: Vijay Chauhan <vijay.chauhan@netapp.com>
Tested-by: Vijay Chauhan <vijay.chauhan@netapp.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 block/blk-settings.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index d3234fc..565a678 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -143,8 +143,7 @@ void blk_set_stacking_limits(struct queue_limits *lim)
 	lim->discard_zeroes_data = 1;
 	lim->max_segments = USHRT_MAX;
 	lim->max_hw_sectors = UINT_MAX;
-
-	lim->max_sectors = BLK_DEF_MAX_SECTORS;
+	lim->max_sectors = UINT_MAX;
 }
 EXPORT_SYMBOL(blk_set_stacking_limits);
 
-- 
1.7.4.4

Re: [RESEND PATCH] block: do not artificially constrain max_sectors for stacking drivers

[Jens Axboe]

On 08/01/2012 02:39 AM, Mike Snitzer wrote:
> blk_set_stacking_limits is intended to allow stacking drivers to build
> up the limits of the stacked device based on the underlying devices'
> limits.  But defaulting 'max_sectors' to BLK_DEF_MAX_SECTORS (1024)
> doesn't allow the stacking driver to inherit a max_sectors larger than
> 1024 -- due to blk_stack_limits' use of min_not_zero.
> 
> It is now clear that this artificial limit is getting in the way so
> change blk_set_stacking_limits's max_sectors to UINT_MAX (which allows
> stacking drivers like dm-multipath to inherit 'max_sectors' from the
> underlying paths).

Thanks Mike (and Vijay), applied for 3.6.

-- 
Jens Axboe