* Multipath target does not shift block offset?
@ 2005-02-10 5:02 Tim Burgess
2005-02-10 17:18 ` Alasdair G Kergon
0 siblings, 1 reply; 6+ messages in thread
From: Tim Burgess @ 2005-02-10 5:02 UTC (permalink / raw)
To: dm-devel
Hi,
I've just been playing with the multipath dm target, and would like to
create a single device mapper device consisting of several,
concatenated, multipathed devices. The intuitive way to do this would be:
# HBA 2 failing to 3
0 286749488 multipath 2 round-robin 1 0 /dev/sdd round-robin 1 0 /dev/sdl
286749488 286749488 multipath 2 round-robin 1 0 /dev/sde round-robin 1 0
/dev/sdm
573498976 286749488 multipath 2 round-robin 1 0 /dev/sdf round-robin 1 0
/dev/sdn
<snip>
However, when one tries to set up such a device, we get the error message:
kernel: device-mapper: device /dev/sde too small for target
I would have assumed that the mapping specified above would map the
whole (sector range 0-286749487) of the sde/sdm device into the range
286749488-573498975 of the dm device. However, dm appears to be trying
to map the range 286749488-573498975 of the dm device to the same
offsets in the sde/sdm device.
Is this what was intended?
Cheers,
Tim
--------------------------------------------------------------------------
ANU Supercomputer Facility
tim.burgess@anu.edu.au and APAC National Facility
Phone: +61 2 6125 1431 Leonard Huxley Bldg (No. 56)
Fax: +61 2 6125 8199 Australian National University
Canberra, ACT, 0200, Australia
--------------------------------------------------------------------------
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Multipath target does not shift block offset?
2005-02-10 5:02 Multipath target does not shift block offset? Tim Burgess
@ 2005-02-10 17:18 ` Alasdair G Kergon
2005-02-10 19:23 ` Kevin Corry
0 siblings, 1 reply; 6+ messages in thread
From: Alasdair G Kergon @ 2005-02-10 17:18 UTC (permalink / raw)
To: device-mapper development
On Thu, Feb 10, 2005 at 04:02:28PM +1100, Tim Burgess wrote:
> However, dm appears to be trying
> to map the range 286749488-573498975 of the dm device to the same
> offsets in the sde/sdm device.
> Is this what was intended?
No.
In dm-mpath.c try adding to multipath_map() at the top of the function:
bio->bi_sector = (bio->bi_sector - ti->begin);
Alasdair
--
agk@redhat.com
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Multipath target does not shift block offset?
2005-02-10 17:18 ` Alasdair G Kergon
@ 2005-02-10 19:23 ` Kevin Corry
2005-02-11 18:29 ` Alasdair G Kergon
0 siblings, 1 reply; 6+ messages in thread
From: Kevin Corry @ 2005-02-10 19:23 UTC (permalink / raw)
To: dm-devel; +Cc: Alasdair G Kergon
On Thursday 10 February 2005 11:18 am, Alasdair G Kergon wrote:
> On Thu, Feb 10, 2005 at 04:02:28PM +1100, Tim Burgess wrote:
> > However, dm appears to be trying
> > to map the range 286749488-573498975 of the dm device to the same
> > offsets in the sde/sdm device.
> >
> > Is this what was intended?
>
> No.
>
> In dm-mpath.c try adding to multipath_map() at the top of the function:
>
> bio->bi_sector = (bio->bi_sector - ti->begin);
Actually, now that you point this out, I think this responsibility should
really be handled by the core driver's I/O path instead of each target
module. There's really no reason for the target modules to care or even
know about the presence of multiple targets within a device table. We can
move this line into the core's __map_bio() and get rid of a lot of
duplicate code. Here's a patch to demonstrate what I'm talking about.
--
Kevin Corry
kevcorry@us.ibm.com
http://evms.sourceforge.net/
--- diff/drivers/md/dm-crypt.c 2005-02-10 13:15:17.521210768 -0600
+++ source/drivers/md/dm-crypt.c 2005-02-10 13:05:48.847662288 -0600
@@ -808,7 +808,7 @@
struct convert_context ctx;
struct bio *clone;
unsigned int remaining = bio->bi_size;
- sector_t sector = bio->bi_sector - ti->begin;
+ sector_t sector = bio->bi_sector;
unsigned int bvec_idx = 0;
io->target = ti;
--- diff/drivers/md/dm-flakey.c 2005-02-10 13:15:17.471218368 -0600
+++ source/drivers/md/dm-flakey.c 2005-02-10 13:06:25.068155936 -0600
@@ -97,7 +97,7 @@
else {
bio->bi_bdev = f->dev->bdev;
- bio->bi_sector = f->start + (bio->bi_sector - ti->begin);
+ bio->bi_sector += f->start;
}
return 1;
--- diff/drivers/md/dm-linear.c 2005-02-10 13:15:17.557205296 -0600
+++ source/drivers/md/dm-linear.c 2005-02-10 13:07:53.098773248 -0600
@@ -71,7 +71,7 @@
struct linear_c *lc = (struct linear_c *) ti->private;
bio->bi_bdev = lc->dev->bdev;
- bio->bi_sector = lc->start + (bio->bi_sector - ti->begin);
+ bio->bi_sector += lc->start;
return 1;
}
--- diff/drivers/md/dm-raid1.c 2005-02-10 13:15:17.540207880 -0600
+++ source/drivers/md/dm-raid1.c 2005-02-10 13:09:39.481600600 -0600
@@ -687,7 +687,7 @@
static void map_bio(struct mirror_set *ms, struct mirror *m, struct bio *bio)
{
bio->bi_bdev = m->dev->bdev;
- bio->bi_sector = m->offset + (bio->bi_sector - ms->ti->begin);
+ bio->bi_sector += m->offset;
}
static void do_reads(struct mirror_set *ms, struct bio_list *reads)
--- diff/drivers/md/dm-stripe.c 2005-02-10 13:15:17.492215176 -0600
+++ source/drivers/md/dm-stripe.c 2005-02-10 13:13:50.842387952 -0600
@@ -172,13 +172,12 @@
{
struct stripe_c *sc = (struct stripe_c *) ti->private;
- sector_t offset = bio->bi_sector - ti->begin;
- sector_t chunk = offset >> sc->chunk_shift;
+ sector_t chunk = bio->bi_sector >> sc->chunk_shift;
uint32_t stripe = do_div(chunk, sc->stripes);
bio->bi_bdev = sc->stripe[stripe].dev->bdev;
bio->bi_sector = sc->stripe[stripe].physical_start +
- (chunk << sc->chunk_shift) + (offset & sc->chunk_mask);
+ (chunk << sc->chunk_shift) + (bio->bi_sector & sc->chunk_mask);
return 1;
}
--- diff/drivers/md/dm.c 2005-02-10 13:15:17.508212744 -0600
+++ source/drivers/md/dm.c 2005-02-10 13:04:25.069398520 -0600
@@ -366,6 +366,7 @@
* this io.
*/
atomic_inc(&tio->io->io_count);
+ clone->bi_sector -= ti->begin;
r = ti->type->map(ti, clone, &tio->info);
if (r > 0)
/* the bio has been remapped so dispatch it */
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Multipath target does not shift block offset?
2005-02-10 19:23 ` Kevin Corry
@ 2005-02-11 18:29 ` Alasdair G Kergon
0 siblings, 0 replies; 6+ messages in thread
From: Alasdair G Kergon @ 2005-02-11 18:29 UTC (permalink / raw)
To: Kevin Corry; +Cc: dm-devel
On Thu, Feb 10, 2005 at 01:23:30PM -0600, Kevin Corry wrote:
> Actually, now that you point this out, I think this responsibility should
> really be handled by the core driver's I/O path instead of each target
> module.
Absolutely - I didn't notice the bug before because I assumed it already
worked that way:-)
Alasdair
--
agk@redhat.com
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Multipath target does not shift block offset?
@ 2005-02-12 4:45 Tim Burgess
2005-02-16 16:02 ` Kevin Corry
0 siblings, 1 reply; 6+ messages in thread
From: Tim Burgess @ 2005-02-12 4:45 UTC (permalink / raw)
To: dm-devel
Re the patch from Kevin:
there looks like there is another reference to ti->begin in dm-raid1.c
that the patch does not remove (in do_write). I wasn't sure whether to
leave it there or not, since you were talking about making each target
unaware of its position within the overall mapped device...?
(note that my copy is not the latest - it's SUSE SLES SP1, so I
apologise if anything I say is not 100% true for the latest code :S)
Related:
I noticed that a similar collection of concatenated raid1 devices
(description below) was behaving strangely also, and splitting each
raid1 map into its own table fixed the problem...
For some reason, each of the mirror pairs was writing to its primary
leg, but only the first one listed in the file was writing to its second
leg... (note that this is before Kevin's patches - will try them in a
moment!).
> On Thursday 10 February 2005 11:18 am, Alasdair G Kergon wrote:
>> On Thu, Feb 10, 2005 at 04:02:28PM +1100, Tim Burgess wrote:
>> > However, dm appears to be trying
>> > to map the range 286749488-573498975 of the dm device to the same
>> > offsets in the sde/sdm device.
>> >
>> > Is this what was intended?
>>
>> No.
>>
>> In dm-mpath.c try adding to multipath_map() at the top of the function:
>>
>> bio->bi_sector = (bio->bi_sector - ti->begin);
>
> Actually, now that you point this out, I think this responsibility should
> really be handled by the core driver's I/O path instead of each target
> module. There's really no reason for the target modules to care or even
> know about the presence of multiple targets within a device table. We can
> move this line into the core's __map_bio() and get rid of a lot of
> duplicate code. Here's a patch to demonstrate what I'm talking about.
>
--
--------------------------------------------------------------------------
ANU Supercomputer Facility
tim.burgess@anu.edu.au and APAC National Facility
Phone: +61 2 6125 1431 Leonard Huxley Bldg (No. 56)
Fax: +61 2 6125 8199 Australian National University
Canberra, ACT, 0200, Australia
--------------------------------------------------------------------------
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Multipath target does not shift block offset?
2005-02-12 4:45 Tim Burgess
@ 2005-02-16 16:02 ` Kevin Corry
0 siblings, 0 replies; 6+ messages in thread
From: Kevin Corry @ 2005-02-16 16:02 UTC (permalink / raw)
To: dm-devel
On Fri February 11 2005 10:45 pm, Tim Burgess wrote:
> Re the patch from Kevin:
>
> there looks like there is another reference to ti->begin in dm-raid1.c
> that the patch does not remove (in do_write). I wasn't sure whether to
> leave it there or not, since you were talking about making each target
> unaware of its position within the overall mapped device...?
Oops. You're right, there are some other occurrances of ti->begin in the
target modules that I missed in my previous patch. I accidentally only
reviewing the mapping code. But dm-crypt and dm-raid1 each have code in their
workqueue routines that used that field as well. Here's a new version of last
week's patch with the two extra fixes.
And now that I think about it, we may want to make a similar patch for the 2.4
version of DM. I'll try to write one up a little later.
--
Kevin Corry
kevcorry@us.ibm.com
http://www.ibm.com/linux/ltc/
http://evms.sourceforge.net/
--- diff/drivers/md/dm-crypt.c 2005-02-10 13:15:17.000000000 -0600
+++ source/drivers/md/dm-crypt.c 2005-02-16 09:51:42.211358152 -0600
@@ -476,7 +476,7 @@
int r;
crypt_convert_init(cc, &ctx, io->bio, io->bio,
- io->bio->bi_sector - io->target->begin, 0);
+ io->bio->bi_sector, 0);
r = crypt_convert(cc, &ctx);
dec_pending(io, r);
@@ -808,7 +808,7 @@
struct convert_context ctx;
struct bio *clone;
unsigned int remaining = bio->bi_size;
- sector_t sector = bio->bi_sector - ti->begin;
+ sector_t sector = bio->bi_sector;
unsigned int bvec_idx = 0;
io->target = ti;
--- diff/drivers/md/dm-flakey.c 2005-02-10 13:15:17.000000000 -0600
+++ source/drivers/md/dm-flakey.c 2005-02-10 13:06:25.000000000 -0600
@@ -97,7 +97,7 @@
else {
bio->bi_bdev = f->dev->bdev;
- bio->bi_sector = f->start + (bio->bi_sector - ti->begin);
+ bio->bi_sector += f->start;
}
return 1;
--- diff/drivers/md/dm-linear.c 2005-02-10 13:15:17.000000000 -0600
+++ source/drivers/md/dm-linear.c 2005-02-10 13:07:53.000000000 -0600
@@ -71,7 +71,7 @@
struct linear_c *lc = (struct linear_c *) ti->private;
bio->bi_bdev = lc->dev->bdev;
- bio->bi_sector = lc->start + (bio->bi_sector - ti->begin);
+ bio->bi_sector += lc->start;
return 1;
}
--- diff/drivers/md/dm-raid1.c 2005-02-10 13:15:17.000000000 -0600
+++ source/drivers/md/dm-raid1.c 2005-02-16 09:52:51.626805400 -0600
@@ -687,7 +687,7 @@
static void map_bio(struct mirror_set *ms, struct mirror *m, struct bio *bio)
{
bio->bi_bdev = m->dev->bdev;
- bio->bi_sector = m->offset + (bio->bi_sector - ms->ti->begin);
+ bio->bi_sector += m->offset;
}
static void do_reads(struct mirror_set *ms, struct bio_list *reads)
@@ -764,7 +764,7 @@
m = ms->mirror + i;
io[i].bdev = m->dev->bdev;
- io[i].sector = m->offset + (bio->bi_sector - ms->ti->begin);
+ io[i].sector = m->offset + bio->bi_sector;
io[i].count = bio->bi_size >> 9;
}
--- diff/drivers/md/dm-stripe.c 2005-02-10 13:15:17.000000000 -0600
+++ source/drivers/md/dm-stripe.c 2005-02-10 13:13:50.000000000 -0600
@@ -172,13 +172,12 @@
{
struct stripe_c *sc = (struct stripe_c *) ti->private;
- sector_t offset = bio->bi_sector - ti->begin;
- sector_t chunk = offset >> sc->chunk_shift;
+ sector_t chunk = bio->bi_sector >> sc->chunk_shift;
uint32_t stripe = do_div(chunk, sc->stripes);
bio->bi_bdev = sc->stripe[stripe].dev->bdev;
bio->bi_sector = sc->stripe[stripe].physical_start +
- (chunk << sc->chunk_shift) + (offset & sc->chunk_mask);
+ (chunk << sc->chunk_shift) + (bio->bi_sector & sc->chunk_mask);
return 1;
}
--- diff/drivers/md/dm.c 2005-02-10 13:15:17.000000000 -0600
+++ source/drivers/md/dm.c 2005-02-10 13:04:25.000000000 -0600
@@ -366,6 +366,7 @@
* this io.
*/
atomic_inc(&tio->io->io_count);
+ clone->bi_sector -= ti->begin;
r = ti->type->map(ti, clone, &tio->info);
if (r > 0)
/* the bio has been remapped so dispatch it */
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2005-02-16 16:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-10 5:02 Multipath target does not shift block offset? Tim Burgess
2005-02-10 17:18 ` Alasdair G Kergon
2005-02-10 19:23 ` Kevin Corry
2005-02-11 18:29 ` Alasdair G Kergon
-- strict thread matches above, loose matches on Subject: below --
2005-02-12 4:45 Tim Burgess
2005-02-16 16:02 ` Kevin Corry
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.