svn commit: r246417 - in head/sys: fs/nfs kern nfsclient sys

[John Baldwin]

On Wednesday, February 06, 2013 12:06:52 pm John Baldwin wrote:
> Author: jhb
> Date: Wed Feb  6 17:06:51 2013
> New Revision: 246417
> URL: http://svnweb.freebsd.org/changeset/base/246417
> 
> Log:
>   Rework the handling of stop signals in the NFS client.  The changes in
>   195702, 195703, and 195821 prevented a thread from suspending while holding
>   locks inside of NFS by forcing the thread to fail sleeps with EINTR or
>   ERESTART but defer the thread suspension to the user boundary.  However,
>   this had the effect that stopping a process during an NFS request could
>   abort the request and trigger EINTR errors that were visible to userland
>   processes (previously the thread would have suspended and completed the
>   request once it was resumed).
>   
>   This change instead effectively masks stop signals while in the NFS client.
>   It uses the existing TDF_SBDRY flag to effect this since SIGSTOP cannot
>   be masked directly.  Also, instead of setting PBDRY on individual sleeps,
>   the NFS client now sets the TDF_SBDRY flag around each NFS request and
>   stop signals are masked for all sleeps during that region (the previous
>   change missed sleeps in lockmgr locks).  The end result is that stop
>   signals sent to threads performing an NFS request are completely
>   ignored until after the NFS request has finished processing and the
>   thread prepares to return to userland.  This restores the behavior of
>   stop signals being transparent to userland processes while still
>   preventing threads from suspending while holding NFS locks.
>   
>   Reviewed by:	kib
>   MFC after:	1 month

I have a test case (included below).  You give it a path to a file on an
interruptible NFS mount as the sole argument.  In the broken case you will
see lots of reads fail with EINTR.

#include <sys/types.h>
#include <sys/stat.h>
#include <err.h>
#include <errno.h>
#include <fcntl.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

static void
usage(void)
{

	fprintf(stderr, "Usage: nfsintr <path>\n");
	exit(1);
}

static volatile sig_atomic_t info;

static void
child_info_handler(int sig)
{

	info = 1;
}

/*
 * Check to see if STOP/CONT signals affect I/O.  One process
 * continually opens a file with O_DIRECT and reads it while another
 * process keeps pausing the first with SIGSTOP.
 */
static void
child(const char *path)
{
	char buf[128 * 1024];
	struct stat sb;
	ssize_t nread;
	off_t off;
	int fd, shorts;

	if (signal(SIGINFO, child_info_handler) == SIG_ERR)
		err(1, "signal(SIGINFO) (child)");
	shorts = 0;
	while (getppid() != 1) {
		fd = open(path, O_RDONLY | O_DIRECT);
		while (fd < 0) {
			if (errno == EINTR)
				warn("open(%s)", path);
			else
				err(1, "open(%s)", path);
		}
		while (fstat(fd, &sb) < 0) {
			if (errno == EINTR)
				warn("fstat");
			else
				err(1, "fstat");
		}
		for (;;) {
			if (info) {
				if (shorts > 0)
					printf("nfsintr: %d short reads\n",
					    shorts);
				shorts = 0;
				info = 0;
			}
			nread = read(fd, buf, sizeof(buf));
			if (nread < 0) {
				if (errno == EINTR)
					warn("read");
				else
					err(1, "read");
				continue;
			}
			if (nread == 0)
				break;
			if (nread == sizeof(buf))
				continue;
			if (nread < (ssize_t)sizeof(buf)) {
				off = lseek(fd, SEEK_CUR, 0);
				if (off < 0)
					err(1, "lseek");
				if (off == sb.st_size)
					break;
				/*
				 * These happen a lot.
				warnx("short read: %zd", nread);
				 */
				shorts++;
				continue;
			}
		}
		close(fd);
	}
}

static void
parent(pid_t pid)
{

	for (;;) {
		if (kill(pid, SIGSTOP) < 0) {
			if (errno == ESRCH)
				return;
			err(1, "kill(SIGSTOP)");
		}
		usleep(500);
		if (kill(pid, SIGCONT) < 0) {
			if (errno == ESRCH)
				return;
			err(1, "kill(SIGCONT)");
		}
		usleep(500);
	}
}

int
main(int ac, char **av)
{
	pid_t pid;

	if (ac != 2)
		usage();
	if (access(av[1], R_OK) < 0)
		err(1, "Unable to access file %s", av[1]);
	if (signal(SIGCHLD, SIG_IGN) == SIG_ERR)
		err(1, "signal(SIGCHLD)");
	pid = fork();
	if (pid < 0)
		err(1, "fork");
	if (pid == 0)
		child(av[1]);
	else
		parent(pid);
	return (0);
}

-- 
John Baldwin