Module Name:    src
Committed By:   martin
Date:           Mon Oct  4 14:48:25 UTC 2021

Modified Files:
        src/sys/miscfs/fifofs [netbsd-9]: fifo_vnops.c
        src/tests/kernel/kqueue/write [netbsd-9]: t_fifo.c

Log Message:
Pull up following revision(s) (requested by thorpej in ticket #1353):

        sys/miscfs/fifofs/fifo_vnops.c: revision 1.90
        tests/kernel/kqueue/write/t_fifo.c: revision 1.5

- Add a new EVFILT_WRITE test case for FIFOs that correctly validates
the writability thresholds.
- Fix a bug in fifo_kqfilter() exposed by the new test case; in the
EVFILT_WRITE case, we were attaching the wrong end of the socket
pair to the knote!
- In filt_fiforead(), use ">= so->so_rcv.sb_lowat" rather than "> 0"
for consistency with fifo_poll().  NFC.


To generate a diff of this commit:
cvs rdiff -u -r1.79.8.2 -r1.79.8.3 src/sys/miscfs/fifofs/fifo_vnops.c
cvs rdiff -u -r1.4 -r1.4.16.1 src/tests/kernel/kqueue/write/t_fifo.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/fifofs/fifo_vnops.c
diff -u src/sys/miscfs/fifofs/fifo_vnops.c:1.79.8.2 src/sys/miscfs/fifofs/fifo_vnops.c:1.79.8.3
--- src/sys/miscfs/fifofs/fifo_vnops.c:1.79.8.2	Mon Oct  4 14:32:38 2021
+++ src/sys/miscfs/fifofs/fifo_vnops.c	Mon Oct  4 14:48:25 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: fifo_vnops.c,v 1.79.8.2 2021/10/04 14:32:38 martin Exp $	*/
+/*	$NetBSD: fifo_vnops.c,v 1.79.8.3 2021/10/04 14:48:25 martin Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: fifo_vnops.c,v 1.79.8.2 2021/10/04 14:32:38 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fifo_vnops.c,v 1.79.8.3 2021/10/04 14:48:25 martin Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -613,7 +613,7 @@ filt_fiforead(struct knote *kn, long hin
 		rv = 1;
 	} else {
 		kn->kn_flags &= ~EV_EOF;
-		rv = (kn->kn_data > 0);
+		rv = (kn->kn_data >= so->so_rcv.sb_lowat);
 	}
 	if (hint != NOTE_SUBMIT)
 		sounlock(so);
@@ -680,13 +680,14 @@ fifo_kqfilter(void *v)
 	struct socket	*so;
 	struct sockbuf	*sb;
 
-	so = (struct socket *)ap->a_vp->v_fifoinfo->fi_readsock;
 	switch (ap->a_kn->kn_filter) {
 	case EVFILT_READ:
+		so = (struct socket *)ap->a_vp->v_fifoinfo->fi_readsock;
 		ap->a_kn->kn_fop = &fiforead_filtops;
 		sb = &so->so_rcv;
 		break;
 	case EVFILT_WRITE:
+		so = (struct socket *)ap->a_vp->v_fifoinfo->fi_writesock;
 		ap->a_kn->kn_fop = &fifowrite_filtops;
 		sb = &so->so_snd;
 		break;

Index: src/tests/kernel/kqueue/write/t_fifo.c
diff -u src/tests/kernel/kqueue/write/t_fifo.c:1.4 src/tests/kernel/kqueue/write/t_fifo.c:1.4.16.1
--- src/tests/kernel/kqueue/write/t_fifo.c:1.4	Fri Jan 13 21:30:41 2017
+++ src/tests/kernel/kqueue/write/t_fifo.c	Mon Oct  4 14:48:25 2021
@@ -1,11 +1,11 @@
-/* $NetBSD: t_fifo.c,v 1.4 2017/01/13 21:30:41 christos Exp $ */
+/* $NetBSD: t_fifo.c,v 1.4.16.1 2021/10/04 14:48:25 martin Exp $ */
 
 /*-
- * Copyright (c) 2002, 2008 The NetBSD Foundation, Inc.
+ * Copyright (c) 2021 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
- * by Luke Mewburn and Jaromir Dolecek.
+ * by Jason R. Thorpe.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -30,68 +30,116 @@
  */
 
 #include <sys/cdefs.h>
-__COPYRIGHT("@(#) Copyright (c) 2008\
+__COPYRIGHT("@(#) Copyright (c) 2021\
  The NetBSD Foundation, inc. All rights reserved.");
-__RCSID("$NetBSD: t_fifo.c,v 1.4 2017/01/13 21:30:41 christos Exp $");
+__RCSID("$NetBSD: t_fifo.c,v 1.4.16.1 2021/10/04 14:48:25 martin Exp $");
 
 #include <sys/event.h>
 #include <sys/stat.h>
-#include <sys/wait.h>
 
+#include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <inttypes.h>
 
 #include <atf-c.h>
 
-#include "h_macros.h"
+static const char	fifo_path[] = "fifo";
 
-#define FIFONAME "fifo"
+static void
+fifo_support(void)
+{
+	errno = 0;
+	if (mkfifo(fifo_path, 0600) == 0) {
+		ATF_REQUIRE(unlink(fifo_path) == 0);
+		return;
+	}
+
+	if (errno == EOPNOTSUPP) {
+		atf_tc_skip("the kernel does not support FIFOs");
+	} else {
+		atf_tc_fail("mkfifo(2) failed");
+	}
+}
 
-ATF_TC(fifo);
+ATF_TC_WITH_CLEANUP(fifo);
 ATF_TC_HEAD(fifo, tc)
 {
-	atf_tc_set_md_var(tc, "descr", "Checks EVFILT_WRITE for fifo");
+	atf_tc_set_md_var(tc, "descr", "Checks EVFILT_WRITE on fifo");
 }
 ATF_TC_BODY(fifo, tc)
 {
-	char buffer[128];
+	const struct timespec to = { 0, 0 };
 	struct kevent event[1];
-	pid_t child;
-	int kq, n, fd, status;
-
-	RL(mkfifo(FIFONAME, 0644));
-	RL(fd = open(FIFONAME, O_RDWR, 0644));
-	RL(kq = kqueue());
-
-	/* spawn child reader */
-	RL(child = fork());
-	if (child == 0) {
-		int sz = read(fd, buffer, 128);
-		if (sz > 0)
-			(void)printf("fifo: child read '%.*s'\n", sz, buffer);
-		_exit(sz <= 0);
+	char *buf;
+	int rfd, wfd, kq;
+	long pipe_buf;
+
+	fifo_support();
+
+	ATF_REQUIRE(mkfifo(fifo_path, 0600) == 0);
+	ATF_REQUIRE((rfd = open(fifo_path, O_RDONLY | O_NONBLOCK)) >= 0);
+	ATF_REQUIRE((wfd = open(fifo_path, O_WRONLY | O_NONBLOCK)) >= 0);
+	ATF_REQUIRE((kq = kqueue()) >= 0);
+
+	/* Get the maximum atomic pipe write size. */
+	pipe_buf = fpathconf(wfd, _PC_PIPE_BUF);
+	ATF_REQUIRE(pipe_buf > 1);
+
+	buf = malloc(pipe_buf);
+	ATF_REQUIRE(buf != NULL);
+
+	EV_SET(&event[0], wfd, EVFILT_WRITE, EV_ADD|EV_ENABLE, 0, 0, 0);
+	ATF_REQUIRE(kevent(kq, event, 1, NULL, 0, NULL) == 0);
+
+	/* We expect the FIFO to be writable. */
+	ATF_REQUIRE(kevent(kq, NULL, 0, event, 1, &to) == 1);
+	ATF_REQUIRE(event[0].ident == (uintptr_t)wfd);
+	ATF_REQUIRE(event[0].filter == EVFILT_WRITE);
+
+	/*
+	 * Write data into the FIFO until it is full, which is
+	 * defined as insufficient buffer space to hold the
+	 * maximum atomic pipe write size.
+	 */
+	while (write(wfd, buf, pipe_buf) != -1) {
+		continue;
 	}
+	ATF_REQUIRE(errno == EAGAIN);
 
-	EV_SET(&event[0], fd, EVFILT_WRITE, EV_ADD|EV_ENABLE, 0, 0, 0);
-	RL(kevent(kq, event, 1, NULL, 0, NULL));
-
-	(void)memset(event, 0, sizeof(event));
-	RL(n = kevent(kq, NULL, 0, event, 1, NULL));
-
-	(void)printf("kevent num %d filt %d flags: %#x, fflags: %#x, "
-	    "data: %" PRId64 "\n", n, event[0].filter, event[0].flags,
-	    event[0].fflags, event[0].data);
+	/* We expect the FIFO to no longer be writable. */
+	ATF_REQUIRE(kevent(kq, NULL, 0, event, 1, &to) == 0);
 
-	ATF_REQUIRE_EQ(event[0].filter, EVFILT_WRITE);
+	/* Read a single byte of data from the FIFO. */
+	ATF_REQUIRE(read(rfd, buf, 1) == 1);
 
-	RL(write(fd, "foo", 3));
-	(void)printf("fifo: wrote 'foo'\n");
-	RL(close(fd));
+	/*
+	 * Because we have read only a single byte out, there will
+	 * be insufficient space for a pipe_buf-sized message, so
+	 * the FIFO should still not be writable.
+	 */
+	ATF_REQUIRE(kevent(kq, NULL, 0, event, 1, &to) == 0);
+
+	/*
+	 * Now read enough so that exactly pipe_buf space should be
+	 * available.  The FIFO should be writable after that.
+	 */
+	ATF_REQUIRE(read(rfd, buf, pipe_buf - 1) == pipe_buf - 1);
+	ATF_REQUIRE(kevent(kq, NULL, 0, event, 1, &to) == 1);
+	ATF_REQUIRE(event[0].ident == (uintptr_t)wfd);
+	ATF_REQUIRE(event[0].filter == EVFILT_WRITE);
+
+	(void)close(wfd);
+	(void)close(rfd);
+	(void)close(kq);
+}
 
-	(void)waitpid(child, &status, 0);
+ATF_TC_CLEANUP(fifo, tc)
+{
+	(void)unlink(fifo_path);
 }
 
 ATF_TP_ADD_TCS(tp)

Reply via email to