Messages in this thread Patch in this message | | | Date | Fri, 19 Aug 2005 22:14:30 -0400 | From | Dave Johnson <> | Subject | Re: [PATCH] fix cramfs making duplicate entries in inode cache |
| |
Dave Johnson writes: > Phillip Lougher writes: > > Doesn't iget_locked() assume inode numbers are unique? > > > > In Cramfs inode numbers are set to 1 for non-data inodes (fifos, > > sockets, devices, empty directories), i.e > > > > %stat device namedpipe > > File: `device' > > Size: 0 Blocks: 0 IO Block: 4096 character > > special file > > Device: 700h/1792d Inode: 1 Links: 1 Device type: 1,1 > > Access: (0644/crw-r--r--) Uid: ( 0/ root) Gid: ( 0/ root) > > Access: 1970-01-01 01:00:00.000000000 +0100 > > Modify: 1970-01-01 01:00:00.000000000 +0100 > > Change: 1970-01-01 01:00:00.000000000 +0100 > > File: `namedpipe' > > Size: 0 Blocks: 0 IO Block: 4096 fifo > > Device: 700h/1792d Inode: 1 Links: 1 > > Access: (0644/prw-r--r--) Uid: ( 0/ root) Gid: ( 0/ root) > > Access: 1970-01-01 01:00:00.000000000 +0100 > > Modify: 1970-01-01 01:00:00.000000000 +0100 > > Change: 1970-01-01 01:00:00.000000000 +0100 > > > > Should iget5_locked() be used here? > > Yep, that was busted. Below patch should be better.
Doh, 2nd attempt wasn't verifying the inode number in the test function to protect against hash collisions (iget_locked does this but iget5_locked doesn't)
It should be good now.
-- Dave Johnson Starent Networks
===== fs/cramfs/inode.c 1.42 vs edited ===== --- 1.42/fs/cramfs/inode.c 2005-07-14 12:24:48 -04:00 +++ edited/fs/cramfs/inode.c 2005-08-19 22:06:44 -04:00 @@ -42,12 +42,46 @@ #define CRAMINO(x) ((x)->offset?(x)->offset<<2:1) #define OFFSET(x) ((x)->i_ino) + +static int cramfs_iget5_test(struct inode *inode, void *opaque) +{ + struct cramfs_inode * cramfs_inode = (struct cramfs_inode *)opaque; + + if (inode->i_ino != CRAMINO(cramfs_inode)) + return 0; /* does not match */ + + if (inode->i_ino != 1) + return 1; + + /* all empty directories, char, block, pipe, and sock, share inode #1 */ + + if ((inode->i_mode != cramfs_inode->mode) || + (inode->i_gid != cramfs_inode->gid) || + (inode->i_uid != cramfs_inode->uid)) + return 0; /* does not match */ + + if ((S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) && + (inode->i_rdev != old_decode_dev(cramfs_inode->size))) + return 0; /* does not match */ + + return 1; /* matches */ +} + +static int cramfs_iget5_set(struct inode *inode, void *opaque) +{ + struct cramfs_inode * cramfs_inode = (struct cramfs_inode *)opaque; + inode->i_ino = CRAMINO(cramfs_inode); + return 0; +} + static struct inode *get_cramfs_inode(struct super_block *sb, struct cramfs_inode * cramfs_inode) { - struct inode * inode = new_inode(sb); + struct inode * inode = iget5_locked(sb, CRAMINO(cramfs_inode), + cramfs_iget5_test, cramfs_iget5_set, + cramfs_inode); static struct timespec zerotime; - if (inode) { + if (inode && (inode->i_state & I_NEW)) { inode->i_mode = cramfs_inode->mode; inode->i_uid = cramfs_inode->uid; inode->i_size = cramfs_inode->size; @@ -63,7 +99,6 @@ but it's the best we can do without reading the directory contents. 1 yields the right result in GNU find, even without -noleaf option. */ - insert_inode_hash(inode); if (S_ISREG(inode->i_mode)) { inode->i_fop = &generic_ro_fops; inode->i_data.a_ops = &cramfs_aops; @@ -75,6 +110,7 @@ init_special_inode(inode, inode->i_mode, old_decode_dev(cramfs_inode->size)); } + unlock_new_inode(inode); } return inode; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |