Re: [PATCH] NFS: introduce writeback wait queue

[Wu Fengguang]

Hi Jens,

This is a bug fix for 2.6.32. Maybe other not block-queue based
filesystems will have similar issues ..

Thanks,
Fengguang


On Sun, Oct 04, 2009 at 11:01:53AM +0800, Wu Fengguang wrote:
> The generic writeback routines are departing from congestion_wait()
> in preference of get_request_wait(), aka. to wait on the block queues.
> 
> Introduce the missing writeback wait queue for NFS, otherwise its
> writeback pages will grow out of control.
> 
> The SYNC writes can use the full queue space (2*nfs_congestion_kb), while
> the ASYNC writes can only use half queue space. This way SYNC writes won't
> be blocked by the ASYNC ones at all.
> 
> We'll be waiting inside the NFS_INO_FLUSHING lock, hence also be
> blocking possible dirtiers.  This should not be a bit problem.
> And we should be able to obsolete the NFS_INO_FLUSHING with more
> general writeback improvements in long term.
> 
> CC: Jens Axboe <jens.axboe@xxxxxxxxxx> 
> CC: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> Signed-off-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
> ---
>  fs/nfs/client.c           |    2 
>  fs/nfs/write.c            |   73 +++++++++++++++++++++++++++++-------
>  include/linux/nfs_fs_sb.h |    1 
>  3 files changed, 62 insertions(+), 14 deletions(-)
> 
> --- linux.orig/fs/nfs/write.c	2009-10-04 08:47:16.000000000 +0800
> +++ linux/fs/nfs/write.c	2009-10-04 10:55:32.000000000 +0800
> @@ -189,24 +189,58 @@ static int wb_priority(struct writeback_
>  
>  int nfs_congestion_kb;
>  
> +/*
> + * SYNC requests will be blocked on NFS_SYNC_*_THRESH
> + * ASYNC requests will be blocked on NFS_CONGESTION_*_THRESH
> + */
> +#define NFS_SYNC_WAIT_THRESH 	(nfs_congestion_kb >> (PAGE_SHIFT-11))
> +#define NFS_SYNC_WAKEUP_THRESH	\
> +	(NFS_SYNC_WAIT_THRESH - (NFS_SYNC_WAIT_THRESH >> 2))
> +
>  #define NFS_CONGESTION_ON_THRESH 	(nfs_congestion_kb >> (PAGE_SHIFT-10))
>  #define NFS_CONGESTION_OFF_THRESH	\
>  	(NFS_CONGESTION_ON_THRESH - (NFS_CONGESTION_ON_THRESH >> 2))
>  
> -static int nfs_set_page_writeback(struct page *page)
> +static void nfs_writeback_wait(struct page *page, struct writeback_control *wbc)
>  {
> -	int ret = test_set_page_writeback(page);
> +	struct inode *inode = page->mapping->host;
> +	struct nfs_server *nfss = NFS_SERVER(inode);
> +	int is_sync = wbc->sync_mode == WB_SYNC_NONE;
> +	DEFINE_WAIT(wait);
>  
> -	if (!ret) {
> -		struct inode *inode = page->mapping->host;
> -		struct nfs_server *nfss = NFS_SERVER(inode);
> +	if (atomic_long_inc_return(&nfss->writeback) < NFS_CONGESTION_ON_THRESH)
> +		return;
>  
> -		if (atomic_long_inc_return(&nfss->writeback) >
> -				NFS_CONGESTION_ON_THRESH) {
> -			set_bdi_congested(&nfss->backing_dev_info,
> -						BLK_RW_ASYNC);
> -		}
> +	set_bdi_congested(&nfss->backing_dev_info, BLK_RW_ASYNC);
> +
> +	if (is_sync && atomic_long_read(&nfss->writeback) <
> +	    NFS_SYNC_WAIT_THRESH)
> +		return;
> +
> +	for (;;) {
> +		prepare_to_wait_exclusive(&nfss->writeback_wait[is_sync], &wait,
> +					  TASK_UNINTERRUPTIBLE);
> +
> +		io_schedule();
> +
> +		finish_wait(&nfss->writeback_wait[is_sync], &wait);
> +
> +		if (atomic_long_read(&nfss->writeback) <
> +		    NFS_CONGESTION_OFF_THRESH)
> +			break;
> +		if (is_sync && atomic_long_read(&nfss->writeback) <
> +		    NFS_SYNC_WAKEUP_THRESH)
> +			break;
>  	}
> +}
> +
> +static int nfs_set_page_writeback(struct page *page, struct writeback_control *wbc)
> +{
> +	int ret = test_set_page_writeback(page);
> +
> +	if (!ret)
> +		nfs_writeback_wait(page, wbc);
> +
>  	return ret;
>  }
>  
> @@ -216,8 +250,18 @@ static void nfs_end_page_writeback(struc
>  	struct nfs_server *nfss = NFS_SERVER(inode);
>  
>  	end_page_writeback(page);
> -	if (atomic_long_dec_return(&nfss->writeback) < NFS_CONGESTION_OFF_THRESH)
> -		clear_bdi_congested(&nfss->backing_dev_info, BLK_RW_ASYNC);
> +
> +	if (atomic_long_dec_return(&nfss->writeback) < NFS_SYNC_WAKEUP_THRESH) {
> +		if (waitqueue_active(&nfss->writeback_wait[1]))
> +			wake_up(&nfss->writeback_wait[1]);
> +		if (atomic_long_read(&nfss->writeback) <
> +		    NFS_CONGESTION_OFF_THRESH) {
> +			clear_bdi_congested(&nfss->backing_dev_info, BLK_RW_ASYNC);
> +			if (waitqueue_active(&nfss->writeback_wait[0]))
> +				wake_up(&nfss->writeback_wait[0]);
> +		}
> +	}
> +
>  }
>  
>  static struct nfs_page *nfs_find_and_lock_request(struct page *page)
> @@ -254,6 +298,7 @@ static struct nfs_page *nfs_find_and_loc
>   * May return an error if the user signalled nfs_wait_on_request().
>   */
>  static int nfs_page_async_flush(struct nfs_pageio_descriptor *pgio,
> +				struct writeback_control *wbc,
>  				struct page *page)
>  {
>  	struct nfs_page *req;
> @@ -266,7 +311,7 @@ static int nfs_page_async_flush(struct n
>  	if (IS_ERR(req))
>  		goto out;
>  
> -	ret = nfs_set_page_writeback(page);
> +	ret = nfs_set_page_writeback(page, wbc);
>  	BUG_ON(ret != 0);
>  	BUG_ON(test_bit(PG_CLEAN, &req->wb_flags));
>  
> @@ -286,7 +331,7 @@ static int nfs_do_writepage(struct page 
>  	nfs_add_stats(inode, NFSIOS_WRITEPAGES, 1);
>  
>  	nfs_pageio_cond_complete(pgio, page->index);
> -	return nfs_page_async_flush(pgio, page);
> +	return nfs_page_async_flush(pgio, wbc, page);
>  }
>  
>  /*
> --- linux.orig/include/linux/nfs_fs_sb.h	2009-10-04 09:31:25.000000000 +0800
> +++ linux/include/linux/nfs_fs_sb.h	2009-10-04 09:58:11.000000000 +0800
> @@ -108,6 +108,7 @@ struct nfs_server {
>  	struct nfs_iostats *	io_stats;	/* I/O statistics */
>  	struct backing_dev_info	backing_dev_info;
>  	atomic_long_t		writeback;	/* number of writeback pages */
> +	wait_queue_head_t	writeback_wait[2];
>  	int			flags;		/* various flags */
>  	unsigned int		caps;		/* server capabilities */
>  	unsigned int		rsize;		/* read size */
> --- linux.orig/fs/nfs/client.c	2009-10-04 09:59:46.000000000 +0800
> +++ linux/fs/nfs/client.c	2009-10-04 10:00:55.000000000 +0800
> @@ -991,6 +991,8 @@ static struct nfs_server *nfs_alloc_serv
>  	INIT_LIST_HEAD(&server->master_link);
>  
>  	atomic_set(&server->active, 0);
> +	init_waitqueue_head(&server->writeback_wait[BLK_RW_SYNC]);
> +	init_waitqueue_head(&server->writeback_wait[BLK_RW_ASYNC]);
>  
>  	server->io_stats = nfs_alloc_iostats();
>  	if (!server->io_stats) {
--
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/