Date: Mon, 14 Jan 2013 19:50:52 -0500 (EST) From: Rick Macklem <rmacklem@uoguelph.ca> To: John Baldwin <jhb@freebsd.org> Cc: Rick Macklem <rmacklem@freebsd.org>, Doug Rabson <dfr@freebsd.org>, fs@freebsd.org Subject: Re: [PATCH] Properly handle signals on interruptible NFS mounts Message-ID: <21875538.1984621.1358211052621.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <201301141437.05040.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
John Baldwin wrote:
> When the new RPC layer was brought in, the RPC_INTR return value (to
> indicate
> an RPC request was interrupted by a signal) was not handled in the NFS
> client.
> As a result, if an NFS request is interrupted by a signal (on a mount
> with the
> "intr" option), then the nfs_request() functions would fall through to
> the
> default case and return EACCES rather than EINTR. While here, I
> noticed that
> the new RPC layer also lost all of the RPC statistics the old client
> used to
> keep (but that are still reported in 'nfsstat -c'). I've added back as
> many
> of the statistics as I could, but retries are not easy to do as only
> the RPC
> layer knows about them and not the NFS client.
>
> Index: fs/nfs/nfs_commonkrpc.c
> ===================================================================
> --- fs/nfs/nfs_commonkrpc.c (revision 245225)
> +++ fs/nfs/nfs_commonkrpc.c (working copy)
> @@ -767,12 +767,18 @@
> if (stat == RPC_SUCCESS) {
> error = 0;
> } else if (stat == RPC_TIMEDOUT) {
> + NFSINCRGLOBAL(newnfsstats.rpctimeouts);
> error = ETIMEDOUT;
> } else if (stat == RPC_VERSMISMATCH) {
> + NFSINCRGLOBAL(newnfsstats.rpcinvalid);
> error = EOPNOTSUPP;
> } else if (stat == RPC_PROGVERSMISMATCH) {
> + NFSINCRGLOBAL(newnfsstats.rpcinvalid);
> error = EPROTONOSUPPORT;
> + } else if (stat == RPC_INTR) {
> + error = EINTR;
> } else {
> + NFSINCRGLOBAL(newnfsstats.rpcinvalid);
> error = EACCES;
> }
> if (error) {
> Index: nfsclient/nfs_krpc.c
> ===================================================================
> --- nfsclient/nfs_krpc.c (revision 245225)
> +++ nfsclient/nfs_krpc.c (working copy)
> @@ -549,14 +549,21 @@
> */
> if (stat == RPC_SUCCESS)
> error = 0;
> - else if (stat == RPC_TIMEDOUT)
> + else if (stat == RPC_TIMEDOUT) {
> + nfsstats.rpctimeouts++;
> error = ETIMEDOUT;
> - else if (stat == RPC_VERSMISMATCH)
> + } else if (stat == RPC_VERSMISMATCH) {
> + nfsstats.rpcinvalid++;
> error = EOPNOTSUPP;
> - else if (stat == RPC_PROGVERSMISMATCH)
> + } else if (stat == RPC_PROGVERSMISMATCH) {
> + nfsstats.rpcinvalid++;
> error = EPROTONOSUPPORT;
> - else
> + } else if (stat == RPC_INTR) {
> + error = EINTR;
> + } else {
> + nfsstats.rpcinvalid++;
> error = EACCES;
> + }
> if (error)
> goto nfsmout;
>
> @@ -572,6 +579,7 @@
> if (error == ENOMEM) {
> m_freem(mrep);
> AUTH_DESTROY(auth);
> + nfsstats.rpcinvalid++;
> return (error);
> }
>
This patch looks fine to me, rick
>
> --
> John Baldwin
> _______________________________________________
> freebsd-fs@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-fs
> To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?21875538.1984621.1358211052621.JavaMail.root>