Re: [NFS] [PATCH 001 of 8] knfsd: Add nfs-export support to tmpfs

[Hugh Dickins]

On Fri, 29 Sep 2006, Neil Brown wrote:
> On Thursday September 28, akpm@xxxxxxxx wrote:
> > 
> > Why don't other filesystems have the same problem?
> 
> Because most filesystems that hash their inodes do so at the point
> where the 'struct inode' is initialised, and that has suitable locking
> (I_NEW).  Here in shmem, we are hashing the inode later, the first
> time we need an NFS file handle for it.  We no longer have I_NEW to
> ensure only one thread tries to add it to the hash table.
> 
> The comment tries to explain this, but obviously isn't completely
> successful.

That makes sense.

This patch looks mostly good to me, thanks to David and you for it;
and my apologies to Atal and Gilad for never quite getting around
to looking at theirs properly (but the hashing in this new one does
look better than their linear search).  Though I know tmpfs, I
don't know NFS, so I'm glad this is coming from your direction.

But one anxiety, regarding i_ino.  That's an unsigned long originating
from last_inode in fs/inode.c, isn't it?  Here getting cast to a __u32
to make up one part of the file handle, which will then be cast back
to an unsigned long for ilookup later.

So on 64-bit arches, it'll only take one dedicated creator and
deleter of files to advance the last_inode count to the point where
the high int is non-zero, and shmem_get_dentry won't ever recognize
any inodes created thereafter, always reporting -ESTALE?  I'm on
unfamiliar territory, but it looks to me like we need another
__u32 in the file handle to cope with that?

Then there's the lesser but more tiresome problem, of i_ino collisions
on 32-bit arches.  tmpfs hasn't worried about that to date, just taken
whatever new_inode() has given it from last_inode: but perhaps these
file handles now make that more of a concern?

Hugh
-
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/