Re: part 3 - Re: icreate comments
From: Jan Harkes <jaharkes@cs.cmu.edu>
Date: 2002-05-07 02:02:43
On Tue, May 07, 2002 at 02:26:59AM +0100, Anton Altaparmakov wrote:
quoted hunk ↗ jump to hunk
diff -urN iget_locked-5a/fs/inode.c iget_locked-6a/fs/inode.c--- iget_locked-5a/fs/inode.c Mon May 6 16:42:54 2002 +++ iget_locked-6a/fs/inode.c Mon May 6 16:46:36 2002@@ -557,7 +557,7 @@ if (test) old = find_inode(sb, head, test, data); else { - unsigned long ino = *(unsigned long *)data; + unsigned long ino = *(unsigned long)data;Looks like the * went missing in action in the unsigned long type cast... -6b doesn't have this typo...
Keeping multiple variants in sync gets to be a bit hard. Al's initial suggestion was I believe the 'b' variant, and you prefer it as well. Merged your comments, which seemed good ones to me. Changed the naming again. Guess what, I'm too much of a pushover, __iget_locked is now iget5_locked.
I would vote for the "b" series, patches 1-5 to go in. I don't see any benefits emerging from 6 but I am probably just missing the point, so I am indifferent about it...
patch-6 is more to drive home the fact that a filesystem lacking anything useable for i_ino can still use iget_locked (and therefore the inode cache) as long as it has 'some' way of identifying it's own inodes with 'test' and 'set'. i.e. it could simply use '0' as the hashval and although the inode chains would be longer, everything will work fine. Really the only reason to still have an i_ino exposed in the VFS is because stat returns it by pulling it off the inode instead of asking the filesystem for the right value. (And for find_inode_fast because it's faster to do a quick local test instead of calling a function to check 4 bytes of an inode). Jan