Subject: Re: Second patch for lseek() sparse file extension
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Reinoud Zandijk <reinoud@netbsd.org>
List: tech-kern
Date: 09/22/2006 14:39:14
--WIyZ46R2i8wDzkSu
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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.

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.

> >  vop_seek {
> >  	IN struct vnode *vp;
> > -	IN off_t oldoff;
> > -	IN off_t newoff;
> > +	IN off_t oldoffset;
> > +	IN int whence;
> > +	IN off_t givenoffset;
> > +	OUT off_t *newoffset;
> > +	IN struct vattr *vattr;
> >  	IN kauth_cred_t cred;
> >  };
> 
> you need to introduce some locking because you are
> introducing a race with block allocation.

see above on the locking of the node remark.

Thanks for the feedback,
Reinoud

--WIyZ46R2i8wDzkSu
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (NetBSD)

iQEVAwUBRRPZa4KcNwBDyKpoAQILugf/er6BZVDzLNm7EaBuIAUD4kR3c7E7pQUl
HBIjxVewaJf8s1ssmqarPjabQLgI/9TCejzbuXDrDiiOMB2YGC6JQlQ2RF+q8BGH
OEADlawPj1HXIh3KbOYH9l3ad/yJnojEKtWivkRn2xiqXP6PcTey62rlGCkew30L
cl4gR55hasDAgN2PRXbs6jIdi4QH5XPj0MgQL1WqkylPCbJDMi5L4CRGMeNTP/zE
s5s8Bg8MT2xjMPtA5IHphyPidfkAU4Kltb/qTBkAkWmV2+PgGC5OFYkqFc+qTkUB
4YkSnVbnQ0Yxu86oYje5T4gw7oA62YWgBR+tI/sfxo4YZbR88EYmhw==
=SKC3
-----END PGP SIGNATURE-----

--WIyZ46R2i8wDzkSu--