dm: update max_io_len to support a split_io that is not a power of 2
 

On Mon, Apr 30 2012 at 12:10pm -0400,
Alasdair G Kergon <agk@redhat.com> wrote:

> On Sat, Apr 28, 2012 at 12:44:28AM -0400, Mike Snitzer wrote:
> > Required to support a target's use of a non power of 2 blocksize.
>
> For which targets?

striped and thin-pool for starters.

> (merge_bvec supported?)

Yes.

> > + boundary = ti->split_io - do_div(tmp, ti->split_io);
>
> sector_div()?
>
> What about 32-bit arch + LBD + large split_io (from raid?)
> - Is a 32-bit restriction on split_io unreasonable nowadays?
> - OR reasonable on 32bit/LBD?
> - OR fallback to old code there?

I cannot see why we'd need a split_io that is larger than 32 bits -- a
32bit split_io can support up to 2TB (2**32 * 512b sectors). Even
on a LBD (raid) the stripe size (split_io) will not be so large.

(though yes we would need to establish a check in DM core that split_io is
limited to 32-bit -- even though the 'sector_t' is used for split_io;
and the comment inside the 'struct dm_target' would need updating).

But what I think what you're driving at is: is there a benefit/reason to
maintain the old code for some target that won't ever use non power of 2
split_io (e.g. dm-raid at the moment)? I see no point for the duality
in the code but I'm open to the idea if you have a specific reason in
mind -- are you concerned about perf on more obscure/older hardware?

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

dm: update max_io_len to support a split_io that is not a power of 2
 

On Mon, Apr 30 2012 at 2:36pm -0400,
Alasdair G Kergon <agk@redhat.com> wrote:

> On Mon, Apr 30, 2012 at 01:24:00PM -0400, Mike Snitzer wrote:
> > On Mon, Apr 30 2012 at 12:10pm -0400,
> > Alasdair G Kergon <agk@redhat.com> wrote:
> > > On Sat, Apr 28, 2012 at 12:44:28AM -0400, Mike Snitzer wrote:
> > > > Required to support a target's use of a non power of 2 blocksize.
> > > For which targets?
> > striped and thin-pool for starters.
> > > (merge_bvec supported?)
> > Yes.
>
> But there's overlap between merge_bvec and split_io.
> - Why does stripe_merge() have:
>
> if (!q->merge_bvec_fn)
> return max_size;
>
> when it's already done the division?
>
> - Couldn't that be changed to avoid split_io causing a split?
> (Except, as ever, across a table reload, which prevents us
> removing it completely.)
>
> > I cannot see why we'd need a split_io that is larger than 32 bits -- a
> > 32bit split_io can support up to 2TB (2**32 * 512b sectors). Even
> > on a LBD (raid) the stripe size (split_io) will not be so large.
>
> But is that enforced in the raid code or not?

No idea, need Jon to weigh in here. I'm hopeful we can impose 32bit
within dm-raid and coordinate with Neil on getting the appropriate MD
code (chunk_sectors) to reflect the reality that 32 bit is adequate.

> > But what I think what you're driving at is:
>
> (I'm not convinced the proposed patch has been tested on 32-bit+LBD,
> attempting to divide by a 64-bit number etc.)

Right, it wasn't tested on 32bit. It'll fail to build due to split_io
being sector_t.

> > is there a benefit/reason to
> > maintain the old code for some target that won't ever use non power of 2
> > split_io (e.g. dm-raid at the moment)? I see no point for the duality
> > in the code but I'm open to the idea if you have a specific reason in
> > mind -- are you concerned about perf on more obscure/older hardware?
>
> EITHER the 32 bit split_io *must* be enforced (after we've convinced
> ourselves 64 bits will never be required);
> OR we keep it 64-bit and add some compat code.

Yeap, I'm hopeful we can go with the former.

Jon, what do you think?

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel