Re: [PATCH 04/19] fs: Implement lazy LRU updates for inodes.

[Nick Piggin]

On Sat, Oct 16, 2010 at 07:13:58PM +1100, Dave Chinner wrote:
> @@ -502,11 +527,15 @@ static void prune_icache(int nr_to_scan)
>  			iput(inode);
>  			spin_lock(&inode_lock);
>  
> -			if (inode != list_entry(inode_unused.next,
> -						struct inode, i_list))
> -				continue;	/* wrong inode or list_empty */
> -			if (!can_unuse(inode))
> +			/*
> +			 * if we can't reclaim this inode immediately, give it
> +			 * another pass through the free list so we don't spin
> +			 * on it.
> +			 */
> +			if (!can_unuse(inode)) {
> +				list_move(&inode->i_list, &inode_unused);
>  				continue;
> +			}
>  		}
>  		list_move(&inode->i_list, &freeable);
>  		WARN_ON(inode->i_state & I_NEW);

This is a bug, actually 2 bugs, which is why I omitted it in the version
you picked up. I agree we want the optimisation though, so I've added it
back in my tree.

After you iput() and then re take the inode lock, you can't reference
the inode because you don't know what happened to it. You need to keep
that pointer check to verify it is still there.

Secondly, iput sets I_REFERENCED, so you can't leave can_unuse
unmodified otherwise then it's basically always a false negative. It
needs to check for (i_state & ~I_REFERENCED)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/