Date: Sat, 14 Jun 2008 13:11:09 -0400
To: NeilBrown <neilb@suse.de>
Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] knfsd: nfsd: Handle ERESTARTSYS from syscalls.
Message-ID: <20080614171109.GA27041@fieldses.org>
References: <20080613213759.26929.patches@notabene> <1080613114215.27095@suse.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1080613114215.27095@suse.de>
User-Agent: Mutt/1.5.18 (2008-05-17)
From: "J. Bruce Fields" <bfields@fieldses.org>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2703
Lines: 75

On Fri, Jun 13, 2008 at 09:42:15PM +1000, NeilBrown wrote:
> 
> OCFS2 can return -ERESTARTSYS from write requests (and possibly
> elsewhere) if there is a signal pending.
> 
> If nfsd is shutdown (by sending a signal to each thread) while there
> is still an IO load from the client, each thread could handle one last
> request with a signal pending.  This can result in -ERESTARTSYS
> which is not understood by nfserrno() and so is reflected back to
> the client as nfserr_io aka -EIO.  This is wrong.
> 
> Instead, interpret ERESTARTSYS to mean "don't send a reply".
> The client will resend and - if the server is restarted - the write will
> (hopefully) be successful and everyone will be happy.

Thanks, applied--with a trivial change:

> 
> Signed-off-by: Neil Brown <neilb@suse.de>
> 
> ### Diffstat output
>  ./fs/nfsd/nfsproc.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> ----
> Funny how the shortest patches sometimes have the longest
> descriptions.

I added this to the body of the patch description:
> 
> The symptom that I narrowed down to this was:
>    copy a large file via NFS to an OCFS2 filesystem, and restart
>    the nfs server during the copy.
>    The 'cp' might get an -EIO, and the file will be corrupted -
>    presumably holes in the middle were writes appeared to fail.

On the grounds that someone might run across the same problem or need to
reproduce it for some reason, so a good, simple description of the case
that found it might help.

--b.

> 
> diff .prev/fs/nfsd/nfsproc.c ./fs/nfsd/nfsproc.c
> --- .prev/fs/nfsd/nfsproc.c	2008-06-13 21:31:53.000000000 +1000
> +++ ./fs/nfsd/nfsproc.c	2008-06-13 21:31:57.000000000 +1000
> @@ -614,6 +614,7 @@ nfserrno (int errno)
>  #endif
>  		{ nfserr_stale, -ESTALE },
>  		{ nfserr_jukebox, -ETIMEDOUT },
> +		{ nfserr_dropit, -ERESTARTSYS },
>  		{ nfserr_dropit, -EAGAIN },
>  		{ nfserr_dropit, -ENOMEM },
>  		{ nfserr_badname, -ESRCH },
> 
> ### Diffstat output
>  ./fs/nfsd/nfsproc.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff .prev/fs/nfsd/nfsproc.c ./fs/nfsd/nfsproc.c
> --- .prev/fs/nfsd/nfsproc.c	2008-06-13 21:31:53.000000000 +1000
> +++ ./fs/nfsd/nfsproc.c	2008-06-13 21:31:57.000000000 +1000
> @@ -614,6 +614,7 @@ nfserrno (int errno)
>  #endif
>  		{ nfserr_stale, -ESTALE },
>  		{ nfserr_jukebox, -ETIMEDOUT },
> +		{ nfserr_dropit, -ERESTARTSYS },
>  		{ nfserr_dropit, -EAGAIN },
>  		{ nfserr_dropit, -ENOMEM },
>  		{ nfserr_badname, -ESRCH },
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/