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

Nick Piggin
2010-10-16T09:29:23Z

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)