On Wed, Oct 16, 2013 at 11:45:28AM +0300, Benny Halevy wrote: > I'm reluctant about the layout stateids as it uses common hashing data structs > locking infrastructure, and code with all other nfsd4 stateids. The layout stateids are another thing that confuses me in the current pnfs code. Depending on what we find the generic state id might be embedded into the layout stateid or they might be separate, but there don't seem to be clear rules on how to deal with freeing things in the later case. I haven't brought this up because I didn't dig deep into it enough, but it's for sure more than confusing. > We can have a similar hash table for pnfsd_inode similar to nfs4_file Yes, that was the plan. > Note that nfs4_file is also per inode, not per file as might be reflected from its name. > Maybe moving it nfs4state.c also warrants renaming it to something more accurate. Yes, it should have an inode instead of file in the name, and it really should be nfsd4_ prefix instead of using the client namespace. Given that it's open/locking specific I'm not even sure that name should be that generic. > And while there, change file_hashval to hash on i_ino rather than the inode ptr. That's a bad idea. i_ino is 32-bit only while many NFS-exported filesystems have 64-bit inode numbers. Hashing on kernel pointers genetrally is a fairly good idea, and we do it for the inode in other places, too. -- 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