linux-kernel - Re: [PATCH] Fix queueing work if !bdi_cap_writeback

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: <20120912024249.GB17922@localhost>
Date:	Wed, 12 Sep 2012 10:42:49 +0800
From:	Fengguang Wu <fengguang.wu@...el.com>
To:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
Cc:	viro@...iv.linux.org.uk, jack@...e.cz, hch@....de,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix queueing work if !bdi_cap_writeback_dirty()

On Wed, Sep 12, 2012 at 03:28:42AM +0900, OGAWA Hirofumi wrote:
> 
> If bdi has BDI_CAP_NO_WRITEBACK, bdi_forker_thread() doesn't start
> writeback thread. This means there is no consumer of work item made
> by bdi_queue_work().
> 
> This adds to checking of !bdi_cap_writeback_dirty(sb->s_bdi) before
> calling bdi_queue_work(), otherwise queued work never be consumed.

Thanks for catching this! Does this bug have any side effects other
than memory leaking? It may be possible for some caller that actually
expect it to do some work to make progress, otherwise will eventually
block.  If so, we'll need to fix the caller.

Thanks,
Fengguang

> Signed-off-by: OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
> ---
> 
>  fs/fs-writeback.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff -puN fs/fs-writeback.c~noop_backing_dev_info-check-fix fs/fs-writeback.c
> --- linux/fs/fs-writeback.c~noop_backing_dev_info-check-fix	2012-09-11 06:12:30.000000000 +0900
> +++ linux-hirofumi/fs/fs-writeback.c	2012-09-11 06:12:30.000000000 +0900
> @@ -120,6 +120,9 @@ __bdi_start_writeback(struct backing_dev
>  {
>  	struct wb_writeback_work *work;
>  
> +	if (!bdi_cap_writeback_dirty(bdi))
> +		return;
> +
>  	/*
>  	 * This is WB_SYNC_NONE writeback, so if allocation fails just
>  	 * wakeup the thread for old dirty data writeback
> @@ -1310,7 +1313,7 @@ void writeback_inodes_sb_nr(struct super
>  		.reason			= reason,
>  	};
>  
> -	if (sb->s_bdi == &noop_backing_dev_info)
> +	if (!bdi_cap_writeback_dirty(sb->s_bdi))
>  		return;
>  	WARN_ON(!rwsem_is_locked(&sb->s_umount));
>  	bdi_queue_work(sb->s_bdi, &work);
> @@ -1396,7 +1399,7 @@ void sync_inodes_sb(struct super_block *
>  	};
>  
>  	/* Nothing to do? */
> -	if (sb->s_bdi == &noop_backing_dev_info)
> +	if (!bdi_cap_writeback_dirty(sb->s_bdi))
>  		return;
>  	WARN_ON(!rwsem_is_locked(&sb->s_umount));
>  
> _
> 
> -- 
> 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/