[PATCH] Properly handle signals on interruptible NFS mounts

Rick Macklem rmacklem at uoguelph.ca
Tue Jan 15 00:51:00 UTC 2013


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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-fs
> To unsubscribe, send any mail to "freebsd-fs-unsubscribe at freebsd.org"


More information about the freebsd-fs mailing list