* [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-18 16:24 ` Jun'ichi Nomura
0 siblings, 0 replies; 30+ messages in thread
From: Jun'ichi Nomura @ 2009-09-18 16:24 UTC (permalink / raw)
To: Jens Axboe, Mike Snitzer, Martin K. Petersen, Alasdair G Kergon
Cc: device-mapper development, linux-kernel
This patch is a preparation for the last patch in this patchset
which changes blk_set_default_limits() to set 0 to max_sectors.
dm uses blk_stack_limits() to merge limits of underlying devices
and copy the end result to the queue.
But if there's no underlying device (like 'zero' target),
max_sectors/max_hw_sectors are left unchanged from the default 0
and just copying it to the queue causes problems.
Provide blk_queue_copy_limits() to get a safe copy with
invalid values fixed-up.
Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Cc: David Strand <dpstrand@gmail.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
block/blk-settings.c | 28 ++++++++++++++++++++++++++++
include/linux/blkdev.h | 1 +
2 files changed, 29 insertions(+)
Index: linux-2.6.31.work/block/blk-settings.c
===================================================================
--- linux-2.6.31.work.orig/block/blk-settings.c
+++ linux-2.6.31.work/block/blk-settings.c
@@ -122,6 +122,34 @@ void blk_set_default_limits(struct queue
EXPORT_SYMBOL(blk_set_default_limits);
/**
+ * blk_queue_copy_limits - copy limits to queue
+ * @q: the request queue whose limits as a copy destination
+ * @lim: the queue_limits structure as a copy source
+ *
+ * Description:
+ * Copies a queue_limit struct contents to @q with fix-ups to
+ * invalid values.
+ */
+void blk_queue_copy_limits(struct request_queue *q, struct queue_limits
*lim)
+{
+ q->limits = *lim;
+
+ /*
+ * blk_set_default_limits() sets 0 to max_sectors/max_hw_sectors
+ * so that blk_stack_limits() appropriately propagate the values
+ * of lower-stack by min_not_zero().
+ * However, if the default value 0 is unchanged (e.g. the stacking
+ * device is virtual and has no underlying device), it results
+ * in unusable device.
+ * Check if max_sectors/max_hw_sectors have non-zero values,
+ * and set SAFE_MAX_SECTORS if they do.
+ */
+ if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
+ blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
+}
+EXPORT_SYMBOL(blk_queue_copy_limits);
+
+/**
* blk_queue_make_request - define an alternate make_request function
for a device
* @q: the request queue for the device to be affected
* @mfn: the alternate make_request function
Index: linux-2.6.31.work/include/linux/blkdev.h
===================================================================
--- linux-2.6.31.work.orig/include/linux/blkdev.h
+++ linux-2.6.31.work/include/linux/blkdev.h
@@ -917,6 +917,7 @@ extern void blk_limits_io_min(struct que
extern void blk_queue_io_min(struct request_queue *q, unsigned int min);
extern void blk_queue_io_opt(struct request_queue *q, unsigned int opt);
extern void blk_set_default_limits(struct queue_limits *lim);
+extern void blk_queue_copy_limits(struct request_queue *q, struct
queue_limits *lim);
extern int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
sector_t offset);
extern void disk_stack_limits(struct gendisk *disk, struct block_device
*bdev,
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 2/3] dm: Use blk_queue_copy_limits()
2009-09-18 16:24 ` Jun'ichi Nomura
@ 2009-09-18 16:26 ` Jun'ichi Nomura
-1 siblings, 0 replies; 30+ messages in thread
From: Jun'ichi Nomura @ 2009-09-18 16:26 UTC (permalink / raw)
To: Jens Axboe, Mike Snitzer, Martin K. Petersen, Alasdair G Kergon
Cc: David Strand, device-mapper development, linux-kernel
Use new blk_queue_copy_limits() so that invalid limits
(max_sectors == 0) are fixed up appropriately when copied to the queue.
Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Reported-by: David Strand <dpstrand@gmail.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
drivers/md/dm-table.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.31.work/drivers/md/dm-table.c
===================================================================
--- linux-2.6.31.work.orig/drivers/md/dm-table.c
+++ linux-2.6.31.work/drivers/md/dm-table.c
@@ -1090,7 +1090,7 @@ void dm_table_set_restrictions(struct dm
/*
* Copy table's limits to the DM device's request_queue
*/
- q->limits = *limits;
+ blk_queue_copy_limits(q, limits);
if (limits->no_cluster)
queue_flag_clear_unlocked(QUEUE_FLAG_CLUSTER, q);
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2/3] dm: Use blk_queue_copy_limits()
@ 2009-09-18 16:26 ` Jun'ichi Nomura
0 siblings, 0 replies; 30+ messages in thread
From: Jun'ichi Nomura @ 2009-09-18 16:26 UTC (permalink / raw)
To: Jens Axboe, Mike Snitzer, Martin K. Petersen, Alasdair G Kergon
Cc: device-mapper development, linux-kernel
Use new blk_queue_copy_limits() so that invalid limits
(max_sectors == 0) are fixed up appropriately when copied to the queue.
Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Reported-by: David Strand <dpstrand@gmail.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
drivers/md/dm-table.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.31.work/drivers/md/dm-table.c
===================================================================
--- linux-2.6.31.work.orig/drivers/md/dm-table.c
+++ linux-2.6.31.work/drivers/md/dm-table.c
@@ -1090,7 +1090,7 @@ void dm_table_set_restrictions(struct dm
/*
* Copy table's limits to the DM device's request_queue
*/
- q->limits = *limits;
+ blk_queue_copy_limits(q, limits);
if (limits->no_cluster)
queue_flag_clear_unlocked(QUEUE_FLAG_CLUSTER, q);
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/3] dm: Use blk_queue_copy_limits()
2009-09-18 16:26 ` Jun'ichi Nomura
@ 2009-09-18 19:11 ` Mike Snitzer
-1 siblings, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2009-09-18 19:11 UTC (permalink / raw)
To: Jun'ichi Nomura
Cc: Jens Axboe, Martin K. Petersen, Alasdair G Kergon, David Strand,
device-mapper development, linux-kernel
On Fri, Sep 18 2009 at 12:26pm -0400,
Jun'ichi Nomura <j-nomura@ce.jp.nec.com> wrote:
> Use new blk_queue_copy_limits() so that invalid limits
> (max_sectors == 0) are fixed up appropriately when copied to the queue.
>
> Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
> Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
> Reported-by: David Strand <dpstrand@gmail.com>
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: Alasdair G Kergon <agk@redhat.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> ---
> drivers/md/dm-table.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-2.6.31.work/drivers/md/dm-table.c
> ===================================================================
> --- linux-2.6.31.work.orig/drivers/md/dm-table.c
> +++ linux-2.6.31.work/drivers/md/dm-table.c
> @@ -1090,7 +1090,7 @@ void dm_table_set_restrictions(struct dm
> /*
> * Copy table's limits to the DM device's request_queue
> */
> - q->limits = *limits;
> + blk_queue_copy_limits(q, limits);
>
> if (limits->no_cluster)
> queue_flag_clear_unlocked(QUEUE_FLAG_CLUSTER, q);
How about just having 2 patches in the series and folding the above DM
change into the first patch?
Doesn't _really_ matter I guess...
Anyway, these minor points aside: I like what you've done with this
series.
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/3] dm: Use blk_queue_copy_limits()
@ 2009-09-18 19:11 ` Mike Snitzer
0 siblings, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2009-09-18 19:11 UTC (permalink / raw)
To: Jun'ichi Nomura
Cc: Martin K. Petersen, linux-kernel, device-mapper development,
Jens Axboe, Alasdair G Kergon
On Fri, Sep 18 2009 at 12:26pm -0400,
Jun'ichi Nomura <j-nomura@ce.jp.nec.com> wrote:
> Use new blk_queue_copy_limits() so that invalid limits
> (max_sectors == 0) are fixed up appropriately when copied to the queue.
>
> Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
> Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
> Reported-by: David Strand <dpstrand@gmail.com>
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: Alasdair G Kergon <agk@redhat.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> ---
> drivers/md/dm-table.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-2.6.31.work/drivers/md/dm-table.c
> ===================================================================
> --- linux-2.6.31.work.orig/drivers/md/dm-table.c
> +++ linux-2.6.31.work/drivers/md/dm-table.c
> @@ -1090,7 +1090,7 @@ void dm_table_set_restrictions(struct dm
> /*
> * Copy table's limits to the DM device's request_queue
> */
> - q->limits = *limits;
> + blk_queue_copy_limits(q, limits);
>
> if (limits->no_cluster)
> queue_flag_clear_unlocked(QUEUE_FLAG_CLUSTER, q);
How about just having 2 patches in the series and folding the above DM
change into the first patch?
Doesn't _really_ matter I guess...
Anyway, these minor points aside: I like what you've done with this
series.
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 3/3] block: blk_set_default_limits sets 0 to max_sectors
2009-09-18 16:24 ` Jun'ichi Nomura
@ 2009-09-18 16:29 ` Jun'ichi Nomura
-1 siblings, 0 replies; 30+ messages in thread
From: Jun'ichi Nomura @ 2009-09-18 16:29 UTC (permalink / raw)
To: Jens Axboe, Mike Snitzer, Martin K. Petersen, Alasdair G Kergon
Cc: David Strand, device-mapper development, linux-kernel
max_sectors and max_hw_sectors of dm device are set to smaller values
than those of underlying devices. E.g:
# cat /sys/block/sdj/queue/max_sectors_kb
512
# cat /sys/block/sdj/queue/max_hw_sectors_kb
32767
# echo "0 10 linear /dev/sdj 0" | dmsetup create test
# cat /sys/block/dm-0/queue/max_sectors_kb
127
# cat /sys/block/dm-0/queue/max_hw_sectors_kb
127
This prevents the I/O size of struct request from becoming large,
and causes undesired request fragmentation in request-based dm.
This is caused by the queue_limits stacking.
In dm_calculate_queue_limits(), the block-layer's safe default value
(SAFE_MAX_SECTORS, 255) is included in the merging process of target's
queue_limits. So underlying queue_limits is not propagated correctly.
Initialize default values of all max_sectors to '0'
in blk_set_default_limits() so that the values propagate properly
from underlying devices.
Check this thread for further background:
https://www.redhat.com/archives/dm-devel/2009-September/msg00176.html
Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Reported-by: David Strand <dpstrand@gmail.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
block/blk-settings.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6.31.work/block/blk-settings.c
===================================================================
--- linux-2.6.31.work.orig/block/blk-settings.c
+++ linux-2.6.31.work/block/blk-settings.c
@@ -111,7 +111,7 @@ void blk_set_default_limits(struct queue
lim->max_hw_segments = MAX_HW_SEGMENTS;
lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
lim->max_segment_size = MAX_SEGMENT_SIZE;
- lim->max_sectors = lim->max_hw_sectors = SAFE_MAX_SECTORS;
+ lim->max_sectors = lim->max_hw_sectors = 0;
lim->logical_block_size = lim->physical_block_size = lim->io_min = 512;
lim->bounce_pfn = (unsigned long)(BLK_BOUNCE_ANY >> PAGE_SHIFT);
lim->alignment_offset = 0;
@@ -192,6 +192,7 @@ void blk_queue_make_request(struct reque
q->unplug_timer.data = (unsigned long)q;
blk_set_default_limits(&q->limits);
+ blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
/*
* If the caller didn't supply a lock, fall back to our embedded
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 3/3] block: blk_set_default_limits sets 0 to max_sectors
@ 2009-09-18 16:29 ` Jun'ichi Nomura
0 siblings, 0 replies; 30+ messages in thread
From: Jun'ichi Nomura @ 2009-09-18 16:29 UTC (permalink / raw)
To: Jens Axboe, Mike Snitzer, Martin K. Petersen, Alasdair G Kergon
Cc: device-mapper development, linux-kernel
max_sectors and max_hw_sectors of dm device are set to smaller values
than those of underlying devices. E.g:
# cat /sys/block/sdj/queue/max_sectors_kb
512
# cat /sys/block/sdj/queue/max_hw_sectors_kb
32767
# echo "0 10 linear /dev/sdj 0" | dmsetup create test
# cat /sys/block/dm-0/queue/max_sectors_kb
127
# cat /sys/block/dm-0/queue/max_hw_sectors_kb
127
This prevents the I/O size of struct request from becoming large,
and causes undesired request fragmentation in request-based dm.
This is caused by the queue_limits stacking.
In dm_calculate_queue_limits(), the block-layer's safe default value
(SAFE_MAX_SECTORS, 255) is included in the merging process of target's
queue_limits. So underlying queue_limits is not propagated correctly.
Initialize default values of all max_sectors to '0'
in blk_set_default_limits() so that the values propagate properly
from underlying devices.
Check this thread for further background:
https://www.redhat.com/archives/dm-devel/2009-September/msg00176.html
Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Reported-by: David Strand <dpstrand@gmail.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
block/blk-settings.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6.31.work/block/blk-settings.c
===================================================================
--- linux-2.6.31.work.orig/block/blk-settings.c
+++ linux-2.6.31.work/block/blk-settings.c
@@ -111,7 +111,7 @@ void blk_set_default_limits(struct queue
lim->max_hw_segments = MAX_HW_SEGMENTS;
lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
lim->max_segment_size = MAX_SEGMENT_SIZE;
- lim->max_sectors = lim->max_hw_sectors = SAFE_MAX_SECTORS;
+ lim->max_sectors = lim->max_hw_sectors = 0;
lim->logical_block_size = lim->physical_block_size = lim->io_min = 512;
lim->bounce_pfn = (unsigned long)(BLK_BOUNCE_ANY >> PAGE_SHIFT);
lim->alignment_offset = 0;
@@ -192,6 +192,7 @@ void blk_queue_make_request(struct reque
q->unplug_timer.data = (unsigned long)q;
blk_set_default_limits(&q->limits);
+ blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
/*
* If the caller didn't supply a lock, fall back to our embedded
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
2009-09-18 16:24 ` Jun'ichi Nomura
@ 2009-09-18 19:07 ` Mike Snitzer
-1 siblings, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2009-09-18 19:07 UTC (permalink / raw)
To: Jun'ichi Nomura
Cc: Jens Axboe, Martin K. Petersen, Alasdair G Kergon, David Strand,
device-mapper development, linux-kernel
On Fri, Sep 18 2009 at 12:24pm -0400,
Jun'ichi Nomura <j-nomura@ce.jp.nec.com> wrote:
> This patch is a preparation for the last patch in this patchset
> which changes blk_set_default_limits() to set 0 to max_sectors.
should read: changes blk_set_default_limits() to set max_sectors to 0.
> dm uses blk_stack_limits() to merge limits of underlying devices
> and copy the end result to the queue.
> But if there's no underlying device (like 'zero' target),
> max_sectors/max_hw_sectors are left unchanged from the default 0
> and just copying it to the queue causes problems.
>
> Provide blk_queue_copy_limits() to get a safe copy with
> invalid values fixed-up.
>
> Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
> Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
> Cc: David Strand <dpstrand@gmail.com>
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: Alasdair G Kergon <agk@redhat.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> ---
> block/blk-settings.c | 28 ++++++++++++++++++++++++++++
> include/linux/blkdev.h | 1 +
> 2 files changed, 29 insertions(+)
>
> Index: linux-2.6.31.work/block/blk-settings.c
> ===================================================================
> --- linux-2.6.31.work.orig/block/blk-settings.c
> +++ linux-2.6.31.work/block/blk-settings.c
> @@ -122,6 +122,34 @@ void blk_set_default_limits(struct queue
> EXPORT_SYMBOL(blk_set_default_limits);
>
> /**
> + * blk_queue_copy_limits - copy limits to queue
> + * @q: the request queue whose limits as a copy destination
> + * @lim: the queue_limits structure as a copy source
> + *
> + * Description:
> + * Copies a queue_limit struct contents to @q with fix-ups to
> + * invalid values.
> + */
> +void blk_queue_copy_limits(struct request_queue *q, struct queue_limits
> *lim)
> +{
> + q->limits = *lim;
> +
> + /*
> + * blk_set_default_limits() sets 0 to max_sectors/max_hw_sectors
> + * so that blk_stack_limits() appropriately propagate the values
> + * of lower-stack by min_not_zero().
> + * However, if the default value 0 is unchanged (e.g. the stacking
> + * device is virtual and has no underlying device), it results
> + * in unusable device.
Likewise:
blk_set_default_limits() sets max_sectors/max_hw_sectors to 0?
> + * Check if max_sectors/max_hw_sectors have non-zero values,
> + * and set SAFE_MAX_SECTORS if they do.
> + */
> + if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
> + blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
Shouldn't this check (and the entire comment above it) get added in the
3rd patch once max_sectors/max_hw_sectors sre actually set to 0? I'm
being really pedantic here but...
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-18 19:07 ` Mike Snitzer
0 siblings, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2009-09-18 19:07 UTC (permalink / raw)
To: Jun'ichi Nomura
Cc: Martin K. Petersen, linux-kernel, device-mapper development,
Jens Axboe, Alasdair G Kergon
On Fri, Sep 18 2009 at 12:24pm -0400,
Jun'ichi Nomura <j-nomura@ce.jp.nec.com> wrote:
> This patch is a preparation for the last patch in this patchset
> which changes blk_set_default_limits() to set 0 to max_sectors.
should read: changes blk_set_default_limits() to set max_sectors to 0.
> dm uses blk_stack_limits() to merge limits of underlying devices
> and copy the end result to the queue.
> But if there's no underlying device (like 'zero' target),
> max_sectors/max_hw_sectors are left unchanged from the default 0
> and just copying it to the queue causes problems.
>
> Provide blk_queue_copy_limits() to get a safe copy with
> invalid values fixed-up.
>
> Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
> Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
> Cc: David Strand <dpstrand@gmail.com>
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: Alasdair G Kergon <agk@redhat.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> ---
> block/blk-settings.c | 28 ++++++++++++++++++++++++++++
> include/linux/blkdev.h | 1 +
> 2 files changed, 29 insertions(+)
>
> Index: linux-2.6.31.work/block/blk-settings.c
> ===================================================================
> --- linux-2.6.31.work.orig/block/blk-settings.c
> +++ linux-2.6.31.work/block/blk-settings.c
> @@ -122,6 +122,34 @@ void blk_set_default_limits(struct queue
> EXPORT_SYMBOL(blk_set_default_limits);
>
> /**
> + * blk_queue_copy_limits - copy limits to queue
> + * @q: the request queue whose limits as a copy destination
> + * @lim: the queue_limits structure as a copy source
> + *
> + * Description:
> + * Copies a queue_limit struct contents to @q with fix-ups to
> + * invalid values.
> + */
> +void blk_queue_copy_limits(struct request_queue *q, struct queue_limits
> *lim)
> +{
> + q->limits = *lim;
> +
> + /*
> + * blk_set_default_limits() sets 0 to max_sectors/max_hw_sectors
> + * so that blk_stack_limits() appropriately propagate the values
> + * of lower-stack by min_not_zero().
> + * However, if the default value 0 is unchanged (e.g. the stacking
> + * device is virtual and has no underlying device), it results
> + * in unusable device.
Likewise:
blk_set_default_limits() sets max_sectors/max_hw_sectors to 0?
> + * Check if max_sectors/max_hw_sectors have non-zero values,
> + * and set SAFE_MAX_SECTORS if they do.
> + */
> + if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
> + blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
Shouldn't this check (and the entire comment above it) get added in the
3rd patch once max_sectors/max_hw_sectors sre actually set to 0? I'm
being really pedantic here but...
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dm-devel] [PATCH 1/3] block: Add blk_queue_copy_limits()
2009-09-18 16:24 ` Jun'ichi Nomura
@ 2009-09-18 19:28 ` Martin K. Petersen
-1 siblings, 0 replies; 30+ messages in thread
From: Martin K. Petersen @ 2009-09-18 19:28 UTC (permalink / raw)
To: device-mapper development
Cc: Jens Axboe, Mike Snitzer, Martin K. Petersen, Alasdair G Kergon,
linux-kernel
>>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
+ if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
+ blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
I'm really not keen on perpetuating SAFE_MAX_SECTORS for something that
was written in this millennium.
I'd much rather we just do this, then:
block: Set max_sectors correctly for stacking devices
The topology changes unintentionally caused SAFE_MAX_SECTORS to be set
for stacking devices. Set the default limit to BLK_DEF_MAX_SECTORS and
provide SAFE_MAX_SECTORS in blk_queue_make_request() for legacy hw
drivers that depend on the old behavior.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 83413ff..cd9b730 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -111,7 +111,7 @@ void blk_set_default_limits(struct queue_limits *lim)
lim->max_hw_segments = MAX_HW_SEGMENTS;
lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
lim->max_segment_size = MAX_SEGMENT_SIZE;
- lim->max_sectors = lim->max_hw_sectors = SAFE_MAX_SECTORS;
+ lim->max_sectors = lim->max_hw_sectors = BLK_DEF_MAX_SECTORS;
lim->logical_block_size = lim->physical_block_size = lim->io_min = 512;
lim->bounce_pfn = (unsigned long)(BLK_BOUNCE_ANY >> PAGE_SHIFT);
lim->alignment_offset = 0;
@@ -164,6 +164,7 @@ void blk_queue_make_request(struct request_queue *q, make_request_fn *mfn)
q->unplug_timer.data = (unsigned long)q;
blk_set_default_limits(&q->limits);
+ blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
/*
* If the caller didn't supply a lock, fall back to our embedded
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-18 19:28 ` Martin K. Petersen
0 siblings, 0 replies; 30+ messages in thread
From: Martin K. Petersen @ 2009-09-18 19:28 UTC (permalink / raw)
To: device-mapper development
Cc: linux-kernel, Alasdair G Kergon, Martin K. Petersen,
Mike Snitzer, Jens Axboe
>>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
+ if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
+ blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
I'm really not keen on perpetuating SAFE_MAX_SECTORS for something that
was written in this millennium.
I'd much rather we just do this, then:
block: Set max_sectors correctly for stacking devices
The topology changes unintentionally caused SAFE_MAX_SECTORS to be set
for stacking devices. Set the default limit to BLK_DEF_MAX_SECTORS and
provide SAFE_MAX_SECTORS in blk_queue_make_request() for legacy hw
drivers that depend on the old behavior.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 83413ff..cd9b730 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -111,7 +111,7 @@ void blk_set_default_limits(struct queue_limits *lim)
lim->max_hw_segments = MAX_HW_SEGMENTS;
lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
lim->max_segment_size = MAX_SEGMENT_SIZE;
- lim->max_sectors = lim->max_hw_sectors = SAFE_MAX_SECTORS;
+ lim->max_sectors = lim->max_hw_sectors = BLK_DEF_MAX_SECTORS;
lim->logical_block_size = lim->physical_block_size = lim->io_min = 512;
lim->bounce_pfn = (unsigned long)(BLK_BOUNCE_ANY >> PAGE_SHIFT);
lim->alignment_offset = 0;
@@ -164,6 +164,7 @@ void blk_queue_make_request(struct request_queue *q, make_request_fn *mfn)
q->unplug_timer.data = (unsigned long)q;
blk_set_default_limits(&q->limits);
+ blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
/*
* If the caller didn't supply a lock, fall back to our embedded
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
2009-09-18 19:28 ` Martin K. Petersen
@ 2009-09-18 20:30 ` Mike Snitzer
-1 siblings, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2009-09-18 20:30 UTC (permalink / raw)
To: Martin K. Petersen
Cc: device-mapper development, Jens Axboe, Alasdair G Kergon, linux-kernel
On Fri, Sep 18 2009 at 3:28pm -0400,
Martin K. Petersen <martin.petersen@oracle.com> wrote:
> >>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
>
> + if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
> + blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
>
> I'm really not keen on perpetuating SAFE_MAX_SECTORS for something that
> was written in this millennium.
>
> I'd much rather we just do this, then:
>
> block: Set max_sectors correctly for stacking devices
>
> The topology changes unintentionally caused SAFE_MAX_SECTORS to be set
> for stacking devices. Set the default limit to BLK_DEF_MAX_SECTORS and
> provide SAFE_MAX_SECTORS in blk_queue_make_request() for legacy hw
> drivers that depend on the old behavior.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Nice. Avoids the need for a safe queue_limits copy and associated naunce.
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-18 20:30 ` Mike Snitzer
0 siblings, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2009-09-18 20:30 UTC (permalink / raw)
To: Martin K. Petersen
Cc: device-mapper development, linux-kernel, Alasdair G Kergon, Jens Axboe
On Fri, Sep 18 2009 at 3:28pm -0400,
Martin K. Petersen <martin.petersen@oracle.com> wrote:
> >>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
>
> + if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
> + blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
>
> I'm really not keen on perpetuating SAFE_MAX_SECTORS for something that
> was written in this millennium.
>
> I'd much rather we just do this, then:
>
> block: Set max_sectors correctly for stacking devices
>
> The topology changes unintentionally caused SAFE_MAX_SECTORS to be set
> for stacking devices. Set the default limit to BLK_DEF_MAX_SECTORS and
> provide SAFE_MAX_SECTORS in blk_queue_make_request() for legacy hw
> drivers that depend on the old behavior.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Nice. Avoids the need for a safe queue_limits copy and associated naunce.
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dm-devel] [PATCH 1/3] block: Add blk_queue_copy_limits()
2009-09-18 19:28 ` Martin K. Petersen
@ 2009-09-19 15:22 ` Jun'ichi Nomura
-1 siblings, 0 replies; 30+ messages in thread
From: Jun'ichi Nomura @ 2009-09-19 15:22 UTC (permalink / raw)
To: Martin K. Petersen
Cc: device-mapper development, linux-kernel, Alasdair G Kergon,
Mike Snitzer, Jens Axboe
Martin K. Petersen wrote:
>>>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
>
> + if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
> + blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
>
> I'm really not keen on perpetuating SAFE_MAX_SECTORS for something that
> was written in this millennium.
>
> I'd much rather we just do this, then:
>
> block: Set max_sectors correctly for stacking devices
>
> The topology changes unintentionally caused SAFE_MAX_SECTORS to be set
> for stacking devices. Set the default limit to BLK_DEF_MAX_SECTORS and
> provide SAFE_MAX_SECTORS in blk_queue_make_request() for legacy hw
> drivers that depend on the old behavior.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
>
> ---
>
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 83413ff..cd9b730 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -111,7 +111,7 @@ void blk_set_default_limits(struct queue_limits *lim)
> lim->max_hw_segments = MAX_HW_SEGMENTS;
> lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
> lim->max_segment_size = MAX_SEGMENT_SIZE;
> - lim->max_sectors = lim->max_hw_sectors = SAFE_MAX_SECTORS;
> + lim->max_sectors = lim->max_hw_sectors = BLK_DEF_MAX_SECTORS;
Umm, with this, BLK_DEF_MAX_SECTORS becomes upper bound of max_hw_sectors
and the values of underlying devices are not propagated to the stacking
devices.
Is it intended?
> lim->logical_block_size = lim->physical_block_size = lim->io_min = 512;
> lim->bounce_pfn = (unsigned long)(BLK_BOUNCE_ANY >> PAGE_SHIFT);
> lim->alignment_offset = 0;
> @@ -164,6 +164,7 @@ void blk_queue_make_request(struct request_queue *q, make_request_fn *mfn)
> q->unplug_timer.data = (unsigned long)q;
>
> blk_set_default_limits(&q->limits);
> + blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
>
> /*
> * If the caller didn't supply a lock, fall back to our embedded
Thanks,
--
Jun'ichi Nomura, NEC Corporation
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-19 15:22 ` Jun'ichi Nomura
0 siblings, 0 replies; 30+ messages in thread
From: Jun'ichi Nomura @ 2009-09-19 15:22 UTC (permalink / raw)
To: Martin K. Petersen
Cc: device-mapper development, Mike Snitzer, linux-kernel,
Alasdair G Kergon, Jens Axboe
Martin K. Petersen wrote:
>>>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
>
> + if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
> + blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
>
> I'm really not keen on perpetuating SAFE_MAX_SECTORS for something that
> was written in this millennium.
>
> I'd much rather we just do this, then:
>
> block: Set max_sectors correctly for stacking devices
>
> The topology changes unintentionally caused SAFE_MAX_SECTORS to be set
> for stacking devices. Set the default limit to BLK_DEF_MAX_SECTORS and
> provide SAFE_MAX_SECTORS in blk_queue_make_request() for legacy hw
> drivers that depend on the old behavior.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
>
> ---
>
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 83413ff..cd9b730 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -111,7 +111,7 @@ void blk_set_default_limits(struct queue_limits *lim)
> lim->max_hw_segments = MAX_HW_SEGMENTS;
> lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
> lim->max_segment_size = MAX_SEGMENT_SIZE;
> - lim->max_sectors = lim->max_hw_sectors = SAFE_MAX_SECTORS;
> + lim->max_sectors = lim->max_hw_sectors = BLK_DEF_MAX_SECTORS;
Umm, with this, BLK_DEF_MAX_SECTORS becomes upper bound of max_hw_sectors
and the values of underlying devices are not propagated to the stacking
devices.
Is it intended?
> lim->logical_block_size = lim->physical_block_size = lim->io_min = 512;
> lim->bounce_pfn = (unsigned long)(BLK_BOUNCE_ANY >> PAGE_SHIFT);
> lim->alignment_offset = 0;
> @@ -164,6 +164,7 @@ void blk_queue_make_request(struct request_queue *q, make_request_fn *mfn)
> q->unplug_timer.data = (unsigned long)q;
>
> blk_set_default_limits(&q->limits);
> + blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
>
> /*
> * If the caller didn't supply a lock, fall back to our embedded
Thanks,
--
Jun'ichi Nomura, NEC Corporation
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dm-devel] [PATCH 1/3] block: Add blk_queue_copy_limits()
2009-09-19 15:22 ` Jun'ichi Nomura
@ 2009-09-20 21:00 ` Martin K. Petersen
-1 siblings, 0 replies; 30+ messages in thread
From: Martin K. Petersen @ 2009-09-20 21:00 UTC (permalink / raw)
To: device-mapper development
Cc: Martin K. Petersen, Mike Snitzer, linux-kernel,
Alasdair G Kergon, Jens Axboe
>>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
Jun'ichi> Umm, with this, BLK_DEF_MAX_SECTORS becomes upper bound of
Jun'ichi> max_hw_sectors and the values of underlying devices are not
Jun'ichi> propagated to the stacking devices.
Well, max_sectors is already bounded by this. max_hw_sectors only
really matters for PC commands, so I'm not sure it's a big deal for
DM. But I guess we could set the default max_hw_sectors to -1.
I'm just trying to avoid these scattered if-0-set-it-to-something-else
cases. I'd much rather have the defaults do the right thing.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-20 21:00 ` Martin K. Petersen
0 siblings, 0 replies; 30+ messages in thread
From: Martin K. Petersen @ 2009-09-20 21:00 UTC (permalink / raw)
To: device-mapper development
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer, Martin K. Petersen,
linux-kernel
>>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
Jun'ichi> Umm, with this, BLK_DEF_MAX_SECTORS becomes upper bound of
Jun'ichi> max_hw_sectors and the values of underlying devices are not
Jun'ichi> propagated to the stacking devices.
Well, max_sectors is already bounded by this. max_hw_sectors only
really matters for PC commands, so I'm not sure it's a big deal for
DM. But I guess we could set the default max_hw_sectors to -1.
I'm just trying to avoid these scattered if-0-set-it-to-something-else
cases. I'd much rather have the defaults do the right thing.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dm-devel] [PATCH 1/3] block: Add blk_queue_copy_limits()
2009-09-20 21:00 ` Martin K. Petersen
@ 2009-09-21 16:33 ` Jun'ichi Nomura
-1 siblings, 0 replies; 30+ messages in thread
From: Jun'ichi Nomura @ 2009-09-21 16:33 UTC (permalink / raw)
To: Jens Axboe, Martin K. Petersen
Cc: device-mapper development, Alasdair G Kergon, Mike Snitzer, linux-kernel
Martin K. Petersen wrote:
>>>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
>
> Jun'ichi> Umm, with this, BLK_DEF_MAX_SECTORS becomes upper bound of
> Jun'ichi> max_hw_sectors and the values of underlying devices are not
> Jun'ichi> propagated to the stacking devices.
>
> Well, max_sectors is already bounded by this. max_hw_sectors only
> really matters for PC commands, so I'm not sure it's a big deal for
> DM. But I guess we could set the default max_hw_sectors to -1.
>
> I'm just trying to avoid these scattered if-0-set-it-to-something-else
> cases. I'd much rather have the defaults do the right thing.
I agree with that.
I had to do the if-0-set-it-to-something-else to avoid putting unnecessary
cap on max_hw_sectors.
If we aren't sure, shouldn't we set its default to -1 or putting comments
in blk_set_default_limits() at least to avoid possible confusion in future?
Thanks,
--
Jun'ichi Nomura, NEC Corporation
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-21 16:33 ` Jun'ichi Nomura
0 siblings, 0 replies; 30+ messages in thread
From: Jun'ichi Nomura @ 2009-09-21 16:33 UTC (permalink / raw)
To: Jens Axboe, Martin K. Petersen
Cc: device-mapper development, Mike Snitzer, Alasdair G Kergon, linux-kernel
Martin K. Petersen wrote:
>>>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
>
> Jun'ichi> Umm, with this, BLK_DEF_MAX_SECTORS becomes upper bound of
> Jun'ichi> max_hw_sectors and the values of underlying devices are not
> Jun'ichi> propagated to the stacking devices.
>
> Well, max_sectors is already bounded by this. max_hw_sectors only
> really matters for PC commands, so I'm not sure it's a big deal for
> DM. But I guess we could set the default max_hw_sectors to -1.
>
> I'm just trying to avoid these scattered if-0-set-it-to-something-else
> cases. I'd much rather have the defaults do the right thing.
I agree with that.
I had to do the if-0-set-it-to-something-else to avoid putting unnecessary
cap on max_hw_sectors.
If we aren't sure, shouldn't we set its default to -1 or putting comments
in blk_set_default_limits() at least to avoid possible confusion in future?
Thanks,
--
Jun'ichi Nomura, NEC Corporation
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dm-devel] [PATCH 1/3] block: Add blk_queue_copy_limits()
2009-09-21 16:33 ` Jun'ichi Nomura
@ 2009-09-21 19:43 ` Martin K. Petersen
-1 siblings, 0 replies; 30+ messages in thread
From: Martin K. Petersen @ 2009-09-21 19:43 UTC (permalink / raw)
To: device-mapper development
Cc: Jens Axboe, Martin K. Petersen, Mike Snitzer, Alasdair G Kergon,
linux-kernel
>>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
Jun'ichi> If we aren't sure, shouldn't we set its default to -1 or
Jun'ichi> putting comments in blk_set_default_limits() at least to avoid
Jun'ichi> possible confusion in future?
Jens, what do you think about this? Goes on top of what you have
queued...
block: Do not clamp max_hw_sectors for stacking devices
Stacking devices do not have an inherent max_hw_sector limit. Set the
default to INT_MAX so we are bounded only by capabilities of the
underlying storage.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
diff --git a/block/blk-settings.c b/block/blk-settings.c
index cd9b730..eaf122f 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -111,7 +111,8 @@ void blk_set_default_limits(struct queue_limits *lim)
lim->max_hw_segments = MAX_HW_SEGMENTS;
lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
lim->max_segment_size = MAX_SEGMENT_SIZE;
- lim->max_sectors = lim->max_hw_sectors = BLK_DEF_MAX_SECTORS;
+ lim->max_sectors = BLK_DEF_MAX_SECTORS;
+ lim->max_hw_sectors = INT_MAX;
lim->logical_block_size = lim->physical_block_size = lim->io_min = 512;
lim->bounce_pfn = (unsigned long)(BLK_BOUNCE_ANY >> PAGE_SHIFT);
lim->alignment_offset = 0;
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-21 19:43 ` Martin K. Petersen
0 siblings, 0 replies; 30+ messages in thread
From: Martin K. Petersen @ 2009-09-21 19:43 UTC (permalink / raw)
To: device-mapper development
Cc: linux-kernel, Alasdair G Kergon, Mike Snitzer,
Martin K. Petersen, Jens Axboe
>>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
Jun'ichi> If we aren't sure, shouldn't we set its default to -1 or
Jun'ichi> putting comments in blk_set_default_limits() at least to avoid
Jun'ichi> possible confusion in future?
Jens, what do you think about this? Goes on top of what you have
queued...
block: Do not clamp max_hw_sectors for stacking devices
Stacking devices do not have an inherent max_hw_sector limit. Set the
default to INT_MAX so we are bounded only by capabilities of the
underlying storage.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
diff --git a/block/blk-settings.c b/block/blk-settings.c
index cd9b730..eaf122f 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -111,7 +111,8 @@ void blk_set_default_limits(struct queue_limits *lim)
lim->max_hw_segments = MAX_HW_SEGMENTS;
lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
lim->max_segment_size = MAX_SEGMENT_SIZE;
- lim->max_sectors = lim->max_hw_sectors = BLK_DEF_MAX_SECTORS;
+ lim->max_sectors = BLK_DEF_MAX_SECTORS;
+ lim->max_hw_sectors = INT_MAX;
lim->logical_block_size = lim->physical_block_size = lim->io_min = 512;
lim->bounce_pfn = (unsigned long)(BLK_BOUNCE_ANY >> PAGE_SHIFT);
lim->alignment_offset = 0;
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [dm-devel] [PATCH 1/3] block: Add blk_queue_copy_limits()
2009-09-21 19:43 ` Martin K. Petersen
@ 2009-09-21 19:45 ` Jens Axboe
-1 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2009-09-21 19:45 UTC (permalink / raw)
To: Martin K. Petersen
Cc: device-mapper development, Mike Snitzer, Alasdair G Kergon, linux-kernel
On Mon, Sep 21 2009, Martin K. Petersen wrote:
> >>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
>
> Jun'ichi> If we aren't sure, shouldn't we set its default to -1 or
> Jun'ichi> putting comments in blk_set_default_limits() at least to avoid
> Jun'ichi> possible confusion in future?
>
> Jens, what do you think about this? Goes on top of what you have
> queued...
Look sane, applied.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-21 19:45 ` Jens Axboe
0 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2009-09-21 19:45 UTC (permalink / raw)
To: Martin K. Petersen
Cc: device-mapper development, Alasdair G Kergon, Mike Snitzer, linux-kernel
On Mon, Sep 21 2009, Martin K. Petersen wrote:
> >>>>> "Jun'ichi" == Jun'ichi Nomura <j-nomura@ce.jp.nec.com> writes:
>
> Jun'ichi> If we aren't sure, shouldn't we set its default to -1 or
> Jun'ichi> putting comments in blk_set_default_limits() at least to avoid
> Jun'ichi> possible confusion in future?
>
> Jens, what do you think about this? Goes on top of what you have
> queued...
Look sane, applied.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
2009-09-18 16:24 ` Jun'ichi Nomura
@ 2009-09-18 20:33 ` Jens Axboe
-1 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2009-09-18 20:33 UTC (permalink / raw)
To: Jun'ichi Nomura
Cc: Mike Snitzer, Martin K. Petersen, Alasdair G Kergon,
David Strand, device-mapper development, linux-kernel
On Sat, Sep 19 2009, Jun'ichi Nomura wrote:
> This patch is a preparation for the last patch in this patchset
> which changes blk_set_default_limits() to set 0 to max_sectors.
>
> dm uses blk_stack_limits() to merge limits of underlying devices
> and copy the end result to the queue.
> But if there's no underlying device (like 'zero' target),
> max_sectors/max_hw_sectors are left unchanged from the default 0
> and just copying it to the queue causes problems.
>
> Provide blk_queue_copy_limits() to get a safe copy with
> invalid values fixed-up.
Added for 2.6.32.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-18 20:33 ` Jens Axboe
0 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2009-09-18 20:33 UTC (permalink / raw)
To: Jun'ichi Nomura
Cc: Mike Snitzer, Martin K. Petersen, linux-kernel,
device-mapper development, Alasdair G Kergon
On Sat, Sep 19 2009, Jun'ichi Nomura wrote:
> This patch is a preparation for the last patch in this patchset
> which changes blk_set_default_limits() to set 0 to max_sectors.
>
> dm uses blk_stack_limits() to merge limits of underlying devices
> and copy the end result to the queue.
> But if there's no underlying device (like 'zero' target),
> max_sectors/max_hw_sectors are left unchanged from the default 0
> and just copying it to the queue causes problems.
>
> Provide blk_queue_copy_limits() to get a safe copy with
> invalid values fixed-up.
Added for 2.6.32.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
2009-09-18 20:33 ` Jens Axboe
@ 2009-09-18 20:35 ` Jens Axboe
-1 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2009-09-18 20:35 UTC (permalink / raw)
To: Jun'ichi Nomura
Cc: Mike Snitzer, Martin K. Petersen, Alasdair G Kergon,
David Strand, device-mapper development, linux-kernel
On Fri, Sep 18 2009, Jens Axboe wrote:
> On Sat, Sep 19 2009, Jun'ichi Nomura wrote:
> > This patch is a preparation for the last patch in this patchset
> > which changes blk_set_default_limits() to set 0 to max_sectors.
> >
> > dm uses blk_stack_limits() to merge limits of underlying devices
> > and copy the end result to the queue.
> > But if there's no underlying device (like 'zero' target),
> > max_sectors/max_hw_sectors are left unchanged from the default 0
> > and just copying it to the queue causes problems.
> >
> > Provide blk_queue_copy_limits() to get a safe copy with
> > invalid values fixed-up.
>
> Added for 2.6.32.
And removed, it's white space damaged and doesn't apply to the current
tree.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-18 20:35 ` Jens Axboe
0 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2009-09-18 20:35 UTC (permalink / raw)
To: Jun'ichi Nomura
Cc: Mike Snitzer, Martin K. Petersen, linux-kernel,
device-mapper development, Alasdair G Kergon
On Fri, Sep 18 2009, Jens Axboe wrote:
> On Sat, Sep 19 2009, Jun'ichi Nomura wrote:
> > This patch is a preparation for the last patch in this patchset
> > which changes blk_set_default_limits() to set 0 to max_sectors.
> >
> > dm uses blk_stack_limits() to merge limits of underlying devices
> > and copy the end result to the queue.
> > But if there's no underlying device (like 'zero' target),
> > max_sectors/max_hw_sectors are left unchanged from the default 0
> > and just copying it to the queue causes problems.
> >
> > Provide blk_queue_copy_limits() to get a safe copy with
> > invalid values fixed-up.
>
> Added for 2.6.32.
And removed, it's white space damaged and doesn't apply to the current
tree.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
2009-09-18 20:35 ` Jens Axboe
@ 2009-09-19 15:35 ` Jun'ichi Nomura
-1 siblings, 0 replies; 30+ messages in thread
From: Jun'ichi Nomura @ 2009-09-19 15:35 UTC (permalink / raw)
To: Jens Axboe
Cc: Mike Snitzer, Martin K. Petersen, Alasdair G Kergon,
David Strand, device-mapper development, linux-kernel
Jens Axboe wrote:
> On Fri, Sep 18 2009, Jens Axboe wrote:
>> On Sat, Sep 19 2009, Jun'ichi Nomura wrote:
>>> This patch is a preparation for the last patch in this patchset
>>> which changes blk_set_default_limits() to set 0 to max_sectors.
>>>
>>> dm uses blk_stack_limits() to merge limits of underlying devices
>>> and copy the end result to the queue.
>>> But if there's no underlying device (like 'zero' target),
>>> max_sectors/max_hw_sectors are left unchanged from the default 0
>>> and just copying it to the queue causes problems.
>>>
>>> Provide blk_queue_copy_limits() to get a safe copy with
>>> invalid values fixed-up.
>> Added for 2.6.32.
>
> And removed, it's white space damaged and doesn't apply to the current
> tree.
Sorry... I forgot to disable line-wrapping and broke the patch.
Re-sending with Mike's correction to English applied.
This patch is a preparation for the last patch in this patchset
which changes blk_set_default_limits() to set max_sectors to 0.
dm uses blk_stack_limits() to merge limits of underlying devices
and copy the end result to the queue.
But if there's no underlying device (like 'zero' target),
max_sectors/max_hw_sectors are left unchanged from the default 0
and just copying it to the queue causes problems.
Provide blk_queue_copy_limits() to get a safe copy with
invalid values fixed-up.
Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Cc: David Strand <dpstrand@gmail.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
block/blk-settings.c | 28 ++++++++++++++++++++++++++++
include/linux/blkdev.h | 1 +
2 files changed, 29 insertions(+)
Index: linux-2.6.31.work/block/blk-settings.c
===================================================================
--- linux-2.6.31.work.orig/block/blk-settings.c
+++ linux-2.6.31.work/block/blk-settings.c
@@ -122,6 +122,34 @@ void blk_set_default_limits(struct queue
EXPORT_SYMBOL(blk_set_default_limits);
/**
+ * blk_queue_copy_limits - copy limits to queue
+ * @q: the request queue whose limits as a copy destination
+ * @lim: the queue_limits structure as a copy source
+ *
+ * Description:
+ * Copies a queue_limit struct contents to @q with fix-ups to
+ * invalid values.
+ */
+void blk_queue_copy_limits(struct request_queue *q, struct queue_limits *lim)
+{
+ q->limits = *lim;
+
+ /*
+ * blk_set_default_limits() sets max_sectors/max_hw_sectors to 0
+ * so that blk_stack_limits() appropriately propagate the values
+ * of lower-stack by min_not_zero().
+ * However, if the default value 0 is unchanged (e.g. the stacking
+ * device is virtual and has no underlying device), it results
+ * in unusable device.
+ * Check if max_sectors/max_hw_sectors have non-zero values,
+ * and set SAFE_MAX_SECTORS if they do.
+ */
+ if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
+ blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
+}
+EXPORT_SYMBOL(blk_queue_copy_limits);
+
+/**
* blk_queue_make_request - define an alternate make_request function for a device
* @q: the request queue for the device to be affected
* @mfn: the alternate make_request function
Index: linux-2.6.31.work/include/linux/blkdev.h
===================================================================
--- linux-2.6.31.work.orig/include/linux/blkdev.h
+++ linux-2.6.31.work/include/linux/blkdev.h
@@ -917,6 +917,7 @@ extern void blk_limits_io_min(struct que
extern void blk_queue_io_min(struct request_queue *q, unsigned int min);
extern void blk_queue_io_opt(struct request_queue *q, unsigned int opt);
extern void blk_set_default_limits(struct queue_limits *lim);
+extern void blk_queue_copy_limits(struct request_queue *q, struct queue_limits *lim);
extern int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
sector_t offset);
extern void disk_stack_limits(struct gendisk *disk, struct block_device *bdev,
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 1/3] block: Add blk_queue_copy_limits()
@ 2009-09-19 15:35 ` Jun'ichi Nomura
0 siblings, 0 replies; 30+ messages in thread
From: Jun'ichi Nomura @ 2009-09-19 15:35 UTC (permalink / raw)
To: Jens Axboe
Cc: Mike Snitzer, Martin K. Petersen, linux-kernel,
device-mapper development, Alasdair G Kergon
Jens Axboe wrote:
> On Fri, Sep 18 2009, Jens Axboe wrote:
>> On Sat, Sep 19 2009, Jun'ichi Nomura wrote:
>>> This patch is a preparation for the last patch in this patchset
>>> which changes blk_set_default_limits() to set 0 to max_sectors.
>>>
>>> dm uses blk_stack_limits() to merge limits of underlying devices
>>> and copy the end result to the queue.
>>> But if there's no underlying device (like 'zero' target),
>>> max_sectors/max_hw_sectors are left unchanged from the default 0
>>> and just copying it to the queue causes problems.
>>>
>>> Provide blk_queue_copy_limits() to get a safe copy with
>>> invalid values fixed-up.
>> Added for 2.6.32.
>
> And removed, it's white space damaged and doesn't apply to the current
> tree.
Sorry... I forgot to disable line-wrapping and broke the patch.
Re-sending with Mike's correction to English applied.
This patch is a preparation for the last patch in this patchset
which changes blk_set_default_limits() to set max_sectors to 0.
dm uses blk_stack_limits() to merge limits of underlying devices
and copy the end result to the queue.
But if there's no underlying device (like 'zero' target),
max_sectors/max_hw_sectors are left unchanged from the default 0
and just copying it to the queue causes problems.
Provide blk_queue_copy_limits() to get a safe copy with
invalid values fixed-up.
Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Cc: David Strand <dpstrand@gmail.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
block/blk-settings.c | 28 ++++++++++++++++++++++++++++
include/linux/blkdev.h | 1 +
2 files changed, 29 insertions(+)
Index: linux-2.6.31.work/block/blk-settings.c
===================================================================
--- linux-2.6.31.work.orig/block/blk-settings.c
+++ linux-2.6.31.work/block/blk-settings.c
@@ -122,6 +122,34 @@ void blk_set_default_limits(struct queue
EXPORT_SYMBOL(blk_set_default_limits);
/**
+ * blk_queue_copy_limits - copy limits to queue
+ * @q: the request queue whose limits as a copy destination
+ * @lim: the queue_limits structure as a copy source
+ *
+ * Description:
+ * Copies a queue_limit struct contents to @q with fix-ups to
+ * invalid values.
+ */
+void blk_queue_copy_limits(struct request_queue *q, struct queue_limits *lim)
+{
+ q->limits = *lim;
+
+ /*
+ * blk_set_default_limits() sets max_sectors/max_hw_sectors to 0
+ * so that blk_stack_limits() appropriately propagate the values
+ * of lower-stack by min_not_zero().
+ * However, if the default value 0 is unchanged (e.g. the stacking
+ * device is virtual and has no underlying device), it results
+ * in unusable device.
+ * Check if max_sectors/max_hw_sectors have non-zero values,
+ * and set SAFE_MAX_SECTORS if they do.
+ */
+ if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
+ blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
+}
+EXPORT_SYMBOL(blk_queue_copy_limits);
+
+/**
* blk_queue_make_request - define an alternate make_request function for a device
* @q: the request queue for the device to be affected
* @mfn: the alternate make_request function
Index: linux-2.6.31.work/include/linux/blkdev.h
===================================================================
--- linux-2.6.31.work.orig/include/linux/blkdev.h
+++ linux-2.6.31.work/include/linux/blkdev.h
@@ -917,6 +917,7 @@ extern void blk_limits_io_min(struct que
extern void blk_queue_io_min(struct request_queue *q, unsigned int min);
extern void blk_queue_io_opt(struct request_queue *q, unsigned int opt);
extern void blk_set_default_limits(struct queue_limits *lim);
+extern void blk_queue_copy_limits(struct request_queue *q, struct queue_limits *lim);
extern int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
sector_t offset);
extern void disk_stack_limits(struct gendisk *disk, struct block_device *bdev,
^ permalink raw reply [flat|nested] 30+ messages in thread