On 18/05/22(Wed) 15:53, Alexander Bluhm wrote:
> On Tue, May 17, 2022 at 10:44:54AM +1000, David Gwynne wrote:
> > + cookie = SCARG(uap, proc_cookie);
> > + if (pr->ps_kbind_addr == pc) {
> > + membar_datadep_consumer();
> > + if (pr->ps_kbind_cookie != cookie)
> > + goto sigill;
> > + } else {
>
> You must use membar_consumer() here. membar_datadep_consumer() is
> a barrier between reading pointer and pointed data. Only alpha
> requires membar_datadep_consumer() for that, everywhere else it is
> a NOP.
>
> > + mtx_enter(&pr->ps_mtx);
> > + kpc = pr->ps_kbind_addr;
>
> Do we need kpc variable? I would prefer to read explicit
> pr->ps_kbind_addr in the two places where we use it.
>
> I think the logic of barriers and mutexes is correct.
>
> with the suggestions above OK bluhm@
I believe you should go ahead with the current diff. ok with me. Moving
the field under the scope of another lock can be easily done afterward.
