<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Apr 30, 2012, at 1:59 PM, Mike Snitzer wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; "><div><blockquote type="cite"><blockquote type="cite"><br class="Apple-interchange-newline">I cannot see why we'd need a split_io that is larger than 32 bits -- a<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">32bit split_io can support up to 2TB (2**32 * 512b sectors). Even<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">on a LBD (raid) the stripe size (split_io) will not be so large.<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">But is that enforced in the raid code or not?<br></blockquote><br>No idea, need Jon to weigh in here. I'm hopeful we can impose 32bit<br>within dm-raid and coordinate with Neil on getting the appropriate MD<br>code (chunk_sectors) to reflect the reality that 32 bit is adequate.<br><br><blockquote type="cite"><blockquote type="cite">But what I think what you're driving at is:<span class="Apple-converted-space"> </span><br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">(I'm not convinced the proposed patch has been tested on 32-bit+LBD,<br></blockquote><blockquote type="cite">attempting to divide by a 64-bit number etc.)<br></blockquote><br>Right, it wasn't tested on 32bit. It'll fail to build due to split_io<br>being sector_t.<br><br><blockquote type="cite"><blockquote type="cite">is there a benefit/reason to<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">maintain the old code for some target that won't ever use non power of 2<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">split_io (e.g. dm-raid at the moment)? I see no point for the duality<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">in the code but I'm open to the idea if you have a specific reason in<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">mind -- are you concerned about perf on more obscure/older hardware?<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">EITHER the 32 bit split_io *must* be enforced (after we've convinced<br></blockquote><blockquote type="cite">ourselves 64 bits will never be required);<br></blockquote><blockquote type="cite">OR we keep it 64-bit and add some compat code.<br></blockquote><br>Yeap, I'm hopeful we can go with the former.<br><br>Jon, what do you think?</div></span></blockquote><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div> brassow</div><div><br></div></body></html>