Subject: Re: Second patch for lseek() sparse file extension
To: None <reinoud@netbsd.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 09/22/2006 22:14:49
> Dear Takashi,
> 
> On Fri, Sep 22, 2006 at 11:33:17AM +0900, YAMAMOTO Takashi wrote:
> > > -	*(off_t *)retval = fp->f_offset = newoff;
> > > - out:
> > > +	error = VOP_SEEK(vp, fp->f_offset, SCARG(uap, whence),
> > > +	    SCARG(uap, offset), &newoffset, &vattr, cred);
> > 
> > passing vattr seems a bad idea to me.
> > VOP_SEEK (ie. filesystem) itself should have a better idea of
> > attributes than callers.
> 
> i've been puzzling with this yes. The reason i went for this solution is 
> that the origional code retrieved the vattr of the top vnode that userland 
> sees whereas if the file system would issue vattr it would refer to the 
> bottom vnode. I was worried if that could/would screw things up like 
> permissions say on a user-mapping fs. I'd also have to pass the lwp down 
> making yet another argument to VOP_SEEK. Is this a problem or not? Or 
> should the vattr() be queried by the FS without authentication? Then what 
> authentication does a seek need anyway... why is it passed a kauth_cred_t 
> when it is not used.

getattr() was necessary for the caller to resolve SEEK_END.
because you pushed it to the VOP_SEEK, it isn't necessary anymore.

IMO, passing lwp around here is nonsense.  curlwp is enough.
i don't know the intention of the credential argument of VOP_SEEK.

> Furthermore in the VOP_SEEK(9) docs it states that the node needs to be 
> locked on entry and left locked. The current configuration file doesn't 
> enforce it on lockdebug kernels but i think it could be added without 
> problems.

i always feel vnodeops(9) is not reliable.
don't believe it without consulting the code. :-)

YAMAMOTO Takashi