On Tue, Sep 15, 2020 at 04:38:45PM +0200, Mark Kettenis wrote: > > Date: Tue, 15 Sep 2020 12:34:07 +0200 > > From: Martin Pieuchot <m...@openbsd.org> > > > > Many functions in the kernel take a "struct proc *" as argument. When > > reviewing diffs or reading the signature of such functions it is not > > clear if this pointer can be any thread or if it is, like in many cases, > > pointing to `curproc'. > > > > This distinction matters when it comes to reading/writing members of > > this "struct proc" and that's why a growing number of functions start > > with the following idiom: > > > > KASSERT(p == curproc); > > > > This is verbose and redundant, so I suggested to always use `curproc' > > and stop passing a "struct proc *" as argument when a function isn't > > meant to modify any thread. claudio@ raised a concern of performance > > claiming that `curproc' isn't always cheap. Is it still true? Does > > the KASSERT()s make us pay the cost anyhow? > > Right, because our kernel has DIAGNOSTIC enabled. > > > If that's the case can we adopt a convention to help review functions > > that take a "struct proc *" but only mean `curproc'? What about naming > > this parameter `curp' instead of `p'? > > That'll result in quite a bit of churn. I'd really like to avoid > doing that.
I think the conclusion between the people here at k2k20 is that we should drop the argument from the function and use curproc in the functions (assigning curproc = p early on). This way there will be no way of using the wrong proc. -- :wq Claudio
