[dm-devel] [PATCH 1/2] dm: update max_io_len to support a split_io that is not a power of 2

[Brassow Jonathan]

On Apr 30, 2012, at 1:59 PM, Mike Snitzer wrote:

>>> 
>>> 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?

split_io is used by dm-raid to split I/O on 'region_size' and 'chunk_size' boundaries.  The former is used for bitmap purposes for the write intent log (RAID 1/4/5/6).  The later is used by striping targets (RAID 4/5/6).  There are some sanity checks.  They must  be power of 2, region_size must be larger than 'chunk_size', etc.  However, I don't see any enforcement on the size of those numbers.  There probably should be, given that 'chunk_sectors'/'stripe_sectors' are 32-bit 'int' types.  Additionally, 'chunksize' (the name given for 'region_size' in MD) is a 32-bit type on disk and 'unsigned long' in memory - with no checking for size when the in-memory version is stored on disk.

I think these checks should probably be added, but there would be a limitation on the size of arrays imposed, I think.  Neil mentioned that the maximum size of a bitmap was (1 << 21) bits.  If the max 'region_size' is (1 << 32), then the max size device is (1 << 21) * (1 << 32) * (1 << 9) = 4 EiB.  I don't think that's a problem.

 brassow

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20120501/f995a9b9/attachment.htm>