From owner-freebsd-current  Fri May  2 12:16:07 1997
Return-Path: <owner-current>
Received: (from root@localhost)
          by hub.freebsd.org (8.8.5/8.8.5) id MAA17286
          for current-outgoing; Fri, 2 May 1997 12:16:07 -0700 (PDT)
Received: from phaeton.artisoft.com (phaeton.Artisoft.COM [198.17.250.50])
          by hub.freebsd.org (8.8.5/8.8.5) with SMTP id MAA17185
          for <current@FreeBSD.ORG>; Fri, 2 May 1997 12:14:32 -0700 (PDT)
Received: (from terry@localhost) by phaeton.artisoft.com (8.6.11/8.6.9) id MAA09250; Fri, 2 May 1997 12:12:08 -0700
From: Terry Lambert <terry@lambert.org>
Message-Id: <199705021912.MAA09250@phaeton.artisoft.com>
Subject: Re: vnode->v_usage
To: phk@dk.tfs.com (Poul-Henning Kamp)
Date: Fri, 2 May 1997 12:12:08 -0700 (MST)
Cc: dufault@hda.com, phk@dk.tfs.com, current@FreeBSD.ORG
In-Reply-To: <2055.862583803@critter> from "Poul-Henning Kamp" at May 2, 97 04:36:43 pm
X-Mailer: ELM [version 2.4 PL24]
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: owner-current@FreeBSD.ORG
X-Loop: FreeBSD.org
Precedence: bulk

> Test results and comments most welcome!

[ ... ]

>  	struct	simplelock v_interlock;		/* lock on usecount and flag */
>  	struct	lock *v_vnlock;			/* used for non-locking fs's */
>  	enum	vtagtype v_tag;			/* type of underlying data */
>  	void 	*v_data;			/* private data for fs */
> +	LIST_HEAD(, namecache) v_cache_src;	/* Cache entries from us */
> +	TAILQ_HEAD(, namecache) v_cache_dst;	/* Cache entries to us */
> +	struct	vnode *v_dd;			/* .. vnode */
> +	u_long	v_ddid;				/* .. capability identifier */
>  };

Could this be changed to something like:

 	struct	simplelock v_interlock;		/* lock on usecount and flag */
 	struct	lock *v_vnlock;			/* used for non-locking fs's */
	struct _cachedata {
		LIST_HEAD(, namecache) v_cache_src;	/* entries from us */
		TAILQ_HEAD(, namecache) v_cache_dst;	/* entries to us */
		struct	vnode *v_dd;			/* .. vnode */
		u_long	v_ddid;				/* .. capability */
	} v_namecache;
 	enum	vtagtype v_tag;			/* type of underlying data */
 	void 	*v_data;			/* private data for fs */
};

instead?  It should be before the enum and the v_data because the enum
should be murdered, and the v_data should be last.

I would vastly prefer it if this structure were accessed by pointer.

	
Also, I'm not comfortable with this:

> - * For simplicity (and economy of storage), names longer than
> - * a maximum length of NCHNAMLEN are not cached; they occur
> - * infrequently in any case, and are almost never of interest.
> - *

The limit on path component name caching was intentional, and I thought,
well reasoned.  This will tend to artifically inflate the cache effects,
and will make it hard to do an apples/apples comparison of the code on
random data.  The limit should remain for side-by-side testing.


Finally, though I dislike the idea of hanging the cache entry off the
directory vnode to increase sparsity, what about:

1)	Hang the cache off the directory vnode (via pointer).  I don't
	see a lot of use for the _cachedata fields on a non-directory
	inode.

2)	Since the directory vnode is available to do the lookup, it's
	likely the pages in the directory are in core or can be easily
	faulted, if necessary.  So instead of copying the names, point
	to the name data in the directory entry itself.  This seems to
	be a better method of removing the length limit in any case,
	since it drastically reduces the storage requirements.

This would also reduce the cache entry space requirements, and make them
(once again) static sized.


					Regards,
					Terry Lambert
					terry@lambert.org
---
Any opinions in this posting are my own and not those of my present
or previous employers.