[PATCH] Make sure QUEUE_FLAG_CLUSTER is set properly for dm.

[PATCH] Make sure QUEUE_FLAG_CLUSTER is set properly for dm.

[Neil Brown]

Hi,
 the block layer has a flag - QUEUE_FLAG_CLUSTER - which can be set
 (well, can be cleared, it defaults to set) by device drivers to
 encourage clustering of blocks in a bio - the presence of the flag
 essentially allows bios to be made bigger.

 Currently this flag is not set for dm (or md) devices, so bios are
 not made a big as they could be in some cases.

 The following patch sets the flag in the dm device, if all of the
 underlying devices have it set.

NeilBrown



------------

This flag should be set for a virtual device iff it is set
for all underlying devices.

Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/md/dm-table.c         |    9 +++++++++
 ./include/linux/device-mapper.h |    1 +
 2 files changed, 10 insertions(+)

diff ./drivers/md/dm-table.c~current~ ./drivers/md/dm-table.c
--- ./drivers/md/dm-table.c~current~	2005-12-07 14:07:56.000000000 +1100
+++ ./drivers/md/dm-table.c	2005-12-07 14:14:46.000000000 +1100
@@ -97,6 +97,8 @@ static void combine_restrictions_low(str
 
 	lhs->seg_boundary_mask =
 		min_not_zero(lhs->seg_boundary_mask, rhs->seg_boundary_mask);
+
+	lhs->no_cluster |= rhs->no_cluster;
 }
 
 /*
@@ -525,6 +527,8 @@ int dm_get_device(struct dm_target *ti, 
 		rs->seg_boundary_mask =
 			min_not_zero(rs->seg_boundary_mask,
 				     q->seg_boundary_mask);
+
+		rs->no_cluster |= !test_bit(QUEUE_FLAG_CLUSTER, &q->queue_flags);
 	}
 
 	return r;
@@ -834,6 +838,11 @@ void dm_table_set_restrictions(struct dm
 	q->hardsect_size = t->limits.hardsect_size;
 	q->max_segment_size = t->limits.max_segment_size;
 	q->seg_boundary_mask = t->limits.seg_boundary_mask;
+	if (t->limits.no_cluster)
+		q->queue_flags &= ~(1 << QUEUE_FLAG_CLUSTER);
+	else
+		q->queue_flags |= (1 << QUEUE_FLAG_CLUSTER);
+
 }
 
 unsigned int dm_table_get_num_targets(struct dm_table *t)

diff ./include/linux/device-mapper.h~current~ ./include/linux/device-mapper.h
--- ./include/linux/device-mapper.h~current~	2005-12-07 14:07:56.000000000 +1100
+++ ./include/linux/device-mapper.h	2005-12-07 14:14:46.000000000 +1100
@@ -97,6 +97,7 @@ struct io_restrictions {
 	unsigned short		hardsect_size;
 	unsigned int		max_segment_size;
 	unsigned long		seg_boundary_mask;
+	unsigned char		no_cluster; /* inverted so that 0 is default */
 };
 
 struct dm_target {

Re: [PATCH] Make sure QUEUE_FLAG_CLUSTER is set properly for dm.

[Alasdair G Kergon]

On Wed, Dec 07, 2005 at 02:21:58PM +1100, Neil Brown wrote:
>  the block layer has a flag - QUEUE_FLAG_CLUSTER - which can be set

Yes, but does it make much difference at the moment, given that we 
need to use small bios to avoid having to split them if they cross 
mapping boundaries?  (That's the real problem that needs tackling.)

Alasdair
-- 
agk@redhat.com

[PATCH] Make sure QUEUE_FLAG_CLUSTER is set properly for dm.

[NeilBrown]

This flag should be set for a virtual device iff it is set
for all underlying devices.

Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/md/dm-table.c         |    9 +++++++++
 ./include/linux/device-mapper.h |    1 +
 2 files changed, 10 insertions(+)

diff ./drivers/md/dm-table.c~current~ ./drivers/md/dm-table.c
--- ./drivers/md/dm-table.c~current~	2006-02-13 09:18:02.000000000 +1100
+++ ./drivers/md/dm-table.c	2006-02-13 09:18:02.000000000 +1100
@@ -98,6 +98,8 @@ static void combine_restrictions_low(str
 
 	lhs->seg_boundary_mask =
 		min_not_zero(lhs->seg_boundary_mask, rhs->seg_boundary_mask);
+
+	lhs->no_cluster |= rhs->no_cluster;
 }
 
 /*
@@ -526,6 +528,8 @@ int dm_get_device(struct dm_target *ti, 
 		rs->seg_boundary_mask =
 			min_not_zero(rs->seg_boundary_mask,
 				     q->seg_boundary_mask);
+
+		rs->no_cluster |= !test_bit(QUEUE_FLAG_CLUSTER, &q->queue_flags);
 	}
 
 	return r;
@@ -835,6 +839,11 @@ void dm_table_set_restrictions(struct dm
 	q->hardsect_size = t->limits.hardsect_size;
 	q->max_segment_size = t->limits.max_segment_size;
 	q->seg_boundary_mask = t->limits.seg_boundary_mask;
+	if (t->limits.no_cluster)
+		q->queue_flags &= ~(1 << QUEUE_FLAG_CLUSTER);
+	else
+		q->queue_flags |= (1 << QUEUE_FLAG_CLUSTER);
+
 }
 
 unsigned int dm_table_get_num_targets(struct dm_table *t)

diff ./include/linux/device-mapper.h~current~ ./include/linux/device-mapper.h
--- ./include/linux/device-mapper.h~current~	2006-02-13 09:18:02.000000000 +1100
+++ ./include/linux/device-mapper.h	2006-02-13 09:18:02.000000000 +1100
@@ -97,6 +97,7 @@ struct io_restrictions {
 	unsigned short		hardsect_size;
 	unsigned int		max_segment_size;
 	unsigned long		seg_boundary_mask;
+	unsigned char		no_cluster; /* inverted so that 0 is default */
 };
 
 struct dm_target {

Re: [PATCH] Make sure QUEUE_FLAG_CLUSTER is set properly for dm.

[Neil Brown]

On Wednesday December 7, agk@redhat.com wrote:
> On Wed, Dec 07, 2005 at 02:21:58PM +1100, Neil Brown wrote:
> >  the block layer has a flag - QUEUE_FLAG_CLUSTER - which can be set
> 
> Yes, but does it make much difference at the moment, given that we 
> need to use small bios to avoid having to split them if they cross 
> mapping boundaries?  (That's the real problem that needs tackling.)
> 

I'm sorry, I didn't see this back in December... I'm still not used to
these lists that set the 'reply-to' header, so I don't get a reply
explicitly addressed to me.... I've have to make my filters smarter!

Does DM *always* use small bios?  Even for multipath targets, where
presumably there would be no need even to limit the size?

If so, I guess this patch isn't going to make a big difference no, but
it would be worth having it in place for whenever handling larger bios
gets implemented.

Thanks,
NeilBrown