--M9NhX3UHpAaciwkO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Aug 03, 2005 at 10:06:53PM +0200, Julio M. Merino Vidal wrote:
> Hi all (and mentors),
>=20
> The existing tmpfs code allocates new vnodes for each
> access within the filesystem; e.g., the lookup operation gets a
> new vnode for each match and returns it.  This hasn't caused
> problems until now (despite it's plain wrong).

Yeah, that's wrong.

> During the past days, I've been implementing the rmdir vnode
> operation.  As this removes information from the file-system,
> problems have started to pop up.  Basically, multiple vnodes
> were pointing to the same tmpfs_node structure; when the
> structure was freed, those vnodes were left with dangling
> pointers in them, causing memory faults in future accesses.
> So I have to change the code to use a single vnode for a
> concrete file.

Indeed.

> So far, I've got a fix that avoids crashes, but I doubt it's correct
> (comparing it to other file-systems' code).  And even if it is,
> I have several doubts that the manual pages (vnode(9) and
> vnodeops(9)) do not solve.
>=20
> First of all, I've changed the file-system specific node structure
> (tmpfs_node) to keep a pointer to a vnode (let's call this
> tn_vnode).  When creating the node, tn_vnode is initialized to
> NULL, because there is no vnode associated to it yet.
>=20
> Later on, when tmpfs needs to get a vnode for a specific
> tmpfs_node, it checks if tn_vnode is NULL.  If it is, it gets a new
> vnode (using getnewvnode), attaches it to the tmpfs_node and
> uses it.  If it's not, it calls vget on tn_vnode and uses that.

Sounds right.

> First question: several file-systems seem to do the same
> (lookup for a existing vnode and return it), but they do _not_
> have a call to vget (or at least I can't see it).  However, if I do
> not add this call, I get a panic (v_usecount =3D=3D -1).  Why?
> Furthermore, I'm afraid about using vget here because... who
> warrants that the vnode is in the free list?

You can vget() a vnode that is not on the free list. Note that the tail=20
removals only happen if v_usecount =3D=3D 0.

As for where file systems are doing it, for instance look at ufs_lookup.c=
=20
Look at the calls to VFS_VGET(). They eventually end up calling vget().

> Then, I have problems with the rmdir operation itself.  I first
> detach a_vp from its tmpfs_node by setting v_data and
> tn_node both to NULL.  Then I call vput over a_vp, as other
> file-systems do.  After this, I free the associated tmpfs_node
> given that no-one else is pointing to it.
>=20
> *But* a_vp is not really "released".  When I unmount the
> file-system, the kernel calls the fsync and reclaim operations
> over the value a_vp had (the vnode used during rmdir).  As
> this vnode has v_data set to NULL, any attempt to access the
> structure obviously fails and panics the system.

Do you either vrele() or vput() the a_vp vnode? That's how the release is=
=20
supposed to happen.

You probably should limit teardown of the vnode to VOP_RECLAIM(). It is=20
the "this is no longer your vnode" call.

> I can workaround this by checking whether v_data is NULL or
> not, but I don't see any other file-system doing this.  What
> should I do to solve this correctly?  Should I tell the system
> to completely forget about that vnode in some way?

Take care,

Bill

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

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

iD8DBQFC8nX0Wz+3JHUci9cRAtQ2AJ9LZ3vIWsPPG10TFfDaVnTRFR045gCfa7tV
kpZeeUnl06Gcn4nj9i6nD4A=
=htt0
-----END PGP SIGNATURE-----

--M9NhX3UHpAaciwkO--