Re: Untangle proc * in VOP_*

[Martin Pieuchot]

On 23/03/20(Mon) 18:51, Ted Unangst wrote:
>  [...]
> If this is a temporary measure, I think it should include some
> annotation to that effect, so that it doesn't continue spreading.

It isn't, it's to help review or upcoming diffs :o)  It's not to make
this nice it's to tedu it!

Re: Untangle proc * in VOP_*

[Ted Unangst]

On 2020-03-23, Martin Pieuchot wrote:
> Most of the VOP_* methods include an argument of type "struct proc *"
> called `a_p'.  This pointer is always set to `curproc' as confirmed by
> the diff below.  The diff has been though base build with NFS on amd64
> and sparc64 as well as a full port bulk on amd64 and is in the current
> octeon port bulk.
> 
> I'd like to commit it in order to start removing the requirement of
> passing a "struct proc *" pointer which is always set to curproc.  This
> is unnecessary and has spread in many other places in the kernel.
> 
> That makes codes unnecessarily complicated:
>  - a "struct proc *" is carried over just to satisfy already complex
>interfaces
>  - it isn't obvious which "struct proc *" is not the curproc and 
>asserts like KASSERT(p == curproc) are being made 
>  - in many places where curproc is used people put XXX as if it was
>wrong
> 
> So this is just a starting diff, cleanups can start afterward.

If this is a temporary measure, I think it should include some
annotation to that effect, so that it doesn't continue spreading.

Untangle proc * in VOP_*

[Martin Pieuchot]

Most of the VOP_* methods include an argument of type "struct proc *"
called `a_p'.  This pointer is always set to `curproc' as confirmed by
the diff below.  The diff has been though base build with NFS on amd64
and sparc64 as well as a full port bulk on amd64 and is in the current
octeon port bulk.

I'd like to commit it in order to start removing the requirement of
passing a "struct proc *" pointer which is always set to curproc.  This
is unnecessary and has spread in many other places in the kernel.

That makes codes unnecessarily complicated:
 - a "struct proc *" is carried over just to satisfy already complex
   interfaces
 - it isn't obvious which "struct proc *" is not the curproc and 
   asserts like KASSERT(p == curproc) are being made 
 - in many places where curproc is used people put XXX as if it was
   wrong

So this is just a starting diff, cleanups can start afterward.

Ok?

Index: kern/vfs_vops.c
===
RCS file: /cvs/src/sys/kern/vfs_vops.c,v
retrieving revision 1.25
diff -u -p -r1.25 vfs_vops.c
--- kern/vfs_vops.c 14 Feb 2020 11:57:56 -  1.25
+++ kern/vfs_vops.c 16 Mar 2020 14:16:42 -
@@ -145,6 +145,8 @@ VOP_OPEN(struct vnode *vp, int mode, str
a.a_cred = cred;
a.a_p = p;
 
+   KASSERT(p == curproc);
+
if (vp->v_op->vop_open == NULL)
return (EOPNOTSUPP);
 
@@ -164,6 +166,7 @@ VOP_CLOSE(struct vnode *vp, int fflag, s
a.a_cred = cred;
a.a_p = p;
 
+   KASSERT(p == curproc);
ASSERT_VP_ISLOCKED(vp);
 
if (vp->v_op->vop_close == NULL)
@@ -184,6 +187,7 @@ VOP_ACCESS(struct vnode *vp, int mode, s
a.a_cred = cred;
a.a_p = p;
 
+   KASSERT(p == curproc);
ASSERT_VP_ISLOCKED(vp);
 
if (vp->v_op->vop_access == NULL)
@@ -202,6 +206,7 @@ VOP_GETATTR(struct vnode *vp, struct vat
a.a_cred = cred;
a.a_p = p;
 
+   KASSERT(p == curproc);
if (vp->v_op->vop_getattr == NULL)
return (EOPNOTSUPP);
 
@@ -219,6 +224,7 @@ VOP_SETATTR(struct vnode *vp, struct vat
a.a_cred = cred;
a.a_p = p;
 
+   KASSERT(p == curproc);
ASSERT_VP_ISLOCKED(vp);
 
if (vp->v_op->vop_setattr == NULL)
@@ -282,6 +288,7 @@ VOP_IOCTL(struct vnode *vp, u_long comma
a.a_cred = cred;
a.a_p = p;
 
+   KASSERT(p == curproc);
if (vp->v_op->vop_ioctl == NULL)
return (EOPNOTSUPP);
 
@@ -300,6 +307,7 @@ VOP_POLL(struct vnode *vp, int fflag, in
a.a_events = events;
a.a_p = p;
 
+   KASSERT(p == curproc);
if (vp->v_op->vop_poll == NULL)
return (EOPNOTSUPP);
 
@@ -343,6 +351,7 @@ VOP_FSYNC(struct vnode *vp, struct ucred
a.a_waitfor = waitfor;
a.a_p = p;
 
+   KASSERT(p == curproc);
ASSERT_VP_ISLOCKED(vp);
 
if (vp->v_op->vop_fsync == NULL)
@@ -564,6 +573,7 @@ VOP_INACTIVE(struct vnode *vp, struct pr
a.a_vp = vp;
a.a_p = p;
 
+   KASSERT(p == curproc);
ASSERT_VP_ISLOCKED(vp);
 
if (vp->v_op->vop_inactive == NULL)
@@ -580,6 +590,7 @@ VOP_RECLAIM(struct vnode *vp, struct pro
a.a_vp = vp;
a.a_p = p;
 
+   KASSERT(p == curproc);
if (vp->v_op->vop_reclaim == NULL)
return (EOPNOTSUPP);