On Fri, 1 May 2015 03:03:24 +0100 Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> On Fri, May 01, 2015 at 11:53:26AM +1000, NeilBrown wrote:
> > While writing that I began to wonder if lookup_one_len is really the right
> > interface to be used, even though it was introduced (in 2.3.99pre2-4)
> > specifically for nfsd.
> > The problem is that it assumes things about the filesystem. So it makes
> > perfect sense for various filesystems to use it on themselves, but I'm not
> > sure how *right* it is for nfsd (or cachefiles etc) to use it on some
> > *other* filesystem.
> > The particular issue is that it avoids the d_revalidate call.
> > Both vfat and reiserfs have that call ... I wonder if that could ever be a
> > problem.
> >
> > So I'm really leaning towards creating a variant of kern_path_mountpoint and
> > using a variant of that which takes a length.
>
> NAK. As in, "no way in hell". And yes, lookup_one_len() *does* revalidate -
> RTFS(lookup_dcache), please.
Damn - I always seems to get lost when I'm following those call paths.
lookup_one_len -> __lookup_hash -> lookup_dcache -> d_lookup,d_revalidate -> __d_lookup
-> lookup_real -> i_op->lookup
I think I was confusing __lookup_hash with __d_lookup in my thoughts.
>
> What kind of consistency warranties do callers expect, BTW? You do realize
> that between iterate_dir() and callbacks an entry might have been removed
> and/or replaced?
For READDIR_PLUS, lookup_one_len is called on each name and it requires
i_mutex, so the code currently holds i_mutex over the whole sequence.
This is triggering a deadlock.
We could just grab/drop i_mutex over each call to lookup_one_len(), but that
sort of thing is usually frowned upon, and we don't really always *need*
i_mutex if the lookup can be served from the d_cache.
So I'm looking for the best way to perform the lookup without holding i_mutex
for too long.
It sounds like you are suggesting something like lookup_one_len_unlocked(),
which .... uhm...
I was going to say uses lookup_dcache, but that needs i_mutex.
It calls d_lookup(), which doesn't seem to really need i_mutex, and
d_revalidate().
Does the later need i_mutex? I don't think so.
So maybe it is just how d_lookup handles failure that needs i_mutex.
So lookup_one_len_unlocked() could call d_lookup and d_revalidate and if
that all worked nicely, return the result. If it didn't, grab i_mutex and try
again??
Or do we just wear the cost of taking i_mutex for each name in the directory
during READDIR_PLUS?
Thanks,
NeilBrown
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
pgplceAMNOq1G.pgp
Description: OpenPGP digital signature
