FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu Kernel Team

 
 
LinkBack Thread Tools
 
Old 02-11-2011, 08:29 PM
 
Default Lucid SRU, NFS: fix the return value of nfs_file_fsync()

The following changes since commit a1be943be789b54ed829eef43a13037a44558350:
Bruce Rogers (1):
virtio_net: Add schedule check to napi_enable call

are available in the git repository at:

git://kernel.ubuntu.com/rtg/ubuntu-lucid.git nfs-file-fsync-lp585657

Tim Gardner (1):
NFS: fix the return value of nfs_file_fsync()

fs/nfs/file.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
 
Old 02-14-2011, 09:47 AM
Stefan Bader
 
Default Lucid SRU, NFS: fix the return value of nfs_file_fsync()

On 02/11/2011 10:29 PM, Tim Gardner wrote:
>
> From 1a20d7bade407d3fbb3f682c9879826971cdfdbb Mon Sep 17 00:00:00 2001
> From: Tim Gardner <tim.gardner@canonical.com>
> Date: Fri, 11 Feb 2011 11:04:37 -0700
> Subject: [PATCH] NFS: fix the return value of nfs_file_fsync()
>
> BugLink: http://bugs.launchpad.net/bugs/585657
>
> Backport of 0702099bd86c33c2dcdbd3963433a61f3f503901
>
> By the commit af7fa16 2010-08-03 NFS: Fix up the fsync code
> close(2) became returning the non-zero value even if it went well.
> nfs_file_fsync() should return 0 when "status" is positive.
>

So if I understand this correctly, then it is a valid fix which has been
attributed to the wrong commit and by that may have missed the right upstream
stable branches. It looks to me that the offending commit was

commit 7b159fc18d417980f57aef64cab3417ee6af70f8
Author: Trond Myklebust <Trond.Myklebust@netapp.com>
Date: Wed Jul 25 14:09:54 2007 -0400

NFS: Fall back to synchronous writes when a background write errors...

and this happened around 2.6.24-rc1 which would in turn looks like the fix is
needed for all stable/longterm trees back to 2.6.24 (Hardy in our case).

It is not that simple to know all the effects. NFS internal calls seem to test
only for nfs_do_fsync <0 which would work in both cases. But the function is
used for the flush and fsync vfs operations and that could cause all sorts of
problems.

> Signed-off-by: J. R. Okajima <hooanon05@yahoo.co.jp>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
> fs/nfs/file.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 6fed6cc..99545e2 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -220,7 +220,7 @@ static int nfs_do_fsync(struct nfs_open_context *ctx, struct inode *inode)
> have_error |= test_bit(NFS_CONTEXT_ERROR_WRITE, &ctx->flags);
> if (have_error)
> ret = xchg(&ctx->error, 0);
> - if (!ret)
> + if (!ret && status < 0)
> ret = status;
> return ret;
> }


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 02-14-2011, 07:15 PM
John Johansen
 
Default Lucid SRU, NFS: fix the return value of nfs_file_fsync()

On 02/14/2011 02:47 AM, Stefan Bader wrote:
> On 02/11/2011 10:29 PM, Tim Gardner wrote:
>>
>> From 1a20d7bade407d3fbb3f682c9879826971cdfdbb Mon Sep 17 00:00:00 2001
>> From: Tim Gardner <tim.gardner@canonical.com>
>> Date: Fri, 11 Feb 2011 11:04:37 -0700
>> Subject: [PATCH] NFS: fix the return value of nfs_file_fsync()
>>
>> BugLink: http://bugs.launchpad.net/bugs/585657
>>
>> Backport of 0702099bd86c33c2dcdbd3963433a61f3f503901
>>
>> By the commit af7fa16 2010-08-03 NFS: Fix up the fsync code
>> close(2) became returning the non-zero value even if it went well.
>> nfs_file_fsync() should return 0 when "status" is positive.
>>
>
> So if I understand this correctly, then it is a valid fix which has been
> attributed to the wrong commit and by that may have missed the right upstream
> stable branches. It looks to me that the offending commit was
>
> commit 7b159fc18d417980f57aef64cab3417ee6af70f8
> Author: Trond Myklebust <Trond.Myklebust@netapp.com>
> Date: Wed Jul 25 14:09:54 2007 -0400
>
> NFS: Fall back to synchronous writes when a background write errors...
>
> and this happened around 2.6.24-rc1 which would in turn looks like the fix is
> needed for all stable/longterm trees back to 2.6.24 (Hardy in our case).
>
> It is not that simple to know all the effects. NFS internal calls seem to test
> only for nfs_do_fsync <0 which would work in both cases. But the function is
> used for the flush and fsync vfs operations and that could cause all sorts of
> problems.

