Joel Becker wrote:
> On Sun, Aug 15, 2010 at 12:19:36PM -0500, Eric Sandeen wrote:
>>>> + (last_fs_block >
>>>> + (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
>>> ^^^ I don't get the pgoff_t check. Shouldn't it rather be
>>> (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
>> Argh that was my fault... Thankfully not too many 1k-blocksize-formatted
>> 16T devices out there, I guess.
>>
>> I went through the math again and also came up with:
>>
>> total fs pages is blocks / (blocks per page)
>> total pages is blocks / (1 << PAGE_CACHE_SHIFT / 1 << blocksize_bits)
>> total pages is blocks / (1 << (PAGE_CACHE_SHIFT - blocksize_bits))
>> total pages is blocks * (1 >> (PAGE_CACHE_SHIFT - blocksize_bits))
>> total pages is blocks >> (PAGE_CACHE_SHIFT - blocksize_bits)
>>
>> too big if total pages is > (pgoff_t)(~0ULL)
>> too big if blocks >> (PAGE_CACHE_SHIFT - blocksize_bits) > (pgoff_t)(~0ULL)
>
> Why not stop here, which is what I put in my other email?
> "blocks >> SHIFT-bits" is "how many pages do I need?".
yeah, ok. Was going for pointless symmetry w/ the other test...
>> too big if blocks > (pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
>> and to not overflow:
>> too big if blocks > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
>
> This still overflows. pgoff_t is a u64 on 64bit machines,
> right? So shift that left by anything and you wrap.
Er, yeah. I had 32 bits in my head since that's the case we're
checking for... whoops.
So I guess your
... ||
((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
(pgoff_t)(!0ULL))) {
is right :) (my feeble brain has a hard time reading that, though, TBH)
-Eric
> Joel
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
