Re: [PATCH] PAG support, try #2

[Christoph Hellwig <hch@infradead.org>]

On Wed, May 14, 2003 at 12:56:04PM +0100, David Howells wrote:
> > So do you have an estimate for the number of users here yet?
> > Adding two more slab caches that are unused for 99% of the users
> > might not be the best choice if there's no strong need.
> 
> There won't be many PAGs. Basically one per login session would be fairly
> typical, and possibly one per SUID program run at some later date.

I think using plain kmalloc is better then.

> > Inline but not static seems strange..
> 
> It's not without precedent within the kernel. The compiler is free to inline
> it, but must also emit an out-of-line copy. Thinking about it, it's probably
> not worth it... these calls aren't going to be called very often and so don't
> need to be optimised for every last ounce of speed.

Right, that's why I didn't complained very loud :)

> > We already discussed the coding style issue,
> 
> Well, the coding style is wrong here IMNSHO. Readability is preferable.

No.  Please follow the style guidelines.  If you say readability is
preferable you habve to say whos.  I always get stuck at code that
uses such strange constructs for example.

> > What protects vfspag->tokens?
> 
> Why does it need to be protected at that point? The PAG no longer has any
> references, and the tokens don't point back to it, and are in any case pinned
> by virtue of being on the list.

Okay.

> > Shouldn't vfs_pag_get hanlde a NULL argument instead?
> 
> Maybe. But then that's introducing a conditional branch that you can't avoid,
> even if you know it's going to succeed:-/

Then add an __vfs_pag_get that expects a non-NULL argument for those
cases where you know for sure it's not NULL.