Re: [PATCH 44/45] NFS: remove NFS_INO_FLUSHING lock

From: Peter Staubach
Date: Wed Oct 07 2009 - 09:13:26 EST


Wu Fengguang wrote:
> It was introduced in 72cb77f4a5ac, and the several issues have been
> addressed in generic writeback:
> - out of order writeback (or interleaved concurrent writeback)
> addressed by the per-bdi writeback and wait queue in balance_dirty_pages()
> - sync livelocked by a fast dirtier
> addressed by throttling all to-be-synced dirty inodes
>

I don't think that we can just remove this support. It is
designed to reduce the effects from doing a stat(2) on a
file which is being actively written to.

If we do remove it, then we will need to replace this patch
with another. Trond and I hadn't quite finished discussing
some aspects of that other patch... :-)

Thanx...

ps

> CC: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> CC: Peter Staubach <staubach@xxxxxxxxxx>
> CC: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> Signed-off-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
> ---
> fs/nfs/file.c | 9 ---------
> fs/nfs/write.c | 11 -----------
> include/linux/nfs_fs.h | 1 -
> 3 files changed, 21 deletions(-)
>
> --- linux.orig/fs/nfs/file.c 2009-10-07 14:31:45.000000000 +0800
> +++ linux/fs/nfs/file.c 2009-10-07 14:32:54.000000000 +0800
> @@ -386,15 +386,6 @@ static int nfs_write_begin(struct file *
> mapping->host->i_ino, len, (long long) pos);
>
> start:
> - /*
> - * Prevent starvation issues if someone is doing a consistency
> - * sync-to-disk
> - */
> - ret = wait_on_bit(&NFS_I(mapping->host)->flags, NFS_INO_FLUSHING,
> - nfs_wait_bit_killable, TASK_KILLABLE);
> - if (ret)
> - return ret;
> -
> page = grab_cache_page_write_begin(mapping, index, flags);
> if (!page)
> return -ENOMEM;
> --- linux.orig/fs/nfs/write.c 2009-10-07 14:31:45.000000000 +0800
> +++ linux/fs/nfs/write.c 2009-10-07 14:32:54.000000000 +0800
> @@ -387,26 +387,15 @@ static int nfs_writepages_callback(struc
> int nfs_writepages(struct address_space *mapping, struct writeback_control *wbc)
> {
> struct inode *inode = mapping->host;
> - unsigned long *bitlock = &NFS_I(inode)->flags;
> struct nfs_pageio_descriptor pgio;
> int err;
>
> - /* Stop dirtying of new pages while we sync */
> - err = wait_on_bit_lock(bitlock, NFS_INO_FLUSHING,
> - nfs_wait_bit_killable, TASK_KILLABLE);
> - if (err)
> - goto out_err;
> -
> nfs_inc_stats(inode, NFSIOS_VFSWRITEPAGES);
>
> nfs_pageio_init_write(&pgio, inode, wb_priority(wbc));
> err = write_cache_pages(mapping, wbc, nfs_writepages_callback, &pgio);
> nfs_pageio_complete(&pgio);
>
> - clear_bit_unlock(NFS_INO_FLUSHING, bitlock);
> - smp_mb__after_clear_bit();
> - wake_up_bit(bitlock, NFS_INO_FLUSHING);
> -
> if (err < 0)
> goto out_err;
> err = pgio.pg_error;
> --- linux.orig/include/linux/nfs_fs.h 2009-10-07 14:31:45.000000000 +0800
> +++ linux/include/linux/nfs_fs.h 2009-10-07 14:32:54.000000000 +0800
> @@ -208,7 +208,6 @@ struct nfs_inode {
> #define NFS_INO_STALE (1) /* possible stale inode */
> #define NFS_INO_ACL_LRU_SET (2) /* Inode is on the LRU list */
> #define NFS_INO_MOUNTPOINT (3) /* inode is remote mountpoint */
> -#define NFS_INO_FLUSHING (4) /* inode is flushing out data */
> #define NFS_INO_FSCACHE (5) /* inode can be cached by FS-Cache */
> #define NFS_INO_FSCACHE_LOCK (6) /* FS-Cache cookie management lock */
--
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/