Date: Tue, 31 Mar 2015 09:40:58 -0400 From: John Baldwin <jhb@freebsd.org> To: Randall Stewart <rrs@netflix.com> Cc: svn-src-head@freebsd.org, Randall Stewart <rrs@freebsd.org>, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r280785 - in head/sys: kern netgraph/atm/sscop netgraph/atm/uni sys Message-ID: <5710983.pHS9DoOdY8@ralph.baldwin.cx> In-Reply-To: <26047F0C-A975-4DAC-9077-31B5EC4902DA@netflix.com> References: <201503281250.t2SCoOkt020297@svn.freebsd.org> <32487399.PTq7ESkWJT@ralph.baldwin.cx> <26047F0C-A975-4DAC-9077-31B5EC4902DA@netflix.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, March 30, 2015 06:26:25 PM Randall Stewart wrote: >=20 > On Mar 30, 2015, at 9:16 AM, John Baldwin <jhb@freebsd.org> wrote: >=20 > > On Saturday, March 28, 2015 12:50:24 PM Randall Stewart wrote: > >> Author: rrs > >> Date: Sat Mar 28 12:50:24 2015 > >> New Revision: 280785 > >> URL: https://svnweb.freebsd.org/changeset/base/280785 > >>=20 > >> Log: > >> Change the callout to supply -1 to indicate we are not changing > >> CPU, also add protection against invalid CPU's as well as > >> split c_flags and c_iflags so that if a user plays with the activ= e > >> flag (the one expected to be played with by callers in MPSAFE) wi= thout > >> a lock, it won't adversely affect the callout system by causing a= corrupt > >> list. This also means that all callers need to use the macros and= *not* > >> play with the falgs directly (like netgraph used to). > >>=20 > >> Differential Revision: htts://reviews.freebsd.org/D1894 > >> Reviewed by: .. timed out but looked at by jhb, imp, adrian hsela= sky > >> tested by hiren and netflix. > >> Sponsored by:=09Netflix Inc. > >=20 > > Please use NOCPU rather than -1 directly for the CPU field when not= > > moving a callout. > >=20 >=20 > John: >=20 > I have made *all* of your suggested changes, adopting the comments an= d > moving migration to kern_timeout.c.. thanks.. >=20 > Now as to the=20 >=20 > -1 -> NOCPU >=20 > This is like pulling on a string on your sweater.. the only sensible = solution that > I could come up with after chatting with Lawrence is to add=20 > #include <sys/proc.h> > to everyone that uses the callout.h and does not have it already=E2=80= =A6 (putting it into callout.h does not work) .. sigh.. >=20 > Now for this cosmetic change I end up with the following changes (and= as yet I have > not built LINT or universe so there may be more).. I have spent about= 2 hours on this > so far and I can at least build a kernel with the change for amd64 :-= 0 >=20 > Here is what has to change, do you really think that this is worth it= ? >=20 > Note I did not look into moving NOCPU in proc.h it says it means no C= PU is present > which is sort of the meaning we want.. I am not sure if the define co= uld be moved .. but > that too may be yet another string... >=20 > Is this worth it, or do you have another idea on how best to do this?= ?? Ugh. :( I guess leave it at -1 for now. Another alternative would be = to move NOCPU to <sys/param.h> (at least for the kernel) if bde@ would all= ow it. I don't think changing all those files is appropriate. (Also, <sys/proc.h> seems like an odd place for NOCPU now, it should re= ally be in <sys/smp.h> if not param.h. I understand why it was first added in = proc.h, but it is now used in many more places than just td_oncpu.) --=20 John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5710983.pHS9DoOdY8>