svn commit: r366976 - head/sys/kern

[Jessica Clarke]

On 23 Oct 2020, at 16:56, Mateusz Guzik <mjg at FreeBSD.org> wrote:
> 
> Author: mjg
> Date: Fri Oct 23 15:56:22 2020
> New Revision: 366976
> URL: https://svnweb.freebsd.org/changeset/base/366976
> 
> Log:
>  cache: reduce memory waste in struct namecache
> 
>  The previous scheme for calculating the total size was doing sizeof
>  on the struct and then adding the wanted space for the buffer.
> 
>  nc_name is at offset 58 while sizeof(struct namecache) is 64.
>  With CACHE_PATH_CUTOFF of 39 bytes and 1 byte of padding we were
>  allocating 104 bytes for the entry and never accounting for the 6
>  byte padding, wasting that space.
> 
> Modified:
>  head/sys/kern/vfs_cache.c
> 
> Modified: head/sys/kern/vfs_cache.c
> ==============================================================================
> --- head/sys/kern/vfs_cache.c	Fri Oct 23 15:50:49 2020	(r366975)
> +++ head/sys/kern/vfs_cache.c	Fri Oct 23 15:56:22 2020	(r366976)
> @@ -162,6 +162,7 @@ struct	namecache_ts {
> 	struct	timespec nc_time;	/* timespec provided by fs */
> 	struct	timespec nc_dotdottime;	/* dotdot timespec provided by fs */
> 	int	nc_ticks;		/* ticks value when entry was added */
> +	int	nc_pad;
> 	struct namecache nc_nc;
> };
> 
> @@ -172,12 +173,19 @@ struct	namecache_ts {
>  * alignment for everyone. Note this is a nop for 64-bit platforms.
>  */
> #define CACHE_ZONE_ALIGNMENT	UMA_ALIGNOF(time_t)
> -#define	CACHE_PATH_CUTOFF	39
> 
> -#define CACHE_ZONE_SMALL_SIZE		(sizeof(struct namecache) + CACHE_PATH_CUTOFF + 1)
> -#define CACHE_ZONE_SMALL_TS_SIZE	(sizeof(struct namecache_ts) + CACHE_PATH_CUTOFF + 1)
> -#define CACHE_ZONE_LARGE_SIZE		(sizeof(struct namecache) + NAME_MAX + 1)
> -#define CACHE_ZONE_LARGE_TS_SIZE	(sizeof(struct namecache_ts) + NAME_MAX + 1)
> +#ifdef __LP64__
> +#define	CACHE_PATH_CUTOFF	45
> +#define	CACHE_LARGE_PAD		6
> +#else
> +#define	CACHE_PATH_CUTOFF	41
> +#define	CACHE_LARGE_PAD		2
> +#endif

Is there any explanation of where these magic constants come from?
There should at least be a comment IMO, of better yet have a C
expression to evaluate them without needing #ifdef-based hard-coding
(which then annoys things like CHERI that has 128-bit pointers in its
pure capability kernel that causes us to have to go and
reverse-engineer where these numbers came from so we can figure out
what the right value should be for us).

Jess