Re: [08/17] Define functions for page cache handling

Christoph Lameter
2007-04-25T06:27:49Z

On Wed, 25 Apr 2007, Eric Dumazet wrote:

> > struct radix_tree_root page_tree; /* radix tree of all pages */
> > rwlock_t tree_lock; /* and rwlock protecting it */
> > +#ifdef CONFIG_LARGE_BLOCKSIZE
> > + unsigned int order; /* Page order of the pages in
> > here */
> > + unsigned int shift; /* Shift of index */
> > + loff_t offset_mask; /* Mask to get to offset bits
> > */
> > +#endif
> > unsigned int i_mmap_writable;/* count VM_SHARED mappings */
> > struct prio_tree_root i_mmap; /* tree of private and shared
> > mappings */
> > struct list_head i_mmap_nonlinear;/*list VM_NONLINEAR mappings
> > */
>
> Some comments about this :
>
> 1) not optimal placement on 64bits arches (it creates one hole before
> i_mmap_writeable)

loff_t is a 64 bit long. We could put shift behind offset_mask.

> 2) sizeof(struct address_space) is an issue, given it is included in every
> inode, even sockets or pipes.

Right. I had an earlier implementation that just used an order field. See
V2 of this patchset. That did a lot of shifting instead of lookups.

> -> 2.1) Do we really need 32 bits for 'order' and 'shift'

No. u8 would be sufficient. If you can stuff in other stuff in between
then I can switch it.

> -> 2.2) Do we really need a 64bits offset_mask since it's a trivial arithmetic
> from 'shift' (or 'order') ?

It simplifies the generated asm code significantly.

> BTW, I presume splice() is not yet supported ? If yes, each pipe could hold 16
> 'big pages' so we probably can hit some DOS condition sooner...

Not yet. I'd be glad if you could come up with a patch that converts it.