From owner-svn-src-all@FreeBSD.ORG Wed Feb 6 21:05:03 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 6AA6FD52; Wed, 6 Feb 2013 21:05:03 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 415F8A11; Wed, 6 Feb 2013 21:05:03 +0000 (UTC) Received: from pakbsde14.localnet (unknown [38.105.238.108]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 03C01B915; Wed, 6 Feb 2013 16:05:02 -0500 (EST) From: John Baldwin To: src-committers@freebsd.org Subject: Re: svn commit: r246417 - in head/sys: fs/nfs kern nfsclient sys Date: Wed, 6 Feb 2013 12:11:13 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p22; KDE/4.5.5; amd64; ; ) References: <201302061706.r16H6qMo088889@svn.freebsd.org> In-Reply-To: <201302061706.r16H6qMo088889@svn.freebsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201302061211.14184.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Wed, 06 Feb 2013 16:05:02 -0500 (EST) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Feb 2013 21:05:03 -0000 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 #include #include #include #include #include #include #include #include static void usage(void) { fprintf(stderr, "Usage: nfsintr \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