NetBSD Problem Report #57019

From dholland@netbsd.org  Fri Sep 23 01:56:01 2022
Return-Path: <dholland@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id A03A21A9239
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 23 Sep 2022 01:56:01 +0000 (UTC)
Message-Id: <20220923015601.6C1A684D44@mail.netbsd.org>
Date: Fri, 23 Sep 2022 01:56:01 +0000 (UTC)
From: dholland@NetBSD.org
Reply-To: dholland@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: wapbl calls VOP_BMAP on block devices
X-Send-Pr-Version: 3.95

>Number:         57019
>Category:       kern
>Synopsis:       wapbl calls VOP_BMAP on block devices
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Sep 23 02:00:00 +0000 2022
>Originator:     David A. Holland
>Release:        NetBSD 9.99.97 (20220602)
>Organization:
>Environment:
System: NetBSD valkyrie 9.99.97 NetBSD 9.99.97 (VALKYRIE) #10: Thu Aug 18 04:09:11 EDT 2022  dholland@valkyrie:/usr/src/sys/arch/amd64/compile/VALKYRIE amd64
Architecture: x86_64
Machine: amd64
>Description:

There are two calls to VOP_BMAP in vfs_wapbl.c, one in wapbl_start,
one in wapbl_replay_start. Both serve no purpose; it looks like they
are leftover remnants from an attempt to generalize to be able to log
to a regular file instead of to a region on the device. But that
clearly won't work, as it would require every block in the journal be
bmap'd and I can't find any evidence that this happens; there are only
the two calls.

This came to light when PR 57018 reported things broken on zfs; it
seems to me that calling bmap at all is a bug.

>How-To-Repeat:

Code reading.

>Fix:

Preliminary untested patch:

Index: vfs_wapbl.c
===================================================================
RCS file: /cvsroot/src/sys/kern/vfs_wapbl.c,v
retrieving revision 1.109
diff -u -p -r1.109 vfs_wapbl.c
--- vfs_wapbl.c	3 Aug 2021 20:25:43 -0000	1.109
+++ vfs_wapbl.c	23 Sep 2022 01:55:03 -0000
@@ -109,7 +109,6 @@ static inline size_t wapbl_space_free(si
  */
 LIST_HEAD(wapbl_ino_head, wapbl_ino);
 struct wapbl {
-	struct vnode *wl_logvp;	/* r:	log here */
 	struct vnode *wl_devvp;	/* r:	log on this device */
 	struct mount *wl_mount;	/* r:	mountpoint wl is associated with */
 	daddr_t wl_logpbn;	/* r:	Physical block number of start of log */
@@ -491,20 +490,17 @@ wapbl_start_flush_inodes(struct wapbl *w
 }

 int
-wapbl_start(struct wapbl ** wlp, struct mount *mp, struct vnode *vp,
-	daddr_t off, size_t count, size_t blksize, struct wapbl_replay *wr,
+wapbl_start(struct wapbl ** wlp, struct mount *mp, struct vnode *devvp,
+	daddr_t logpbn, size_t count, size_t blksize, struct wapbl_replay *wr,
 	wapbl_flush_fn_t flushfn, wapbl_flush_fn_t flushabortfn)
 {
 	struct wapbl *wl;
-	struct vnode *devvp;
-	daddr_t logpbn;
 	int error;
 	int log_dev_bshift = ilog2(blksize);
 	int fs_dev_bshift = log_dev_bshift;
-	int run;

 	WAPBL_PRINTF(WAPBL_PRINT_OPEN, ("wapbl_start: vp=%p off=%" PRId64
-	    " count=%zu blksize=%zu\n", vp, off, count, blksize));
+	    " count=%zu blksize=%zu\n", devvp, off, count, blksize));

 	if (log_dev_bshift > fs_dev_bshift) {
 		WAPBL_PRINTF(WAPBL_PRINT_OPEN,
@@ -517,7 +513,7 @@ wapbl_start(struct wapbl ** wlp, struct 
 		return ENOSYS;
 	}

-	if (off < 0)
+	if (logpbn < 0)
 		return EINVAL;

 	if (blksize < DEV_BSIZE)
@@ -537,10 +533,6 @@ wapbl_start(struct wapbl ** wlp, struct 
 		return ENOSPC;
 	}

-	if ((error = VOP_BMAP(vp, off, &devvp, &logpbn, &run)) != 0) {
-		return error;
-	}
-
 	wl = wapbl_calloc(1, sizeof(*wl));
 	rw_init(&wl->wl_rwlock);
 	mutex_init(&wl->wl_mtx, MUTEX_DEFAULT, IPL_NONE);
@@ -548,7 +540,6 @@ wapbl_start(struct wapbl ** wlp, struct 
 	TAILQ_INIT(&wl->wl_bufs);
 	SIMPLEQ_INIT(&wl->wl_entries);

-	wl->wl_logvp = vp;
 	wl->wl_devvp = devvp;
 	wl->wl_mount = mp;
 	wl->wl_logpbn = logpbn;
@@ -635,8 +626,10 @@ wapbl_start(struct wapbl ** wlp, struct 
 	for (int i = 0; i < wapbl_journal_iobufs; i++) {
 		struct buf *bp;

-		if ((bp = geteblk(MAXPHYS)) == NULL)
+		if ((bp = geteblk(MAXPHYS)) == NULL) {
+			error = ENOMEM; /* ENOMEM? */
 			goto errout;
+		}

 		mutex_enter(&bufcache_lock);
 		mutex_enter(devvp->v_interlock);
@@ -2045,8 +2038,8 @@ wapbl_print(struct wapbl *wl,
 	struct buf *bp;
 	struct wapbl_entry *we;
 	(*pr)("wapbl %p", wl);
-	(*pr)("\nlogvp = %p, devvp = %p, logpbn = %"PRId64"\n",
-	      wl->wl_logvp, wl->wl_devvp, wl->wl_logpbn);
+	(*pr)("\ndevvp = %p, logpbn = %"PRId64"\n",
+	      wl->wl_devvp, wl->wl_logpbn);
 	(*pr)("circ = %zu, header = %zu, head = %"PRIdMAX" tail = %"PRIdMAX"\n",
 	      wl->wl_circ_size, wl->wl_circ_off,
 	      (intmax_t)wl->wl_head, (intmax_t)wl->wl_tail);
@@ -2966,13 +2959,9 @@ wapbl_replay_start(struct wapbl_replay *
 	if ((off + count) * blksize > vp->v_size)
 		return EINVAL;
 #endif
-	if ((error = VOP_BMAP(vp, off, &devvp, &logpbn, 0)) != 0) {
-		return error;
-	}
-#else /* ! _KERNEL */
+#endif /* ! _KERNEL */
 	devvp = vp;
 	logpbn = off;
-#endif /* ! _KERNEL */

 	scratch = wapbl_alloc(MAXBSIZE);

Home	PR Database Search