Skip to content

Commit ed16fa7

Browse files
author
Matt Macy
committed
Replace cv_{timed}wait_sig with cv_{timed}wait_idle where appropriate
There are a number of places where cv_?_sig is used simply for accounting purposes but the surrounding code has no ability to cope with actually receiving a signal. On FreeBSD it is possible to send signals to individual kernel threads so this could enable undesirable behavior. This patch adds routines on Linux that will do the same idle accounting as _sig without making the task interruptible. On FreeBSD cv_*_idle are all aliases for cv_* Signed-off-by: Matt Macy <[email protected]>
1 parent 6fffc88 commit ed16fa7

File tree

11 files changed

+73
-25
lines changed

11 files changed

+73
-25
lines changed

include/os/freebsd/spl/sys/condvar.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,14 @@ cv_timedwait_sig(kcondvar_t *cvp, kmutex_t *mp, clock_t timo)
142142
return (1);
143143
}
144144

145-
#define cv_timedwait_io cv_timedwait
146-
#define cv_timedwait_sig_io cv_timedwait_sig
145+
#define cv_timedwait_io cv_timedwait
146+
#define cv_timedwait_idle cv_timedwait
147+
#define cv_timedwait_sig_io cv_timedwait_sig
148+
#define cv_wait_io cv_wait
149+
#define cv_wait_io_sig cv_wait_sig
150+
#define cv_wait_idle cv_wait
151+
#define cv_timedwait_io_hires cv_timedwait_hires
152+
#define cv_timedwait_idle_hires cv_timedwait_hires
147153

