Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 29 Oct 2019 20:37:59 +0000 (UTC)
From:      Jeff Roberson <jeff@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r354155 - head/sys/kern
Message-ID:  <201910292037.x9TKbx9c025017@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: jeff
Date: Tue Oct 29 20:37:59 2019
New Revision: 354155
URL: https://svnweb.freebsd.org/changeset/base/354155

Log:
  Drop the object lock in vfs_bio and cluster where it is now safe to do so.
  
  Recent changes to busy/valid/dirty have enabled page based synchronization
  and the object lock is no longer required in many cases.
  
  Reviewed by:	kib
  Sponsored by:	Netflix, Intel
  Differential Revision:	https://reviews.freebsd.org/D21597

Modified:
  head/sys/kern/vfs_bio.c
  head/sys/kern/vfs_cluster.c

Modified: head/sys/kern/vfs_bio.c
==============================================================================
--- head/sys/kern/vfs_bio.c	Tue Oct 29 20:28:02 2019	(r354154)
+++ head/sys/kern/vfs_bio.c	Tue Oct 29 20:37:59 2019	(r354155)
@@ -162,7 +162,7 @@ static void vfs_page_set_valid(struct buf *bp, vm_ooff
 static void vfs_page_set_validclean(struct buf *bp, vm_ooffset_t off,
 		vm_page_t m);
 static void vfs_clean_pages_dirty_buf(struct buf *bp);
-static void vfs_setdirty_locked_object(struct buf *bp);
+static void vfs_setdirty_range(struct buf *bp);
 static void vfs_vmio_invalidate(struct buf *bp);
 static void vfs_vmio_truncate(struct buf *bp, int npages);
 static void vfs_vmio_extend(struct buf *bp, int npages, int size);
@@ -955,8 +955,6 @@ vfs_buf_test_cache(struct buf *bp, vm_ooffset_t foff, 
     vm_offset_t size, vm_page_t m)
 {
 
-	VM_OBJECT_ASSERT_LOCKED(m->object);
-
 	/*
 	 * This function and its results are protected by higher level
 	 * synchronization requiring vnode and buf locks to page in and
@@ -2865,7 +2863,6 @@ vfs_vmio_iodone(struct buf *bp)
 
 	bogus = false;
 	iosize = bp->b_bcount - bp->b_resid;
-	VM_OBJECT_WLOCK(obj);
 	for (i = 0; i < bp->b_npages; i++) {
 		resid = ((foff + PAGE_SIZE) & ~(off_t)PAGE_MASK) - foff;
 		if (resid > iosize)
@@ -2876,7 +2873,10 @@ vfs_vmio_iodone(struct buf *bp)
 		 */
 		m = bp->b_pages[i];
 		if (m == bogus_page) {
-			bogus = true;
+			if (bogus == false) {
+				bogus = true;
+				VM_OBJECT_RLOCK(obj);
+			}
 			m = vm_page_lookup(obj, OFF_TO_IDX(foff));
 			if (m == NULL)
 				panic("biodone: page disappeared!");
@@ -2900,8 +2900,9 @@ vfs_vmio_iodone(struct buf *bp)
 		foff = (foff + PAGE_SIZE) & ~(off_t)PAGE_MASK;
 		iosize -= resid;
 	}
+	if (bogus)
+		VM_OBJECT_RUNLOCK(obj);
 	vm_object_pip_wakeupn(obj, bp->b_npages);
-	VM_OBJECT_WUNLOCK(obj);
 	if (bogus && buf_mapped(bp)) {
 		BUF_CHECK_MAPPED(bp);
 		pmap_qenter(trunc_page((vm_offset_t)bp->b_data),
@@ -3029,7 +3030,6 @@ vfs_vmio_extend(struct buf *bp, int desiredpages, int 
 	 * are not valid for the range covered by the buffer.
 	 */
 	obj = bp->b_bufobj->bo_object;
-	VM_OBJECT_WLOCK(obj);
 	if (bp->b_npages < desiredpages) {
 		/*
 		 * We must allocate system pages since blocking
@@ -3041,11 +3041,13 @@ vfs_vmio_extend(struct buf *bp, int desiredpages, int 
 		 * deadlocks once allocbuf() is called after
 		 * pages are vfs_busy_pages().
 		 */
+		VM_OBJECT_WLOCK(obj);
 		(void)vm_page_grab_pages(obj,
 		    OFF_TO_IDX(bp->b_offset) + bp->b_npages,
 		    VM_ALLOC_SYSTEM | VM_ALLOC_IGN_SBUSY |
 		    VM_ALLOC_NOBUSY | VM_ALLOC_WIRED,
 		    &bp->b_pages[bp->b_npages], desiredpages - bp->b_npages);
+		VM_OBJECT_WUNLOCK(obj);
 		bp->b_npages = desiredpages;
 	}
 
@@ -3076,7 +3078,6 @@ vfs_vmio_extend(struct buf *bp, int desiredpages, int 
 		toff += tinc;
 		tinc = PAGE_SIZE;
 	}
-	VM_OBJECT_WUNLOCK(obj);
 
 	/*
 	 * Step 3, fixup the KVA pmap.
@@ -3656,9 +3657,8 @@ vfs_clean_pages_dirty_buf(struct buf *bp)
 	KASSERT(bp->b_offset != NOOFFSET,
 	    ("vfs_clean_pages_dirty_buf: no buffer offset"));
 
-	VM_OBJECT_WLOCK(bp->b_bufobj->bo_object);
 	vfs_busy_pages_acquire(bp);
-	vfs_setdirty_locked_object(bp);
+	vfs_setdirty_range(bp);
 	for (i = 0; i < bp->b_npages; i++) {
 		noff = (foff + PAGE_SIZE) & ~(off_t)PAGE_MASK;
 		eoff = noff;
@@ -3670,69 +3670,57 @@ vfs_clean_pages_dirty_buf(struct buf *bp)
 		foff = noff;
 	}
 	vfs_busy_pages_release(bp);
-	VM_OBJECT_WUNLOCK(bp->b_bufobj->bo_object);
 }
 
 static void
-vfs_setdirty_locked_object(struct buf *bp)
+vfs_setdirty_range(struct buf *bp)
 {
-	vm_object_t object;
+	vm_offset_t boffset;
+	vm_offset_t eoffset;
 	int i;
 
-	object = bp->b_bufobj->bo_object;
-	VM_OBJECT_ASSERT_WLOCKED(object);
+	/*
+	 * test the pages to see if they have been modified directly
+	 * by users through the VM system.
+	 */
+	for (i = 0; i < bp->b_npages; i++)
+		vm_page_test_dirty(bp->b_pages[i]);
 
 	/*
-	 * We qualify the scan for modified pages on whether the
-	 * object has been flushed yet.
+	 * Calculate the encompassing dirty range, boffset and eoffset,
+	 * (eoffset - boffset) bytes.
 	 */
-	if ((object->flags & OBJ_MIGHTBEDIRTY) != 0) {
-		vm_offset_t boffset;
-		vm_offset_t eoffset;
 
-		/*
-		 * test the pages to see if they have been modified directly
-		 * by users through the VM system.
-		 */
-		for (i = 0; i < bp->b_npages; i++)
-			vm_page_test_dirty(bp->b_pages[i]);
+	for (i = 0; i < bp->b_npages; i++) {
+		if (bp->b_pages[i]->dirty)
+			break;
+	}
+	boffset = (i << PAGE_SHIFT) - (bp->b_offset & PAGE_MASK);
 
-		/*
-		 * Calculate the encompassing dirty range, boffset and eoffset,
-		 * (eoffset - boffset) bytes.
-		 */
-
-		for (i = 0; i < bp->b_npages; i++) {
-			if (bp->b_pages[i]->dirty)
-				break;
+	for (i = bp->b_npages - 1; i >= 0; --i) {
+		if (bp->b_pages[i]->dirty) {
+			break;
 		}
-		boffset = (i << PAGE_SHIFT) - (bp->b_offset & PAGE_MASK);
+	}
+	eoffset = ((i + 1) << PAGE_SHIFT) - (bp->b_offset & PAGE_MASK);
 
-		for (i = bp->b_npages - 1; i >= 0; --i) {
-			if (bp->b_pages[i]->dirty) {
-				break;
-			}
-		}
-		eoffset = ((i + 1) << PAGE_SHIFT) - (bp->b_offset & PAGE_MASK);
+	/*
+	 * Fit it to the buffer.
+	 */
 
-		/*
-		 * Fit it to the buffer.
-		 */
+	if (eoffset > bp->b_bcount)
+		eoffset = bp->b_bcount;
 
-		if (eoffset > bp->b_bcount)
-			eoffset = bp->b_bcount;
+	/*
+	 * If we have a good dirty range, merge with the existing
+	 * dirty range.
+	 */
 
-		/*
-		 * If we have a good dirty range, merge with the existing
-		 * dirty range.
-		 */
-
-		if (boffset < eoffset) {
-			if (bp->b_dirtyoff > boffset)
-				bp->b_dirtyoff = boffset;
-			if (bp->b_dirtyend < eoffset)
-				bp->b_dirtyend = eoffset;
-		}
+	if (boffset < eoffset) {
+		if (bp->b_dirtyoff > boffset)
+			bp->b_dirtyoff = boffset;
+		if (bp->b_dirtyend < eoffset)
+			bp->b_dirtyend = eoffset;
 	}
 }
 
@@ -4472,16 +4460,21 @@ vfs_unbusy_pages(struct buf *bp)
 	int i;
 	vm_object_t obj;
 	vm_page_t m;
+	bool bogus;
 
 	runningbufwakeup(bp);
 	if (!(bp->b_flags & B_VMIO))
 		return;
 
 	obj = bp->b_bufobj->bo_object;
-	VM_OBJECT_WLOCK(obj);
+	bogus = false;
 	for (i = 0; i < bp->b_npages; i++) {
 		m = bp->b_pages[i];
 		if (m == bogus_page) {
+			if (bogus == false) {
+				bogus = true;
+				VM_OBJECT_RLOCK(obj);
+			}
 			m = vm_page_lookup(obj, OFF_TO_IDX(bp->b_offset) + i);
 			if (!m)
 				panic("vfs_unbusy_pages: page missing\n");
@@ -4495,8 +4488,9 @@ vfs_unbusy_pages(struct buf *bp)
 		}
 		vm_page_sunbusy(m);
 	}
+	if (bogus)
+		VM_OBJECT_RUNLOCK(obj);
 	vm_object_pip_wakeupn(obj, bp->b_npages);
-	VM_OBJECT_WUNLOCK(obj);
 }
 
 /*
@@ -4573,7 +4567,6 @@ vfs_busy_pages_acquire(struct buf *bp)
 {
 	int i;
 
-	VM_OBJECT_ASSERT_WLOCKED(bp->b_bufobj->bo_object);
 	for (i = 0; i < bp->b_npages; i++)
 		vm_page_busy_acquire(bp->b_pages[i], VM_ALLOC_SBUSY);
 }
@@ -4583,7 +4576,6 @@ vfs_busy_pages_release(struct buf *bp)
 {
 	int i;
 
-	VM_OBJECT_ASSERT_WLOCKED(bp->b_bufobj->bo_object);
 	for (i = 0; i < bp->b_npages; i++)
 		vm_page_sunbusy(bp->b_pages[i]);
 }
@@ -4616,13 +4608,12 @@ vfs_busy_pages(struct buf *bp, int clear_modify)
 	foff = bp->b_offset;
 	KASSERT(bp->b_offset != NOOFFSET,
 	    ("vfs_busy_pages: no buffer offset"));
-	VM_OBJECT_WLOCK(obj);
 	if ((bp->b_flags & B_CLUSTER) == 0) {
 		vm_object_pip_add(obj, bp->b_npages);
 		vfs_busy_pages_acquire(bp);
 	}
 	if (bp->b_bufsize != 0)
-		vfs_setdirty_locked_object(bp);
+		vfs_setdirty_range(bp);
 	bogus = false;
 	for (i = 0; i < bp->b_npages; i++) {
 		m = bp->b_pages[i];
@@ -4653,7 +4644,6 @@ vfs_busy_pages(struct buf *bp, int clear_modify)
 		}
 		foff = (foff + PAGE_SIZE) & ~(off_t)PAGE_MASK;
 	}
-	VM_OBJECT_WUNLOCK(obj);
 	if (bogus && buf_mapped(bp)) {
 		BUF_CHECK_MAPPED(bp);
 		pmap_qenter(trunc_page((vm_offset_t)bp->b_data),
@@ -4686,8 +4676,6 @@ vfs_bio_set_valid(struct buf *bp, int base, int size)
 	base += (bp->b_offset & PAGE_MASK);
 	n = PAGE_SIZE - (base & PAGE_MASK);
 
-	VM_OBJECT_WLOCK(bp->b_bufobj->bo_object);
-
 	/*
 	 * Busy may not be strictly necessary here because the pages are
 	 * unlikely to be fully valid and the vnode lock will synchronize
@@ -4705,7 +4693,6 @@ vfs_bio_set_valid(struct buf *bp, int base, int size)
 		n = PAGE_SIZE;
 	}
 	vfs_busy_pages_release(bp);
-	VM_OBJECT_WUNLOCK(bp->b_bufobj->bo_object);
 }
 
 /*
@@ -4731,22 +4718,7 @@ vfs_bio_clrbuf(struct buf *bp) 
 	}
 	bp->b_flags &= ~B_INVAL;
 	bp->b_ioflags &= ~BIO_ERROR;
-	VM_OBJECT_WLOCK(bp->b_bufobj->bo_object);
 	vfs_busy_pages_acquire(bp);
-	if ((bp->b_npages == 1) && (bp->b_bufsize < PAGE_SIZE) &&
-	    (bp->b_offset & PAGE_MASK) == 0) {
-		if (bp->b_pages[0] == bogus_page)
-			goto unlock;
-		mask = (1 << (bp->b_bufsize / DEV_BSIZE)) - 1;
-		VM_OBJECT_ASSERT_WLOCKED(bp->b_pages[0]->object);
-		if ((bp->b_pages[0]->valid & mask) == mask)
-			goto unlock;
-		if ((bp->b_pages[0]->valid & mask) == 0) {
-			pmap_zero_page_area(bp->b_pages[0], 0, bp->b_bufsize);
-			bp->b_pages[0]->valid |= mask;
-			goto unlock;
-		}
-	}
 	sa = bp->b_offset & PAGE_MASK;
 	slide = 0;
 	for (i = 0; i < bp->b_npages; i++, sa = 0) {
@@ -4758,7 +4730,6 @@ vfs_bio_clrbuf(struct buf *bp) 
 			continue;
 		j = sa / DEV_BSIZE;
 		mask = ((1 << ((ea - sa) / DEV_BSIZE)) - 1) << j;
-		VM_OBJECT_ASSERT_WLOCKED(bp->b_pages[i]->object);
 		if ((bp->b_pages[i]->valid & mask) == mask)
 			continue;
 		if ((bp->b_pages[i]->valid & mask) == 0)
@@ -4771,11 +4742,10 @@ vfs_bio_clrbuf(struct buf *bp) 
 				}
 			}
 		}
-		bp->b_pages[i]->valid |= mask;
+		vm_page_set_valid_range(bp->b_pages[i], j * DEV_BSIZE,
+		    roundup2(ea - sa, DEV_BSIZE));
 	}
-unlock:
 	vfs_busy_pages_release(bp);
-	VM_OBJECT_WUNLOCK(bp->b_bufobj->bo_object);
 	bp->b_resid = 0;
 }
 
@@ -5186,11 +5156,9 @@ vfs_bio_getpages(struct vnode *vp, vm_page_t *ma, int 
 
 	br_flags = (mp != NULL && (mp->mnt_kern_flag & MNTK_UNMAPPED_BUFS)
 	    != 0) ? GB_UNMAPPED : 0;
-	VM_OBJECT_WLOCK(object);
 again:
 	for (i = 0; i < count; i++)
 		vm_page_busy_downgrade(ma[i]);
-	VM_OBJECT_WUNLOCK(object);
 
 	lbnp = -1;
 	for (i = 0; i < count; i++) {
@@ -5249,11 +5217,9 @@ again:
 		    vm_page_all_valid(m) || i == count - 1,
 		    ("buf %d %p invalid", i, m));
 		if (i == count - 1 && lpart) {
-			VM_OBJECT_WLOCK(object);
 			if (!vm_page_none_valid(m) &&
 			    !vm_page_all_valid(m))
 				vm_page_zero_invalid(m, TRUE);
-			VM_OBJECT_WUNLOCK(object);
 		}
 next_page:;
 	}
@@ -5281,9 +5247,9 @@ end_pages:
 		if (!vm_page_all_valid(ma[i]))
 			redo = true;
 	}
+	VM_OBJECT_WUNLOCK(object);
 	if (redo && error == 0)
 		goto again;
-	VM_OBJECT_WUNLOCK(object);
 	return (error != 0 ? VM_PAGER_ERROR : VM_PAGER_OK);
 }
 

Modified: head/sys/kern/vfs_cluster.c
==============================================================================
--- head/sys/kern/vfs_cluster.c	Tue Oct 29 20:28:02 2019	(r354154)
+++ head/sys/kern/vfs_cluster.c	Tue Oct 29 20:37:59 2019	(r354155)
@@ -417,11 +417,9 @@ cluster_rbuild(struct vnode *vp, u_quad_t filesize, da
 	inc = btodb(size);
 	for (bn = blkno, i = 0; i < run; ++i, bn += inc) {
 		if (i == 0) {
-			VM_OBJECT_WLOCK(tbp->b_bufobj->bo_object);
 			vm_object_pip_add(tbp->b_bufobj->bo_object,
 			    tbp->b_npages);
 			vfs_busy_pages_acquire(tbp);
-			VM_OBJECT_WUNLOCK(tbp->b_bufobj->bo_object);
 		} else {
 			if ((bp->b_npages * PAGE_SIZE) +
 			    round_page(size) > vp->v_mount->mnt_iosize_max) {
@@ -458,13 +456,11 @@ cluster_rbuild(struct vnode *vp, u_quad_t filesize, da
 			 */
 			off = tbp->b_offset;
 			tsize = size;
-			VM_OBJECT_WLOCK(tbp->b_bufobj->bo_object);
 			for (j = 0; tsize > 0; j++) {
 				toff = off & PAGE_MASK;
 				tinc = tsize;
 				if (toff + tinc > PAGE_SIZE)
 					tinc = PAGE_SIZE - toff;
-				VM_OBJECT_ASSERT_WLOCKED(tbp->b_pages[j]->object);
 				if (vm_page_trysbusy(tbp->b_pages[j]) == 0)
 					break;
 				if ((tbp->b_pages[j]->valid &
@@ -482,11 +478,9 @@ clean_sbusy:
 				    j);
 				for (k = 0; k < j; k++)
 					vm_page_sunbusy(tbp->b_pages[k]);
-				VM_OBJECT_WUNLOCK(tbp->b_bufobj->bo_object);
 				bqrelse(tbp);
 				break;
 			}
-			VM_OBJECT_WUNLOCK(tbp->b_bufobj->bo_object);
 
 			/*
 			 * Set a read-ahead mark as appropriate
@@ -506,7 +500,6 @@ clean_sbusy:
 			if (tbp->b_blkno == tbp->b_lblkno) {
 				tbp->b_blkno = bn;
 			} else if (tbp->b_blkno != bn) {
-				VM_OBJECT_WLOCK(tbp->b_bufobj->bo_object);
 				goto clean_sbusy;
 			}
 		}
@@ -517,9 +510,9 @@ clean_sbusy:
 		BUF_KERNPROC(tbp);
 		TAILQ_INSERT_TAIL(&bp->b_cluster.cluster_head,
 			tbp, b_cluster.cluster_entry);
-		VM_OBJECT_WLOCK(tbp->b_bufobj->bo_object);
 		for (j = 0; j < tbp->b_npages; j += 1) {
 			vm_page_t m;
+
 			m = tbp->b_pages[j];
 			if ((bp->b_npages == 0) ||
 			    (bp->b_pages[bp->b_npages-1] != m)) {
@@ -529,7 +522,7 @@ clean_sbusy:
 			if (vm_page_all_valid(m))
 				tbp->b_pages[j] = bogus_page;
 		}
-		VM_OBJECT_WUNLOCK(tbp->b_bufobj->bo_object);
+
 		/*
 		 * Don't inherit tbp->b_bufsize as it may be larger due to
 		 * a non-page-aligned size.  Instead just aggregate using
@@ -547,13 +540,10 @@ clean_sbusy:
 	 * Fully valid pages in the cluster are already good and do not need
 	 * to be re-read from disk.  Replace the page with bogus_page
 	 */
-	VM_OBJECT_WLOCK(bp->b_bufobj->bo_object);
 	for (j = 0; j < bp->b_npages; j++) {
-		VM_OBJECT_ASSERT_WLOCKED(bp->b_pages[j]->object);
 		if (vm_page_all_valid(bp->b_pages[j]))
 			bp->b_pages[j] = bogus_page;
 	}
-	VM_OBJECT_WUNLOCK(bp->b_bufobj->bo_object);
 	if (bp->b_bufsize > bp->b_kvasize)
 		panic("cluster_rbuild: b_bufsize(%ld) > b_kvasize(%d)\n",
 		    bp->b_bufsize, bp->b_kvasize);
@@ -988,7 +978,6 @@ cluster_wbuild(struct vnode *vp, long size, daddr_t st
 			if (tbp->b_flags & B_VMIO) {
 				vm_page_t m;
 
-				VM_OBJECT_WLOCK(tbp->b_bufobj->bo_object);
 				if (i == 0) {
 					vfs_busy_pages_acquire(tbp);
 				} else { /* if not first buffer */
@@ -998,8 +987,6 @@ cluster_wbuild(struct vnode *vp, long size, daddr_t st
 							for (j--; j >= 0; j--)
 								vm_page_sunbusy(
 								    tbp->b_pages[j]);
-							VM_OBJECT_WUNLOCK(
-							    tbp->b_object);
 							bqrelse(tbp);
 							goto finishcluster;
 						}
@@ -1015,7 +1002,6 @@ cluster_wbuild(struct vnode *vp, long size, daddr_t st
 						bp->b_npages++;
 					}
 				}
-				VM_OBJECT_WUNLOCK(tbp->b_bufobj->bo_object);
 			}
 			bp->b_bcount += size;
 			bp->b_bufsize += size;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201910292037.x9TKbx9c025017>