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-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