Hrmmm I have to agree it looks like we need this on hardy as well, but I want to
spend a little more time tracing all the paths

>
>> Signed-off-by: J. R. Okajima <hooanon05@yahoo.co.jp>
>> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> Acked-by: Stefan Bader <stefan.bader@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>


>> ---
>> fs/nfs/file.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>> index 6fed6cc..99545e2 100644
>> --- a/fs/nfs/file.c
>> +++ b/fs/nfs/file.c
>> @@ -220,7 +220,7 @@ static int nfs_do_fsync(struct nfs_open_context *ctx, struct inode *inode)
>> have_error |= test_bit(NFS_CONTEXT_ERROR_WRITE, &ctx->flags);
>> if (have_error)
>> ret = xchg(&ctx->error, 0);
>> - if (!ret)
>> + if (!ret && status < 0)
>> ret = status;
>> return ret;
>> }
>
>


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 02-14-2011, 07:25 PM
Tim Gardner
 
Default Lucid SRU, NFS: fix the return value of nfs_file_fsync()

applied and pushed

--
Tim Gardner tim.gardner@canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 02-14-2011, 07:41 PM
Tim Gardner
 
Default Lucid SRU, NFS: fix the return value of nfs_file_fsync()

On 02/14/2011 01:15 PM, John Johansen wrote:

On 02/14/2011 02:47 AM, Stefan Bader wrote:

On 02/11/2011 10:29 PM, Tim Gardner wrote:


From 1a20d7bade407d3fbb3f682c9879826971cdfdbb Mon Sep 17 00:00:00 2001
From: Tim Gardner<tim.gardner@canonical.com>
Date: Fri, 11 Feb 2011 11:04:37 -0700
Subject: [PATCH] NFS: fix the return value of nfs_file_fsync()

BugLink: http://bugs.launchpad.net/bugs/585657

Backport of 0702099bd86c33c2dcdbd3963433a61f3f503901

By the commit af7fa16 2010-08-03 NFS: Fix up the fsync code
close(2) became returning the non-zero value even if it went well.
nfs_file_fsync() should return 0 when "status" is positive.



So if I understand this correctly, then it is a valid fix which has been
attributed to the wrong commit and by that may have missed the right upstream
stable branches. It looks to me that the offending commit was

commit 7b159fc18d417980f57aef64cab3417ee6af70f8
Author: Trond Myklebust<Trond.Myklebust@netapp.com>
Date: Wed Jul 25 14:09:54 2007 -0400

NFS: Fall back to synchronous writes when a background write errors...

and this happened around 2.6.24-rc1 which would in turn looks like the fix is
needed for all stable/longterm trees back to 2.6.24 (Hardy in our case).

It is not that simple to know all the effects. NFS internal calls seem to test
only for nfs_do_fsync<0 which would work in both cases. But the function is
used for the flush and fsync vfs operations and that could cause all sorts of
problems.


Hrmmm I have to agree it looks like we need this on hardy as well, but I want to
spend a little more time tracing all the paths



I've submitted a patch for Hardy. The semantics and call sites that
exist in 2.6.24 appear to be identical to 2.6.32.


rtg
--
Tim Gardner tim.gardner@canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 

Thread Tools




All times are GMT. The time now is 10:56 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org