Subject: Re: changing VOP_REMOVE(), was Re: ufs-ism in lookup(9)
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Bill Studenmund <wrstuden@netbsd.org>
List: tech-kern
Date: 04/04/2004 19:25:24
--s/l3CgOIzMHHjg/5
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Apr 03, 2004 at 11:58:42PM +0900, YAMAMOTO Takashi wrote:
> hi,
>=20
> > > > For ufs, AFAICT we add two int32_t's to hold slot information. We a=
lso=20
> > > > use these values for file creation. So it doesn't seem like a waste=
 to me.
> > >=20
> > > it's a waste because the members are only used during dirops.
> > > you can implement an internal version of ufs_lookup, which stores slo=
t info
> > > into kernel stack or somewhere instead of the in-core inode, and
> > > call it from dirop VOPs.
> >=20
> > I really think you're adding a lot of complexity for two ints.
>=20
> i don't think so.  it's rather simpler and is a better abstraction.
>=20
> besides, it isn't merely a matter of space.
> it's one of reasons why we can't allow concurrent operations on a directo=
ry.

Ok. I'll agree that it explicitly prevents concurrent operations on a
directory. And we probably want to eventually move to some way to store=20
this info in the component info, so it's per-operation. But when we do=20
that, we will need to put work into making concurrent operations work=20
right. We will need to make sure simultaneous identical operations (two=20
identical creates or deletes or renames) only do the operation once.

I think the implementation details of that idea can spark a much larger=20
thread than this one has been. :-)

> > How much code are we talking about? Is it really worth making ourselves=
=20
> > incompatable with all the other BSDs for this?
>=20
> yes, i think.

If you can show a big win, I'll agree with making ourselves incompatable=20
with other BSD OSs. However you haven't made that case yet as far as I can=
=20
tell.

> > One other option is to keep the calling code pattern the same, and add =
a=20
> > NOLOAD flag. Its semantics are we do the lookup, and we?$B_Seturn the v=
node=20
> > if it is in core, but we don't load a vnode in if there isn't one now. =
My=20
> > idea is that since the parent directory will be locked the whole time, =
no=20
> > one can come in and create an entry behind our back. Thus this would be=
 a=20
> > case where it's valid to return NULL with no error from namei(). This=
=20
> > change would mean continuing to cache slot info, but I think that'll be=
=20
> > fine.
>=20
> i think it works.
> however, i don't think it's worth to keep the current code pattern hardly.

Since you aren't listing details (for how to change VOP_REMOVE()), I guess
we should wait for when you do.  I think this thread has indicated a lot=20
of things our current implementation gets right, and any change will have=
=20
to also get these things right.

Take care,

Bill

--s/l3CgOIzMHHjg/5
Content-Type: application/pgp-signature
Content-Disposition: inline

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

iD8DBQFAcMOTWz+3JHUci9cRAiPvAJ48eZHH+q89hpzLR/al0M8d7hlMWACdHHlI
PNPArnk/pf29XE4+55ytrRI=
=d6Ej
-----END PGP SIGNATURE-----

--s/l3CgOIzMHHjg/5--