Date: Sun, 11 Feb 2018 20:47:38 +0000 (UTC) From: Jeff Roberson <jeff@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r329139 - user/jeff/numa/sys/kern Message-ID: <201802112047.w1BKlcr1041920@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jeff Date: Sun Feb 11 20:47:38 2018 New Revision: 329139 URL: https://svnweb.freebsd.org/changeset/base/329139 Log: Fix a wakeup race in the bufspace daemon. Rename bufdomain->bd_request to bd_running to better reflect what it means and remove ambiguity with the dirty bd_request. Modified: user/jeff/numa/sys/kern/vfs_bio.c Modified: user/jeff/numa/sys/kern/vfs_bio.c ============================================================================== --- user/jeff/numa/sys/kern/vfs_bio.c Sun Feb 11 20:35:14 2018 (r329138) +++ user/jeff/numa/sys/kern/vfs_bio.c Sun Feb 11 20:47:38 2018 (r329139) @@ -313,6 +313,7 @@ struct bufqueue __exclusive_cache_line bqdirty; struct bufdomain { struct bufqueue bd_cpuq[MAXCPU]; struct bufqueue bd_cleanq; + struct mtx_padalign bd_run_lock; /* Constants */ long bd_maxbufspace; long bd_hibufspace; @@ -323,7 +324,7 @@ struct bufdomain { int bd_lim; /* atomics */ int bd_wanted; - int __aligned(CACHE_LINE_SIZE) bd_request; + int __aligned(CACHE_LINE_SIZE) bd_running; long __aligned(CACHE_LINE_SIZE) bd_bufspace; int __aligned(CACHE_LINE_SIZE) bd_freebuffers; } __aligned(CACHE_LINE_SIZE); @@ -332,6 +333,9 @@ struct bufdomain { #define BD_LOCK(bd) mtx_lock(BD_LOCKPTR((bd))) #define BD_UNLOCK(bd) mtx_unlock(BD_LOCKPTR((bd))) #define BD_ASSERT_LOCKED(bd) mtx_assert(BD_LOCKPTR((bd)), MA_OWNED) +#define BD_RUN_LOCKPTR(bd) (&(bd)->bd_run_lock) +#define BD_RUN_LOCK(bd) mtx_lock(BD_RUN_LOCKPTR((bd))) +#define BD_RUN_UNLOCK(bd) mtx_unlock(BD_RUN_LOCKPTR((bd))) #define BD_DOMAIN(bd) (bd - bdclean) /* Maximum number of clean buffer domains. */ @@ -474,23 +478,53 @@ bdirtyadd(void) } /* - * bufspace_daemonwakeup: + * bufspace_daemon_wakeup: * * Wakeup the daemons responsible for freeing clean bufs. */ static void -bufspace_daemonwakeup(struct bufdomain *bd) +bufspace_daemon_wakeup(struct bufdomain *bd) { - if (atomic_fetchadd_int(&bd->bd_request, 1) == 0) { - BD_LOCK(bd); - bd->bd_request = 1; - wakeup(&bd->bd_request); - BD_UNLOCK(bd); + /* + * avoid the lock if the daemon is running. + */ + if (atomic_fetchadd_int(&bd->bd_running, 1) == 0) { + BD_RUN_LOCK(bd); + bd->bd_running = 1; + wakeup(&bd->bd_running); + BD_RUN_UNLOCK(bd); } } /* + * bufspace_daemon_wait: + * + * Sleep until the domain falls below a limit or one second passes. + */ +static void +bufspace_daemon_wait(struct bufdomain *bd) +{ + /* + * Re-check our limits and sleep. bd_running must be + * cleared prior to checking the limits to avoid missed + * wakeups. The waker will adjust one of bufspace or + * freebuffers prior to checking bd_running. + */ + BD_RUN_LOCK(bd); + atomic_store_int(&bd->bd_running, 0); + if (bd->bd_bufspace < bd->bd_bufspacethresh && + bd->bd_freebuffers > bd->bd_lofreebuffers) { + msleep(&bd->bd_running, BD_RUN_LOCKPTR(bd), PRIBIO|PDROP, + "-", hz); + } else { + /* Avoid spurious wakeups while running. */ + atomic_store_int(&bd->bd_running, 1); + BD_RUN_UNLOCK(bd); + } +} + +/* * bufspace_adjust: * * Adjust the reported bufspace for a KVA managed buffer, possibly @@ -514,7 +548,7 @@ bufspace_adjust(struct buf *bp, int bufsize) /* Wake up the daemon on the transition. */ if (space < bd->bd_bufspacethresh && space + diff >= bd->bd_bufspacethresh) - bufspace_daemonwakeup(bd); + bufspace_daemon_wakeup(bd); } bp->b_bufsize = bufsize; } @@ -544,7 +578,7 @@ bufspace_reserve(struct bufdomain *bd, int size, bool /* Wake up the daemon on the transition. */ if (space < bd->bd_bufspacethresh && new >= bd->bd_bufspacethresh) - bufspace_daemonwakeup(bd); + bufspace_daemon_wakeup(bd); return (0); } @@ -677,17 +711,7 @@ bufspace_daemon(void *arg) } while (bd->bd_bufspace > bd->bd_lobufspace || bd->bd_freebuffers < bd->bd_hifreebuffers); - /* - * Re-check our limits and sleep. - */ - BD_LOCK(bd); - if (bd->bd_bufspace < bd->bd_bufspacethresh && - bd->bd_freebuffers > bd->bd_lofreebuffers) { - bd->bd_request = 0; - msleep(&bd->bd_request, BD_LOCKPTR(bd), PRIBIO|PDROP, - "-", hz); - } else - BD_UNLOCK(bd); + bufspace_daemon_wait(bd); } } @@ -804,7 +828,7 @@ vfs_buf_test_cache(struct buf *bp, vm_ooffset_t foff, } /* Wake up the buffer daemon if necessary */ -static __inline void +static void bd_wakeup(void) { @@ -1504,7 +1528,7 @@ buf_alloc(struct bufdomain *bd) bp = uma_zalloc(buf_zone, M_NOWAIT); if (bp == NULL) { atomic_fetchadd_int(&bd->bd_freebuffers, 1); - bufspace_daemonwakeup(bd); + bufspace_daemon_wakeup(bd); counter_u64_add(numbufallocfails, 1); return (NULL); } @@ -1512,7 +1536,7 @@ buf_alloc(struct bufdomain *bd) * Wake-up the bufspace daemon on transition below threshold. */ if (freebufs == bd->bd_lofreebuffers) - bufspace_daemonwakeup(bd); + bufspace_daemon_wakeup(bd); if (BUF_LOCK(bp, LK_EXCLUSIVE | LK_NOWAIT, NULL) != 0) panic("getnewbuf_empty: Locked buf %p on free queue.", bp); @@ -1706,6 +1730,7 @@ bd_init(struct bufdomain *bd) for (i = 0; i <= mp_maxid; i++) bq_init(&bd->bd_cpuq[i], QUEUE_CLEAN, i, "bufq clean cpu lock"); + mtx_init(&bd->bd_run_lock, "bufspace daemon run lock", NULL, MTX_DEF); } /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201802112047.w1BKlcr1041920>