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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20101016092916.GA32197@amd>
Date:	Sat, 16 Oct 2010 20:29:16 +1100
From:	Nick Piggin <npiggin@...nel.dk>
To:	Dave Chinner <david@...morbit.com>
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/19] fs: Implement lazy LRU updates for inodes.

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/