[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]