Re: [PATCH 2/2] fat (exportfs): reconnect file handles to evictedinodes/dentries

[Steven J. Magnani]

On Tue, 2012-07-10 at 01:10 +0900, OGAWA Hirofumi wrote: 
> "Steven J. Magnani" <steve@xxxxxxxxxxxxxxx> writes:
> 
> > On Mon, 2012-07-09 at 22:43 +0900, OGAWA Hirofumi wrote: 
> >> "Steven J. Magnani" <steve@xxxxxxxxxxxxxxx> writes:
> >> 
> >> >> We need the key, possible key is - if it is only directory, FAT may be
> >> >> able to use i_start as additional search key.
> >> >
> >> > Interesting idea. I think this, and reformulating the FAT NFS file
> >> > handle to include the parent's i_ino, will greatly simplify (and speed
> >> > up) the code.
> >> 
> >> Does it work even if the inode was rename()'ed?
> >
> > AFAICT. I don't see why it wouldn't; on a rename, the inode's i_pos
> > changes but its i_ino stays the same, right?
> 
> If the inode is not on cache anymore, is there the possibility that
> selects the wrong parent? IIRC, NFS Server can be rebooted at any time
> while the client using the same file handle.

True, but it's looking like we can just use the default handle
constructed by export_encode_fh(), namely (i_ino, i_generation,
parent->i_ino, parent->i_generation). None of those components should
change in a server reboot.

Also, my thinking now is that there's no reliable way to reconstruct
evicted inodes. I was going to drop that portion of the patch, and stick
to fixing reconnection of cached inodes to dentries. Clients who are
sensitive to ESTALE should mount with subtree_check (so that parent
information is included in the NFS file handles, increasing our ability
to reconnect), and either handle ESTALE at the application level, or via
patches such as Jeff Layton's series
(https://lkml.org/lkml/2012/6/29/381 - I will be testing this shortly).

> > Do you have any objection to making the use of a directory logstart
> > cache a mount option that defaults to off? It seems a shame to penalize
> > everyone - particularly embedded systems - with the overhead of such a
> > cache when FAT-backed-NFS seems to be such a small percentage of use
> > cases.
> 
> I'm not sure what did it mean. It means to remove i_logstart from NFS
> file handle?

I am proposing to remove i_logstart from the file handle but what I was
asking here is whether the population of the new logstart index you've
proposed could be optional.

Maybe it's time to post a new spin of the patch so we are all talking
about the same concrete thing.
------------------------------------------------------------------------
Steven J. Magnani               "I claim this network for MARS!
www.digidescorp.com              Earthling, return my space modulator!"

#include <standard.disclaimer>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/