Date: Wed, 31 Jul 2013 16:30:01 GMT From: Klaus Weber <fbsd-bugs-2013-1@unix-admin.de> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/178997: Heavy disk I/O may hang system Message-ID: <201307311630.r6VGU1m1026499@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/178997; it has been noted by GNATS.
From: Klaus Weber <fbsd-bugs-2013-1@unix-admin.de>
To: Bruce Evans <brde@optusnet.com.au>, freebsd-gnats-submit@FreeBSD.org
Cc: Klaus Weber <fbsd-bugs-2013-1@unix-admin.de>
Subject: Re: kern/178997: Heavy disk I/O may hang system
Date: Wed, 31 Jul 2013 18:18:59 +0200
--OgqxwSJOaUobr8KG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
After studying style(9), I have now made a revised version of the
patch, relative to 9/STABLE of about 3 weeks ago:
Index: sys/kern/vfs_bio.c
===================================================================
--- sys/kern/vfs_bio.c (revision 253339)
+++ sys/kern/vfs_bio.c (working copy)
@@ -1146,7 +1146,7 @@
if (bo->bo_dirty.bv_cnt > dirtybufthresh + 10) {
(void) VOP_FSYNC(bp->b_vp, MNT_NOWAIT, curthread);
altbufferflushes++;
- } else if (bo->bo_dirty.bv_cnt > dirtybufthresh) {
+ } else if (bdflush_required(bo)) {
BO_LOCK(bo);
/*
* Try to find a buffer to flush.
@@ -1438,6 +1438,24 @@
}
/*
+ * Flushing is deemed necessary if at least one of the following is true:
+ * - bo is hogging more than dirtybufthresh buffers (previous behavior)
+ * - numdirtybuffers exceeds dirtybufthresh
+ * - we are or act as bufdaemon (TDP_NORUNNINGBUF)
+ */
+int
+bdflush_required(struct bufobj *bo)
+{
+ struct thread *td = curthread;
+
+ KASSERT(bo != NULL, ("bdflush_required: NULL bo"));
+
+ return ((bo->bo_dirty.bv_cnt > dirtybufthresh) ||
+ (numdirtybuffers > dirtybufthresh) ||
+ (td && (td->td_pflags & TDP_NORUNNINGBUF)));
+}
+
+/*
* brelse:
*
* Release a busy buffer and, if requested, free its resources. The
Index: sys/sys/buf.h
===================================================================
--- sys/sys/buf.h (revision 253339)
+++ sys/sys/buf.h (working copy)
@@ -486,6 +486,7 @@
void bdata2bio(struct buf *bp, struct bio *bip);
void bwillwrite(void);
int buf_dirty_count_severe(void);
+int bdflush_required(struct bufobj *bo);
void bremfree(struct buf *);
void bremfreef(struct buf *); /* XXX Force bremfree, only for nfs. */
int bread(struct vnode *, daddr_t, int, struct ucred *, struct buf **);
Index: sys/ufs/ffs/ffs_snapshot.c
===================================================================
--- sys/ufs/ffs/ffs_snapshot.c (revision 253339)
+++ sys/ufs/ffs/ffs_snapshot.c (working copy)
@@ -2161,7 +2161,7 @@
struct buf *nbp;
int bp_bdskip;
- if (bo->bo_dirty.bv_cnt <= dirtybufthresh)
+ if (!bdflush_required(bo))
return;
td = curthread;
[End of patch]
Besides being more concise, I hope this patch no longer has
style-issues, although I was not sure regarding the indention of the
second and third line of the return statement. I chose to left-align
the condtions.
I have also tested limiting numdirtybuffers to hidirtybuffers (instead
of dirtybufthresh), but that does not prevent the system from hanging.
I would appreciate if someone could review and eventually commit this
patch (or a variant thereof), and then close this bug.
Thanks, Klaus
--OgqxwSJOaUobr8KG
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="bo_dirty.bv_cnt_FIX_FINAL.patch"
Index: sys/kern/vfs_bio.c
===================================================================
--- sys/kern/vfs_bio.c (revision 253339)
+++ sys/kern/vfs_bio.c (working copy)
@@ -1146,7 +1146,7 @@
if (bo->bo_dirty.bv_cnt > dirtybufthresh + 10) {
(void) VOP_FSYNC(bp->b_vp, MNT_NOWAIT, curthread);
altbufferflushes++;
- } else if (bo->bo_dirty.bv_cnt > dirtybufthresh) {
+ } else if (bdflush_required(bo)) {
BO_LOCK(bo);
/*
* Try to find a buffer to flush.
@@ -1438,6 +1438,24 @@
}
/*
+ * Flushing is deemed necessary if at least one of the following is true:
+ * - bo is hogging more than dirtybufthresh buffers (previous behavior)
+ * - numdirtybuffers exceeds dirtybufthresh
+ * - we are or act as bufdaemon (TDP_NORUNNINGBUF)
+ */
+int
+bdflush_required(struct bufobj *bo)
+{
+ struct thread *td = curthread;
+
+ KASSERT(bo != NULL, ("bdflush_required: NULL bo"));
+
+ return ((bo->bo_dirty.bv_cnt > dirtybufthresh) ||
+ (numdirtybuffers > dirtybufthresh) ||
+ (td && (td->td_pflags & TDP_NORUNNINGBUF)));
+}
+
+/*
* brelse:
*
* Release a busy buffer and, if requested, free its resources. The
Index: sys/sys/buf.h
===================================================================
--- sys/sys/buf.h (revision 253339)
+++ sys/sys/buf.h (working copy)
@@ -486,6 +486,7 @@
void bdata2bio(struct buf *bp, struct bio *bip);
void bwillwrite(void);
int buf_dirty_count_severe(void);
+int bdflush_required(struct bufobj *bo);
void bremfree(struct buf *);
void bremfreef(struct buf *); /* XXX Force bremfree, only for nfs. */
int bread(struct vnode *, daddr_t, int, struct ucred *, struct buf **);
Index: sys/ufs/ffs/ffs_snapshot.c
===================================================================
--- sys/ufs/ffs/ffs_snapshot.c (revision 253339)
+++ sys/ufs/ffs/ffs_snapshot.c (working copy)
@@ -2161,7 +2161,7 @@
struct buf *nbp;
int bp_bdskip;
- if (bo->bo_dirty.bv_cnt <= dirtybufthresh)
+ if (!bdflush_required(bo))
return;
td = curthread;
--OgqxwSJOaUobr8KG--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201307311630.r6VGU1m1026499>