148154
static inline int
149155
cv_timedwait_hires(kcondvar_t *cvp, kmutex_t *mp, hrtime_t tim, hrtime_t res,

include/os/freebsd/zfs/sys/zfs_context_os.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,6 @@
4141
#include <sys/ccompat.h>
4242
#include <linux/types.h>
4343

44-
#define cv_wait_io(cv, mp) cv_wait(cv, mp)
45-
#define cv_wait_io_sig(cv, mp) cv_wait_sig(cv, mp)
46-
4744
#define cond_resched() kern_yield(PRI_USER)
4845

4946
#define taskq_create_sysdc(a, b, d, e, p, dc, f) \
@@ -84,7 +81,6 @@ typedef int fstrans_cookie_t;
8481
#define signal_pending(x) SIGPENDING(x)
8582
#define current curthread
8683
#define thread_join(x)
87-
#define cv_wait_io(cv, mp) cv_wait(cv, mp)
8884
typedef struct opensolaris_utsname utsname_t;
8985
extern utsname_t *utsname(void);
9086
extern int spa_import_rootpool(const char *name, bool checkpointrewind);

include/os/linux/spl/sys/condvar.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,22 +80,27 @@ extern void __cv_init(kcondvar_t *, char *, kcv_type_t, void *);
8080
extern void __cv_destroy(kcondvar_t *);
8181
extern void __cv_wait(kcondvar_t *, kmutex_t *);
8282
extern void __cv_wait_io(kcondvar_t *, kmutex_t *);
83+
extern void __cv_wait_idle(kcondvar_t *, kmutex_t *);
8384
extern int __cv_wait_io_sig(kcondvar_t *, kmutex_t *);
8485
extern int __cv_wait_sig(kcondvar_t *, kmutex_t *);
8586
extern int __cv_timedwait(kcondvar_t *, kmutex_t *, clock_t);
8687
extern int __cv_timedwait_io(kcondvar_t *, kmutex_t *, clock_t);
8788
extern int __cv_timedwait_sig(kcondvar_t *, kmutex_t *, clock_t);
89+
extern int __cv_timedwait_idle(kcondvar_t *, kmutex_t *, clock_t);
8890
extern int cv_timedwait_hires(kcondvar_t *, kmutex_t *, hrtime_t,
8991
hrtime_t res, int flag);
9092
extern int cv_timedwait_sig_hires(kcondvar_t *, kmutex_t *, hrtime_t,
9193
hrtime_t res, int flag);
94+
extern int cv_timedwait_idle_hires(kcondvar_t *, kmutex_t *, hrtime_t,
95+
hrtime_t res, int flag);
9296
extern void __cv_signal(kcondvar_t *);
9397
extern void __cv_broadcast(kcondvar_t *c);
9498

9599
#define cv_init(cvp, name, type, arg) __cv_init(cvp, name, type, arg)
96100
#define cv_destroy(cvp) __cv_destroy(cvp)
97101
#define cv_wait(cvp, mp) __cv_wait(cvp, mp)
98102
#define cv_wait_io(cvp, mp) __cv_wait_io(cvp, mp)
103+
#define cv_wait_idle(cvp, mp) __cv_wait_idle(cvp, mp)
99104
#define cv_wait_io_sig(cvp, mp) __cv_wait_io_sig(cvp, mp)
100105
#define cv_wait_sig(cvp, mp) __cv_wait_sig(cvp, mp)
101106
#define cv_signal(cvp) __cv_signal(cvp)
@@ -109,5 +114,7 @@ extern void __cv_broadcast(kcondvar_t *c);
109114
#define cv_timedwait(cvp, mp, t) __cv_timedwait(cvp, mp, t)
110115
#define cv_timedwait_io(cvp, mp, t) __cv_timedwait_io(cvp, mp, t)
111116
#define cv_timedwait_sig(cvp, mp, t) __cv_timedwait_sig(cvp, mp, t)
117+
#define cv_timedwait_idle(cvp, mp, t) __cv_timedwait_idle(cvp, mp, t)
118+
112119

113120
#endif /* _SPL_CONDVAR_H */

include/sys/zfs_context.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,11 +325,15 @@ extern void cv_signal(kcondvar_t *cv);
325325
extern void cv_broadcast(kcondvar_t *cv);
326326

327327
#define cv_timedwait_io(cv, mp, at) cv_timedwait(cv, mp, at)
328+
#define cv_timedwait_idle(cv, mp, at) cv_timedwait(cv, mp, at)
328329
#define cv_timedwait_sig(cv, mp, at) cv_timedwait(cv, mp, at)
329330
#define cv_wait_io(cv, mp) cv_wait(cv, mp)
331+
#define cv_wait_idle(cv, mp) cv_wait(cv, mp)
330332
#define cv_wait_io_sig(cv, mp) cv_wait_sig(cv, mp)
331333
#define cv_timedwait_sig_hires(cv, mp, t, r, f) \
332334
cv_timedwait_hires(cv, mp, t, r, f)
335+
#define cv_timedwait_idle_hires(cv, mp, t, r, f) \
336+
cv_timedwait_hires(cv, mp, t, r, f)
333337

334338
/*
335339
* Thread-specific data

module/os/linux/spl/spl-condvar.c

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,18 @@ __cv_wait_sig(kcondvar_t *cvp, kmutex_t *mp)
198198
}
199199
EXPORT_SYMBOL(__cv_wait_sig);
200200

201+
void
202+
__cv_wait_idle(kcondvar_t *cvp, kmutex_t *mp)
203+
{
204+
sigset_t blocked, saved;
205+
206+
sigfillset(&blocked);
207+
(void) sigprocmask(SIG_BLOCK, &blocked, &saved);
208+
cv_wait_common(cvp, mp, TASK_INTERRUPTIBLE, 0);
209+
(void) sigprocmask(SIG_SETMASK, &saved, NULL);
210+
}
211+
EXPORT_SYMBOL(__cv_wait_idle);
212+
201213
#if defined(HAVE_IO_SCHEDULE_TIMEOUT)
202214
#define spl_io_schedule_timeout(t) io_schedule_timeout(t)
203215
#else
@@ -330,6 +342,21 @@ __cv_timedwait_sig(kcondvar_t *cvp, kmutex_t *mp, clock_t exp_time)
330342
}
331343
EXPORT_SYMBOL(__cv_timedwait_sig);
332344

345+
int
346+
__cv_timedwait_idle(kcondvar_t *cvp, kmutex_t *mp, clock_t exp_time)
347+
{
348+
sigset_t blocked, saved;
349+
int rc;
350+
351+
sigfillset(&blocked);
352+
(void) sigprocmask(SIG_BLOCK, &blocked, &saved);
353+
rc = __cv_timedwait_common(cvp, mp, exp_time,
354+
TASK_INTERRUPTIBLE, 0);
355+
(void) sigprocmask(SIG_SETMASK, &saved, NULL);
356+
357+
return (rc);
358+
}
359+
EXPORT_SYMBOL(__cv_timedwait_idle);
333360
/*
334361
* 'expire_time' argument is an absolute clock time in nanoseconds.
335362
* Return value is time left (expire_time - now) or -1 if timeout occurred.
@@ -427,6 +454,23 @@ cv_timedwait_sig_hires(kcondvar_t *cvp, kmutex_t *mp, hrtime_t tim,
427454
}
428455
EXPORT_SYMBOL(cv_timedwait_sig_hires);
429456

457+
int
458+
cv_timedwait_idle_hires(kcondvar_t *cvp, kmutex_t *mp, hrtime_t tim,
459+
hrtime_t res, int flag)
460+
{
461+
sigset_t blocked, saved;
462+
int rc;
463+
464+
sigfillset(&blocked);
465+
(void) sigprocmask(SIG_BLOCK, &blocked, &saved);
466+
rc = cv_timedwait_hires_common(cvp, mp, tim, res, flag,
467+
TASK_INTERRUPTIBLE);
468+
(void) sigprocmask(SIG_SETMASK, &saved, NULL);
469+
470+
return (rc);
471+
}
472+
EXPORT_SYMBOL(cv_timedwait_idle_hires);
473+
430474
void
431475
__cv_signal(kcondvar_t *cvp)
432476
{

module/zfs/arc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9174,7 +9174,7 @@ l2arc_feed_thread(void *unused)
91749174
cookie = spl_fstrans_mark();
91759175
while (l2arc_thread_exit == 0) {
91769176
CALLB_CPR_SAFE_BEGIN(&cpr);
9177-
(void) cv_timedwait_sig(&l2arc_feed_thr_cv,
9177+
(void) cv_timedwait_idle(&l2arc_feed_thr_cv,
91789178
&l2arc_feed_thr_lock, next);
91799179
CALLB_CPR_SAFE_END(&cpr, &l2arc_feed_thr_lock);
91809180
next = ddi_get_lbolt() + hz;

module/zfs/dbuf.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,7 @@ dbuf_evict_thread(void *unused)
718718
while (!dbuf_evict_thread_exit) {
719719
while (!dbuf_cache_above_lowater() && !dbuf_evict_thread_exit) {
720720
CALLB_CPR_SAFE_BEGIN(&cpr);
721-
(void) cv_timedwait_sig_hires(&dbuf_evict_cv,
721+
(void) cv_timedwait_idle_hires(&dbuf_evict_cv,
722722
&dbuf_evict_lock, SEC2NSEC(1), MSEC2NSEC(1), 0);
723723
CALLB_CPR_SAFE_END(&cpr, &dbuf_evict_lock);
724724
}

module/zfs/mmp.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,7 @@ mmp_thread(void *arg)
671671
}
672672

673673
CALLB_CPR_SAFE_BEGIN(&cpr);
674-
(void) cv_timedwait_sig_hires(&mmp->mmp_thread_cv,
674+
(void) cv_timedwait_idle_hires(&mmp->mmp_thread_cv,
675675
&mmp->mmp_thread_lock, next_time, USEC2NSEC(100),
676676
CALLOUT_FLAG_ABSOLUTE);
677677
CALLB_CPR_SAFE_END(&cpr, &mmp->mmp_thread_lock);

module/zfs/txg.c

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -242,16 +242,11 @@ txg_thread_wait(tx_state_t *tx, callb_cpr_t *cpr, kcondvar_t *cv, clock_t time)
242242
{
243243
CALLB_CPR_SAFE_BEGIN(cpr);
244244

245-
/*
246-
* cv_wait_sig() is used instead of cv_wait() in order to prevent
247-
* this process from incorrectly contributing to the system load
248-
* average when idle.
249-
*/
250245
if (time) {
251-
(void) cv_timedwait_sig(cv, &tx->tx_sync_lock,
246+
(void) cv_timedwait_idle(cv, &tx->tx_sync_lock,
252247
ddi_get_lbolt() + time);
253248
} else {
254-
cv_wait_sig(cv, &tx->tx_sync_lock);
249+
cv_wait_idle(cv, &tx->tx_sync_lock);
255250
}
256251

257252
CALLB_CPR_SAFE_END(cpr, &tx->tx_sync_lock);
@@ -760,7 +755,8 @@ txg_wait_open(dsl_pool_t *dp, uint64_t txg, boolean_t should_quiesce)
760755
if (should_quiesce == B_TRUE) {
761756
cv_wait_io(&tx->tx_quiesce_done_cv, &tx->tx_sync_lock);
762757
} else {
763-
cv_wait_sig(&tx->tx_quiesce_done_cv, &tx->tx_sync_lock);
758+
cv_wait_idle(&tx->tx_quiesce_done_cv,
759+
&tx->tx_sync_lock);
764760
}
765761
}
766762
mutex_exit(&tx->tx_sync_lock);

module/zfs/vdev_trim.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,7 @@ vdev_trim_range(trim_args_t *ta, uint64_t start, uint64_t size)
481481
if (ta->trim_type == TRIM_TYPE_MANUAL) {
482482
while (vd->vdev_trim_rate != 0 && !vdev_trim_should_stop(vd) &&
483483
vdev_trim_calculate_rate(ta) > vd->vdev_trim_rate) {
484-
cv_timedwait_sig(&vd->vdev_trim_io_cv,
484+
cv_timedwait_idle(&vd->vdev_trim_io_cv,
485485
&vd->vdev_trim_io_lock, ddi_get_lbolt() +
486486
MSEC_TO_TICK(10));
487487
}

module/zfs/zthr.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -237,15 +237,10 @@ zthr_procedure(void *arg)
237237
t->zthr_func(t->zthr_arg, t);
238238
mutex_enter(&t->zthr_state_lock);
239239
} else {
240-
/*
241-
* cv_wait_sig() is used instead of cv_wait() in
242-
* order to prevent this process from incorrectly
243-
* contributing to the system load average when idle.
244-
*/
245240
if (t->zthr_sleep_timeout == 0) {
246-
cv_wait_sig(&t->zthr_cv, &t->zthr_state_lock);
241+
cv_wait_idle(&t->zthr_cv, &t->zthr_state_lock);
247242
} else {
248-
(void) cv_timedwait_sig_hires(&t->zthr_cv,
243+
(void) cv_timedwait_idle_hires(&t->zthr_cv,
249244
&t->zthr_state_lock, t->zthr_sleep_timeout,
250245
MSEC2NSEC(1), 0);
251246
}

0 commit comments

Comments
 (0)