On Thu, Jul 31, 2008 at 9:00 PM, David Woodhouse <[email protected]> wrote:
> On Thu, 2008-07-31 at 20:53 -0400, Chuck Lever wrote:
>> For now it is sufficient, IMO. NFSv4 doesn't implement a readdirplus
>> operation, and the performance benefits of NFSv3 readdirplus are
>> equivocal -- there isn't a strong desire to replicate the complexity
>> of NFSv3 readdirplus in NFSv4. I'm not even sure you can do it even
>> with a single compound RPC, so even in the long run NFSv4 may not ever
>> have the locking issues that NFSv3 does here.
>
> AFAICT NFSv4 does have the same recursion issues already. The call trace
> goes fs->readdir() ... nfsd4_encode_dirent() ...
> nfsd4_encode_dirent_fattr() ... lookup_one_len() ... fs->lookup().
>
> Or am I mistaken?
It looks like it needs a directory entry's dentry for a couple of reasons:
1. To determine whether a directory entry is a mount point
2. If the client has asked for file handles (via a bitmask) for the
directory entries
So, yep, you're right. NFSv4 will hit this too.
As I recall, the Linux NFSv4 client doesn't use
FATTR4_WORD0_FILEHANDLE during NFSv4 READDIR for the reasons I stated
earlier; and it only uses FATTR4_WORD0_FSID during GETATTR. Other
clients might use these during a READDIR however.
--
"Alright guard, begin the unnecessarily slow-moving dipping mechanism."
--Dr. Evil
On Thu, 2008-07-31 at 21:31 -0400, Chuck Lever wrote:
> It looks like it needs a directory entry's dentry for a couple of
> reasons:
>
> 1. To determine whether a directory entry is a mount point
>
> 2. If the client has asked for file handles (via a bitmask) for the
> directory entries
Theoretically, neither of those actually need the _inode_. If it's a
mountpoint, won't it be in the dcache already? So a purely dcache-based
lookup would find it, without having to call through to the file
system's ->lookup() method on a dcache miss?
And to generate the file handle, all you need in the common case is
i_generation? You don't need to pull every inode into core.
--
dwmw2
On Thu, 2008-07-31 at 21:31 -0400, Chuck Lever wrote:
> On Thu, Jul 31, 2008 at 9:00 PM, David Woodhouse <[email protected]> wrote:
> > On Thu, 2008-07-31 at 20:53 -0400, Chuck Lever wrote:
> >> For now it is sufficient, IMO. NFSv4 doesn't implement a readdirplus
> >> operation, and the performance benefits of NFSv3 readdirplus are
> >> equivocal -- there isn't a strong desire to replicate the complexity
> >> of NFSv3 readdirplus in NFSv4. I'm not even sure you can do it even
> >> with a single compound RPC, so even in the long run NFSv4 may not ever
> >> have the locking issues that NFSv3 does here.
> >
> > AFAICT NFSv4 does have the same recursion issues already. The call trace
> > goes fs->readdir() ... nfsd4_encode_dirent() ...
> > nfsd4_encode_dirent_fattr() ... lookup_one_len() ... fs->lookup().
> >
> > Or am I mistaken?
>
> It looks like it needs a directory entry's dentry for a couple of reasons:
>
> 1. To determine whether a directory entry is a mount point
>
> 2. If the client has asked for file handles (via a bitmask) for the
> directory entries
Those are needed by NFSv3 too -- and can be handled with a lookup_fh()
method in the file system which is guaranteed to be called from within
the filldir callback, and some support in the VFS for checking if it's a
mountpoint.
NFSv4 introduces another problem though, which is that it seems to be
able to return the _full_ getattr() results for each object, and there's
no real way round the fact that we need to do the ->lookup() for that.
If sane clients aren't expected to _ask_ for that, though, then perhaps
it would be OK to fall back to something like the existing
readdir-to-buffer hack for that case, while most normal clients won't
trigger it.
--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation