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