Date: Fri, 5 Jul 2019 08:49:19 -0700 From: John Baldwin <jhb@FreeBSD.org> To: Hans Petter Selasky <hselasky@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: Re: svn commit: r349763 - in stable/12/sys: kern sys Message-ID: <97b7657c-53b9-f3d2-f31a-5e56343da71d@FreeBSD.org> In-Reply-To: <201907051226.x65CQUev056366@repo.freebsd.org> References: <201907051226.x65CQUev056366@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 7/5/19 5:26 AM, Hans Petter Selasky wrote: > Author: hselasky > Date: Fri Jul 5 12:26:30 2019 > New Revision: 349763 > URL: https://svnweb.freebsd.org/changeset/base/349763 > > Log: > MFC r340404, r340415, r340417, r340419 and r340420: > Synchronize epoch(9) code with -head to make merging new patches > and features easier. > The FreeBSD version has been bumped to force recompilation of > external kernel modules. > > Sponsored by: Mellanox Technologies > > MFC r340404: > Uninline epoch(9) entrance and exit. There is no proof that modern > processors would benefit from avoiding a function call, but bloating > code. In fact, clang created an uninlined real function for many > object files in the network stack. > > - Move epoch_private.h into subr_epoch.c. Code copied exactly, avoiding > any changes, including style(9). > - Remove private copies of critical_enter/exit. > > Reviewed by: kib, jtl > Differential Revision: https://reviews.freebsd.org/D17879 > > MFC r340415: > The dualism between epoch_tracker and epoch_thread is fragile and > unnecessary. So, expose CK types to kernel and use a single normal > structure for epoch_tracker. > > Reviewed by: jtl, gallatin > > MFC r340417: > With epoch not inlined, there is no point in using _lite KPI. While here, > remove some unnecessary casts. > > MFC r340419: > style(9), mostly adjusting overly long lines. > > MFC r340420: > epoch(9) revert r340097 - no longer a need for multiple sections per cpu > > I spoke with Samy Bahra and recent changes to CK to make ck_epoch_call and > ck_epoch_poll not modify the record have eliminated the need for this. How does this not break the module KBI? You've removed epoch_*_KBI symbols used by existing modules, and you appear to have changed the size of the 'struct epoch_tracker' object that existing modules allocate on the stack and pass to functions in the kernel. Bumping __FreeBSD_version is not sufficient cover to break the KBI of widely used interfaces in stable (while we don't enforce KBI for all parts of the kernel, locking primitives is one of the things we can't break). > Modified: stable/12/sys/sys/epoch.h > ============================================================================== > --- stable/12/sys/sys/epoch.h Fri Jul 5 10:31:37 2019 (r349762) > +++ stable/12/sys/sys/epoch.h Fri Jul 5 12:26:30 2019 (r349763) > @@ -29,10 +29,17 @@ > > #ifndef _SYS_EPOCH_H_ > #define _SYS_EPOCH_H_ > + > +struct epoch_context { > + void *data[2]; > +} __aligned(sizeof(void *)); > + > +typedef struct epoch_context *epoch_context_t; > + > #ifdef _KERNEL > #include <sys/lock.h> > #include <sys/pcpu.h> > -#endif > +#include <ck_epoch.h> > > struct epoch; > typedef struct epoch *epoch_t; > @@ -43,22 +50,19 @@ typedef struct epoch *epoch_t; > extern epoch_t global_epoch; > extern epoch_t global_epoch_preempt; > > -struct epoch_context { > - void *data[2]; > -} __aligned(sizeof(void *)); > - > -typedef struct epoch_context *epoch_context_t; > - > - > struct epoch_tracker { > - void *datap[3]; > -#ifdef EPOCH_TRACKER_DEBUG > - int datai[5]; > -#else > - int datai[1]; > +#ifdef EPOCH_TRACKER_DEBUG > +#define EPOCH_MAGIC0 0xFADECAFEF00DD00D > +#define EPOCH_MAGIC1 0xBADDBABEDEEDFEED > + uint64_t et_magic_pre; > #endif > + TAILQ_ENTRY(epoch_tracker) et_link; > + struct thread *et_td; > + ck_epoch_section_t et_section; > +#ifdef EPOCH_TRACKER_DEBUG > + uint64_t et_magic_post; > +#endif > } __aligned(sizeof(void *)); > - > typedef struct epoch_tracker *epoch_tracker_t; > > epoch_t epoch_alloc(int flags); > @@ -68,26 +72,15 @@ void epoch_wait_preempt(epoch_t epoch); > void epoch_call(epoch_t epoch, epoch_context_t ctx, void (*callback) (epoch_context_t)); > int in_epoch(epoch_t epoch); > int in_epoch_verbose(epoch_t epoch, int dump_onfail); > -#ifdef _KERNEL > DPCPU_DECLARE(int, epoch_cb_count); > DPCPU_DECLARE(struct grouptask, epoch_cb_task); > #define EPOCH_MAGIC0 0xFADECAFEF00DD00D > #define EPOCH_MAGIC1 0xBADDBABEDEEDFEED > > -void epoch_enter_preempt_KBI(epoch_t epoch, epoch_tracker_t et); > -void epoch_exit_preempt_KBI(epoch_t epoch, epoch_tracker_t et); > -void epoch_enter_KBI(epoch_t epoch); > -void epoch_exit_KBI(epoch_t epoch); > +void epoch_enter_preempt(epoch_t epoch, epoch_tracker_t et); > +void epoch_exit_preempt(epoch_t epoch, epoch_tracker_t et); > +void epoch_enter(epoch_t epoch); > +void epoch_exit(epoch_t epoch); > > - > -#if defined(KLD_MODULE) && !defined(KLD_TIED) > -#define epoch_enter_preempt(e, t) epoch_enter_preempt_KBI((e), (t)) > -#define epoch_exit_preempt(e, t) epoch_exit_preempt_KBI((e), (t)) > -#define epoch_enter(e) epoch_enter_KBI((e)) > -#define epoch_exit(e) epoch_exit_KBI((e)) > -#else > -#include <sys/epoch_private.h> > -#endif /* KLD_MODULE */ > - > -#endif /* _KERNEL */ > -#endif > +#endif /* _KERNEL */ > +#endif /* _SYS_EPOCH_H_ */ > -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?97b7657c-53b9-f3d2-f31a-5e56343da71d>