linux-kernel - Re: The bug of iput() removal from flusher thread?

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]

Date:	Tue, 20 Nov 2012 05:51:15 +0900
From:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To:	Jan Kara <jack@...e.cz>
Cc:	Al Viro <viro@...iv.linux.org.uk>, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: The bug of iput() removal from flusher thread?

Jan Kara <jack@...e.cz> writes:

>>>From 4fdc5d9a66dfe0286ef4f4a7f53fd3b15086470f Mon Sep 17 00:00:00 2001
> From: Jan Kara <jack@...e.cz>
> Date: Mon, 19 Nov 2012 20:01:16 +0100
> Subject: [PATCH] writeback: Put unused inodes to LRU after writeback completion
>
> Commit 169ebd90 removed iget-iput pair from inode writeback. As a side effect,
> inodes that are dirty during iput_final() call won't be ever added to inode LRU
> (iput_final() doesn't add dirty inodes to LRU and later when the inode is
> cleaned there's noone to add the inode there). Thus inodes are effectively
> unreclaimable until someone looks them up again.
>
> Practical effect of this bug is limited by the fact that inodes are
> pinned by a dentry for long enough that the inode gets cleaned. But still
> the bug can have nasty consequences leading up to OOM conditions under
> certain circumstances. Following can easily reproduce the problem:
>
> for (( i = 0; i < 1000; i++ )); do
>   mkdir $i
>   for (( j = 0; j < 1000; j++ )); do
>     touch $i/$j
>     echo 2 > /proc/sys/vm/drop_caches
>   done
> done
>
> then one needs to run 'sync; ls -lR' to make inodes reclaimable again.
>
> We fix the issue by inserting unused clean inodes into the LRU after writeback
> finishes in inode_sync_complete().
>
> CC: Al Viro <viro@...iv.linux.org.uk>
> Reported-by: OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
> Signed-off-by: Jan Kara <jack@...e.cz>

Need to Cc to stable@

> ---
>  fs/fs-writeback.c |    3 +++
>  fs/inode.c        |    2 +-
>  fs/internal.h     |    1 +
>  3 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 51ea267..ed7613b 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -227,6 +227,9 @@ static void requeue_io(struct inode *inode, struct bdi_writeback *wb)
>  
>  static void inode_sync_complete(struct inode *inode)
>  {
> +	/* If inode is clean an unused, put it into LRU now.  */
> +	if (!(inode->i_state & I_DIRTY) && !atomic_read(&inode->i_count))
> +		inode_lru_list_add(inode);

IMHO, open coding this would be bad idea. And another one is
I_REFERENCED. We really want to remove I_REFERENCED?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
--
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/