> Here's a lightly tested patch that turns the check for the two flags
> into a check for WB_SYNC_NONE. It seems to do the right thing, but I
> don't have a clear testcase for it. Does this look reasonable?
Yes, I don't see any problems.
> ------------------[snip]------------------------
>
> NFS: don't use FLUSH_SYNC on WB_SYNC_NONE COMMIT calls
>
> WB_SYNC_NONE is supposed to mean "don't wait on anything". That should
> also include not waiting for COMMIT calls to complete.
>
> WB_SYNC_NONE is also implied when wbc->nonblocking or
> wbc->for_background are set, so we can replace those checks in
> nfs_commit_unstable_pages with a check for WB_SYNC_NONE.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/nfs/write.c | 10 +++++-----
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index 874972d..35bd7d0 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -1436,12 +1436,12 @@ static int nfs_commit_unstable_pages(struct inode *inode, struct writeback_contr
> /* Don't commit yet if this is a non-blocking flush and there are
> * lots of outstanding writes for this mapping.
> */
> - if (wbc->sync_mode == WB_SYNC_NONE &&
> - nfsi->ncommit <= (nfsi->npages >> 1))
> - goto out_mark_dirty;
> -
> - if (wbc->nonblocking || wbc->for_background)
> + if (wbc->sync_mode == WB_SYNC_NONE) {
> + if (nfsi->ncommit <= (nfsi->npages >> 1))
> + goto out_mark_dirty;
> flags = 0;
> + }
> +
nitpick: I'd slightly prefer an one-line change
- if (wbc->nonblocking || wbc->for_background)
+ if (wbc->sync_mode == WB_SYNC_NONE)
flags = 0;
That way the patch will look more obvious and "git blame" friendly,
and the original "Don't commit.." comment will best match its code.
Reviewed-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
