Date: Wed, 13 Jul 2016 12:26:36 +0000 (UTC) From: Hans Petter Selasky <hselasky@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r302768 - in projects/hps_head: share/man/man9 sys/kern sys/sys Message-ID: <201607131226.u6DCQa7D057020@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: hselasky Date: Wed Jul 13 12:26:36 2016 New Revision: 302768 URL: https://svnweb.freebsd.org/changeset/base/302768 Log: Implement new callout_init_lock_function() callout API function to support locking constructions. The purpose of this function is to allow a function callback to do locking before the callback is called, in order to solve race conditions. The function callback, callout_lock_func_t, is passed two arguments. The first is the callout's callback argument, and the second is an integer, if set, indicates a lock operation, and if cleared, indicates an unlock operation. Some clients of the callout API like the TCP stack use locking constructions, that means one more more locks locked in series. By using callout_init_lock_function() these clients can lock all the locks required in order to atomically to complete their operations, instead of using lock-unlock-lock sequences which open up for races. Modified: projects/hps_head/share/man/man9/Makefile projects/hps_head/share/man/man9/timeout.9 projects/hps_head/sys/kern/kern_timeout.c projects/hps_head/sys/sys/_callout.h projects/hps_head/sys/sys/callout.h Modified: projects/hps_head/share/man/man9/Makefile ============================================================================== --- projects/hps_head/share/man/man9/Makefile Wed Jul 13 11:58:21 2016 (r302767) +++ projects/hps_head/share/man/man9/Makefile Wed Jul 13 12:26:36 2016 (r302768) @@ -1751,6 +1751,7 @@ MLINKS+=timeout.9 callout.9 \ timeout.9 callout_init_mtx.9 \ timeout.9 callout_init_rm.9 \ timeout.9 callout_init_rw.9 \ + timeout.9 callout_init_lock_function.9 \ timeout.9 callout_pending.9 \ timeout.9 callout_reset.9 \ timeout.9 callout_reset_curcpu.9 \ Modified: projects/hps_head/share/man/man9/timeout.9 ============================================================================== --- projects/hps_head/share/man/man9/timeout.9 Wed Jul 13 11:58:21 2016 (r302767) +++ projects/hps_head/share/man/man9/timeout.9 Wed Jul 13 12:26:36 2016 (r302768) @@ -42,6 +42,7 @@ .Nm callout_init_mtx , .Nm callout_init_rm , .Nm callout_init_rw , +.Nm callout_init_lock_function , .Nm callout_pending , .Nm callout_reset , .Nm callout_reset_curcpu , @@ -65,6 +66,7 @@ .Bd -literal typedef void timeout_t (void *); typedef void callout_func_t (void *); +typedef void callout_lock_func_t (void *, int do_lock); .Ed .Ft int .Fn callout_active "struct callout *c" @@ -87,6 +89,8 @@ struct callout_handle handle = CALLOUT_H .Fn callout_init_rm "struct callout *c" "struct rmlock *rm" "int flags" .Ft void .Fn callout_init_rw "struct callout *c" "struct rwlock *rw" "int flags" +.Ft void +.Fn callout_init_lock_function "struct callout *c" "callout_lock_func_t *func" "int flags" .Ft int .Fn callout_pending "struct callout *c" .Ft int @@ -247,6 +251,13 @@ flag. This function is similar to .Fn callout_init_mtx , but it accepts a read/write type of lock. +.Pp +.Ft void +.Fn callout_init_lock_function "struct callout *c" "callout_lock_func_t *func" "int flags" +This function is similar to +.Fn callout_init_mtx , +but it accepts a callback function which does locking and unlocking. +This is useful when more than one lock is involved protecting a structure. .Sh SCHEDULING CALLOUTS .Ft struct callout_handle .Fn timeout "timeout_t *func" "void *arg" "int ticks" Modified: projects/hps_head/sys/kern/kern_timeout.c ============================================================================== --- projects/hps_head/sys/kern/kern_timeout.c Wed Jul 13 11:58:21 2016 (r302767) +++ projects/hps_head/sys/kern/kern_timeout.c Wed Jul 13 12:26:36 2016 (r302768) @@ -136,7 +136,7 @@ struct callout_args { int cpu; /* CPU we're scheduled on */ }; -typedef void callout_mutex_op_t(struct lock_object *); +typedef void callout_mutex_op_t(void *, struct lock_object *); struct callout_mutex_ops { callout_mutex_op_t *lock; @@ -147,7 +147,7 @@ enum { CALLOUT_LC_UNUSED_0, CALLOUT_LC_UNUSED_1, CALLOUT_LC_UNUSED_2, - CALLOUT_LC_UNUSED_3, + CALLOUT_LC_FUNCTION, CALLOUT_LC_SPIN, CALLOUT_LC_MUTEX, CALLOUT_LC_RW, @@ -155,61 +155,75 @@ enum { }; static void -callout_mutex_op_none(struct lock_object *lock) +callout_mutex_op_none(void *arg, struct lock_object *lock) { } static void -callout_mutex_lock(struct lock_object *lock) +callout_function_lock(void *arg, struct lock_object *lock) +{ + + ((callout_lock_func_t *)lock)(arg, 1); +} + +static void +callout_function_unlock(void *arg, struct lock_object *lock) +{ + + ((callout_lock_func_t *)lock)(arg, 0); +} + +static void +callout_mutex_lock(void *arg, struct lock_object *lock) { mtx_lock((struct mtx *)lock); } static void -callout_mutex_unlock(struct lock_object *lock) +callout_mutex_unlock(void *arg, struct lock_object *lock) { mtx_unlock((struct mtx *)lock); } static void -callout_mutex_lock_spin(struct lock_object *lock) +callout_mutex_lock_spin(void *arg, struct lock_object *lock) { mtx_lock_spin((struct mtx *)lock); } static void -callout_mutex_unlock_spin(struct lock_object *lock) +callout_mutex_unlock_spin(void *arg, struct lock_object *lock) { mtx_unlock_spin((struct mtx *)lock); } static void -callout_rm_wlock(struct lock_object *lock) +callout_rm_wlock(void *arg, struct lock_object *lock) { rm_wlock((struct rmlock *)lock); } static void -callout_rm_wunlock(struct lock_object *lock) +callout_rm_wunlock(void *arg, struct lock_object *lock) { rm_wunlock((struct rmlock *)lock); } static void -callout_rw_wlock(struct lock_object *lock) +callout_rw_wlock(void *arg, struct lock_object *lock) { rw_wlock((struct rwlock *)lock); } static void -callout_rw_wunlock(struct lock_object *lock) +callout_rw_wunlock(void *arg, struct lock_object *lock) { rw_wunlock((struct rwlock *)lock); @@ -228,9 +242,9 @@ static const struct callout_mutex_ops ca .lock = callout_mutex_op_none, .unlock = callout_mutex_op_none, }, - [CALLOUT_LC_UNUSED_3] = { - .lock = callout_mutex_op_none, - .unlock = callout_mutex_op_none, + [CALLOUT_LC_FUNCTION] = { + .lock = callout_function_lock, + .unlock = callout_function_unlock, }, [CALLOUT_LC_SPIN] = { .lock = callout_mutex_lock_spin, @@ -251,17 +265,17 @@ static const struct callout_mutex_ops ca }; static inline void -callout_lock_client(int c_flags, struct lock_object *c_lock) +callout_lock_client(int c_flags, void *c_arg, struct lock_object *c_lock) { - callout_mutex_ops[CALLOUT_GET_LC(c_flags)].lock(c_lock); + callout_mutex_ops[CALLOUT_GET_LC(c_flags)].lock(c_arg, c_lock); } static inline void -callout_unlock_client(int c_flags, struct lock_object *c_lock) +callout_unlock_client(int c_flags, void *c_arg, struct lock_object *c_lock) { - callout_mutex_ops[CALLOUT_GET_LC(c_flags)].unlock(c_lock); + callout_mutex_ops[CALLOUT_GET_LC(c_flags)].unlock(c_arg, c_lock); } /* @@ -788,7 +802,7 @@ softclock_call_cc(struct callout *c, str /* unlocked region for switching locks */ - callout_lock_client(c_flags, c_lock); + callout_lock_client(c_flags, c_arg, c_lock); /* * Check if the callout may have been cancelled while @@ -798,7 +812,7 @@ softclock_call_cc(struct callout *c, str */ CC_LOCK(cc); if (cc_exec_cancel(cc, direct)) { - callout_unlock_client(c_flags, c_lock); + callout_unlock_client(c_flags, c_arg, c_lock); goto skip_cc_locked; } if (c_lock == &Giant.lock_object) { @@ -859,7 +873,7 @@ softclock_call_cc(struct callout *c, str * "c->c_flags": */ if ((c_flags & CALLOUT_RETURNUNLOCKED) == 0) - callout_unlock_client(c_flags, c_lock); + callout_unlock_client(c_flags, c_arg, c_lock); CC_LOCK(cc); @@ -1203,13 +1217,13 @@ callout_reset_sbt_on(struct callout *c, int callout_schedule_on(struct callout *c, int to_ticks, int cpu) { - return callout_reset_on(c, to_ticks, c->c_func, c->c_arg, cpu); + return (callout_reset_on(c, to_ticks, c->c_func, c->c_arg, cpu)); } int callout_schedule(struct callout *c, int to_ticks) { - return callout_reset_on(c, to_ticks, c->c_func, c->c_arg, c->c_cpu); + return (callout_reset_on(c, to_ticks, c->c_func, c->c_arg, c->c_cpu)); } int @@ -1240,8 +1254,6 @@ callout_drain(struct callout *c) WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, "Draining callout"); - callout_lock_client(c->c_flags, c->c_lock); - /* at this point the "c->c_cpu" field is not changing */ retval = callout_async_drain(c, &callout_drain_function); @@ -1256,12 +1268,6 @@ callout_drain(struct callout *c) cc = callout_lock(c); direct = ((c->c_flags & CALLOUT_DIRECT) != 0); - /* - * We've gotten our callout CPU lock, it is safe to - * drop the initial lock: - */ - callout_unlock_client(c->c_flags, c->c_lock); - /* Wait for drain to complete */ while (cc_exec_curr(cc, direct) == c) { msleep_spin(&callout_drain_function, @@ -1269,8 +1275,6 @@ callout_drain(struct callout *c) } CC_UNLOCK(cc); - } else { - callout_unlock_client(c->c_flags, c->c_lock); } CTR4(KTR_CALLOUT, "%s: %p func %p arg %p", @@ -1284,18 +1288,34 @@ void callout_init(struct callout *c, int mpsafe) { if (mpsafe) { - _callout_init_lock(c, NULL, CALLOUT_RETURNUNLOCKED); + callout_init_lock_object(c, NULL, CALLOUT_RETURNUNLOCKED); } else { - _callout_init_lock(c, &Giant.lock_object, 0); + callout_init_lock_object(c, &Giant.lock_object, 0); } } void -_callout_init_lock(struct callout *c, struct lock_object *lock, int flags) +callout_init_lock_function(struct callout *c, callout_lock_func_t *lock_fn, int flags) +{ + bzero(c, sizeof *c); + + KASSERT((flags & ~CALLOUT_RETURNUNLOCKED) == 0, + ("callout_init_lock_function: bad flags 0x%08x", flags)); + KASSERT(lock_fn != NULL, + ("callout_init_lock_function: lock function is NULL")); + flags &= CALLOUT_RETURNUNLOCKED; + flags |= CALLOUT_SET_LC(CALLOUT_LC_FUNCTION); + c->c_lock = (struct lock_object *)lock_fn; + c->c_flags = flags; + c->c_cpu = timeout_cpu; +} + +void +callout_init_lock_object(struct callout *c, struct lock_object *lock, int flags) { bzero(c, sizeof *c); KASSERT((flags & ~CALLOUT_RETURNUNLOCKED) == 0, - ("callout_init_lock: bad flags 0x%08x", flags)); + ("callout_init_lock_object: bad flags 0x%08x", flags)); flags &= CALLOUT_RETURNUNLOCKED; if (lock != NULL) { struct lock_class *class = LOCK_CLASS(lock); @@ -1308,7 +1328,8 @@ _callout_init_lock(struct callout *c, st else if (class == &lock_class_rw) flags |= CALLOUT_SET_LC(CALLOUT_LC_RW); else - panic("callout_init_lock: Unsupported lock class '%s'\n", class->lc_name); + panic("callout_init_lock_object: Unsupported lock class '%s'\n", + class->lc_name); } else { flags |= CALLOUT_SET_LC(CALLOUT_LC_UNUSED_0); } Modified: projects/hps_head/sys/sys/_callout.h ============================================================================== --- projects/hps_head/sys/sys/_callout.h Wed Jul 13 11:58:21 2016 (r302767) +++ projects/hps_head/sys/sys/_callout.h Wed Jul 13 12:26:36 2016 (r302768) @@ -47,6 +47,7 @@ SLIST_HEAD(callout_slist, callout); TAILQ_HEAD(callout_tailq, callout); typedef void callout_func_t(void *); +typedef void callout_lock_func_t(void *, int); struct callout { union { @@ -58,7 +59,7 @@ struct callout { sbintime_t c_precision; /* delta allowed wrt opt */ void *c_arg; /* function argument */ callout_func_t *c_func; /* function to call */ - struct lock_object *c_lock; /* lock to handle */ + struct lock_object *c_lock; /* pointer to lock handle */ int c_flags; /* state of this entry */ volatile int c_cpu; /* CPU we're scheduled on */ }; Modified: projects/hps_head/sys/sys/callout.h ============================================================================== --- projects/hps_head/sys/sys/callout.h Wed Jul 13 11:58:21 2016 (r302767) +++ projects/hps_head/sys/sys/callout.h Wed Jul 13 12:26:36 2016 (r302768) @@ -75,15 +75,16 @@ struct callout_handle { int callout_drain(struct callout *); int callout_async_drain(struct callout *, callout_func_t *); void callout_init(struct callout *, int); -void _callout_init_lock(struct callout *, struct lock_object *, int); -#define callout_init_mtx(c, mtx, flags) \ - _callout_init_lock((c), ((mtx) != NULL) ? &(mtx)->lock_object : \ +void callout_init_lock_function(struct callout *, callout_lock_func_t *, int); +void callout_init_lock_object(struct callout *, struct lock_object *, int); +#define callout_init_mtx(c, mtx, flags) \ + callout_init_lock_object((c), ((mtx) != NULL) ? &(mtx)->lock_object : \ NULL, (flags)) -#define callout_init_rm(c, rm, flags) \ - _callout_init_lock((c), ((rm) != NULL) ? &(rm)->lock_object : \ +#define callout_init_rm(c, rm, flags) \ + callout_init_lock_object((c), ((rm) != NULL) ? &(rm)->lock_object : \ NULL, (flags)) -#define callout_init_rw(c, rw, flags) \ - _callout_init_lock((c), ((rw) != NULL) ? &(rw)->lock_object : \ +#define callout_init_rw(c, rw, flags) \ + callout_init_lock_object((c), ((rw) != NULL) ? &(rw)->lock_object : \ NULL, (flags)) #define callout_pending(c) ((c)->c_flags & CALLOUT_PENDING) int callout_reset_sbt_on(struct callout *, sbintime_t, sbintime_t,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201607131226.u6DCQa7D057020>