Re: [PATCH] PAG support only

[David Howells]

Muli Ben-Yehuda wrote:

> > + * VFS session authentication token cache
> ...
>
> If you know the type of the data, why keep it all in one binary blob,
> instead of a struct? cache effects? 

No. We _don't_ know the type of the data. A filesystem entrusts us with a
token to keep in the PAG on its behalf. However, since this is meant to be a
generic mechanism, it's entirely dependent on the fs as to what's in the blob.

> > +		if (tsk->vfspag->pag &&
> 
> Isn't the check here meant to be against tsk->vfspag? If not, there
> should be. 

Good point. Fixed.

> If vfs_pag_next wraps around, you will get two pag's with the value
> '1'. 

Not so:

 +		else
 +			goto pag_exists;
 +	}
 +	goto insert_here;
 +
 +	/* we found a PAG of the same ID - walk the tree from that point
 +	 * looking for the next unused PAG */
 + pag_exists:

If we find the target PAG already present, we keep incrementing the counter
(making sure it doesn't go -ve or 0) and searching from the current node in
the tree until we find a gap in the currently instantiated PAG number space.

We can make use of the properties of rb trees to do this more efficiently than
just incrementing and repeating the search every time. rb_next() will return
the node with next highest instantiated PAG number (or NULL when it reaches
the end of the tree).

> Nothing in this patch appears to be using it. You aren't taking a
> reference to the token here, what's protecting it from disappearing
> after the return and before the caller gets a chance to do something
> with it?

Thanks. Fixed.

David