On Tue, Aug 8, 2017 at 3:07 AM, Taylor R Campbell <campbell+netbsd-tech-k...@mumble.net> wrote: >> Date: Mon, 7 Aug 2017 12:40:11 +0900 >> From: Ryota Ozaki <ozak...@netbsd.org> >> >> Thank you for your reviewing! I think I handled all your suggestions. >> Let me know if I missed or misunderstood something. > > Thanks! Looks good. > > Couple tiny nits: > > - `pserialize critical section' ---> `pserialize read section'? > (Although we sometimes say `critical section' in conversation, the > functions are called pserialize_read_enter/exit, and it may not be > immediately clear whether `pserialize critical section' means a > nonexclusive reader or an exclusive writer.) > > - __read_mostly for key_psz?
Thanks. Changed so. > > While skimming the patch, I noticed an odd global variable acq_seq > that is used only under certain #ifdefs and seems to modified without > any synchronization. I wonder what it's there for and whether it is > OK for updates to be lost. I think we can just remove IPSEC_NONBLOCK_ACQUIRE because, I guess, nobody uses the option for long years. > >> BTW can we have an API to assert that we are in a pserialize critical >> section? Sometimes I want to write something like: >> KASSERT(pserialize_in_read_critical_section()); >> rather than writing the constraint in a comment. > > That would be nice to have. It would require for each machine some > way to ask whether the current IPL is at least some specified IPL, > which as far as I know we don't currently have. (Nor do we have any > MI way to compare IPLs (both numerical orders are used, so IPL_VM < > IPL_HIGH may be true or false), nor any MI way to query the current > IPL.) Hmm, IPL things are so MD. Please someone work on it :-| ozaki-r