On Thu 27-11-14 10:25:24, Ted Tso wrote: > This is what I'm currently playing with which I believe fixes the iput() > problem. In fs/ext4/inode.c: > > struct other_inode { > unsigned long orig_ino; > struct ext4_inode *raw_inode; > }; > static int other_inode_match(struct inode * inode, unsigned long ino, > void *data); > > /* > * Opportunistically update the other time fields for other inodes in > * the same inode table block. > */ > static void ext4_update_other_inodes_time(struct super_block *sb, > unsigned long orig_ino, char *buf) > { > struct other_inode oi; > unsigned long ino; > int i, inodes_per_block = EXT4_SB(sb)->s_inodes_per_block; > int inode_size = EXT4_INODE_SIZE(sb); > > oi.orig_ino = orig_ino; > ino = orig_ino & ~(inodes_per_block - 1); > for (i = 0; i < inodes_per_block; i++, ino++, buf += inode_size) { > if (ino == orig_ino) > continue; > oi.raw_inode = (struct ext4_inode *) buf; > (void) find_inode_nowait(sb, ino, other_inode_match, &oi); > } > } > > static int other_inode_match(struct inode * inode, unsigned long ino, > void *data) > { > struct other_inode *oi = (struct other_inode *) data; > > if ((inode->i_ino != ino) || > (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) || > ((inode->i_state & I_DIRTY_TIME) == 0)) > return 0; > spin_lock(&inode->i_lock); > if (((inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) == 0) && > (inode->i_state & I_DIRTY_TIME)) { > struct ext4_inode_info *ei = EXT4_I(inode); > > inode->i_state &= ~I_DIRTY_TIME; > inode->i_ts_dirty_day = 0; > spin_unlock(&inode->i_lock); > inode_requeue_dirtytime(inode); > > spin_lock(&ei->i_raw_lock); > EXT4_INODE_SET_XTIME(i_ctime, inode, oi->raw_inode); > EXT4_INODE_SET_XTIME(i_mtime, inode, oi->raw_inode); > EXT4_INODE_SET_XTIME(i_atime, inode, oi->raw_inode); > ext4_inode_csum_set(inode, oi->raw_inode, ei); > spin_unlock(&ei->i_raw_lock); > trace_ext4_other_inode_update_time(inode, oi->orig_ino); > return -1; > } > spin_unlock(&inode->i_lock); > return -1; > } Hum, but this puts lots of stuff under inode_hash_lock, including writeback list lock. I don't like this too much. I understand that getting handle for each inode is rather more CPU intensive but it should still be a clear win over the current situation and avoids entangling locks like this. Honza > The above uses the following in fs/inode.c (which gets added instead of > find_active_inode_nowait): > > /** > * find_inode_nowait - find an inode in the inode cache > * @sb: super block of file system to search > * @hashval: hash value (usually inode number) to search for > * @match: callback used for comparisons between inodes > * @data: opaque data pointer to pass to @match > * > * Search for the inode specified by @hashval and @data in the inode > * cache, where the helper function @match will return 0 if the inode > * does not match, 1 if the inode does match, and -1 if the search > * should be stopped. The @match function must be responsible for > * taking the i_lock spin_lock and checking i_state for an inode being > * freed or being initialized, and incrementing the reference count > * before returning 1. It also must not sleep, since it is called with > * the inode_hash_lock spinlock held. > * > * This is a even more generalized version of ilookup5() when the > * function must never block --- find_inode() can block in > * __wait_on_freeing_inode() --- or when the caller can not increment > * the reference count because the resulting iput() might cause an > * inode eviction(). The tradeoff is that the @match funtion must be > * very carefully implemented. > */ > struct inode *find_inode_nowait(struct super_block *sb, > unsigned long hashval, > int (*match)(struct inode *, unsigned long, > void *), > void *data) > { > struct hlist_head *head = inode_hashtable + hash(sb, hashval); > struct inode *inode, *ret_inode = NULL; > int mval; > > spin_lock(&inode_hash_lock); > hlist_for_each_entry(inode, head, i_hash) { > if (inode->i_sb != sb) > continue; > mval = match(inode, hashval, data); > if (mval == 0) > continue; > if (mval == 1) > ret_inode = inode; > goto out; > } > out: > spin_unlock(&inode_hash_lock); > return ret_inode; > } > EXPORT_SYMBOL(find_inode_nowait); > > Comments? > > - Ted -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html