Re: [BK PATCH 2.5] Introduce 64-bit versions of PAGE_{CACHE_,}{MASK,ALIGN}

[Eric W. Biederman]

Andrew Morton <[e-mail]> writes:

> Anton Altaparmakov wrote:
> > 
> > Linus,
> > 
> > This patch introduces 64-bit versions of PAGE_{CACHE_,}MASK and
> > PAGE_{CACHE_,}ALIGN:
> >         PAGE_{CACHE_,}MASK_LL and PAGE_{CACHE_,}ALIGN_LL.
> > 
> > These are needed when 64-bit values are worked with on 32-bit
> > architectures, otherwise the high 32-bits are destroyed.
> > 
> > ...
> >  #define PAGE_SIZE      (1UL << PAGE_SHIFT)
> >  #define PAGE_MASK      (~(PAGE_SIZE-1))
> > +#define PAGE_MASK_LL   (~(u64)(PAGE_SIZE-1))
> 
> The problem here is that we've explicitly forced the
> PAGE_foo type to unsigned long.
> 
> If we instead take the "UL" out of PAGE_SIZE altogether,
> the compiler can then promote the type of PAGE_SIZE and PAGE_MASK
> to the widest type being used in the expression (ie: long long)
> and everything should work.
> 
> Which seems to be a much cleaner solution, if it works.
> 
> Will it work?

I don't quite see the point of this work.

There is exactly one operation that must be done in 64bit.
if (my64bitval > max) {
        return -E2BIG;
}
After that the value can be broken into, an index/offset pair.
Which is how the data is used in the page cache.

Eric