Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 23 Oct 2020 17:11:47 +0100
From:      Jessica Clarke <jrtc27@freebsd.org>
To:        Mateusz Guzik <mjg@FreeBSD.org>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head@freebsd.org
Subject:   Re: svn commit: r366976 - head/sys/kern
Message-ID:  <A6C118FC-981E-4702-B22F-17BCDCA597CE@freebsd.org>
In-Reply-To: <202010231556.09NFuMFb079616@repo.freebsd.org>
References:  <202010231556.09NFuMFb079616@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 23 Oct 2020, at 16:56, Mateusz Guzik <mjg@FreeBSD.org> wrote:
>=20
> Author: mjg
> Date: Fri Oct 23 15:56:22 2020
> New Revision: 366976
> URL: https://svnweb.freebsd.org/changeset/base/366976
>=20
> Log:
>  cache: reduce memory waste in struct namecache
>=20
>  The previous scheme for calculating the total size was doing sizeof
>  on the struct and then adding the wanted space for the buffer.
>=20
>  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.
>=20
> Modified:
>  head/sys/kern/vfs_cache.c
>=20
> Modified: head/sys/kern/vfs_cache.c
> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
> --- 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;
> };
>=20
> @@ -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
>=20
> -#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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?A6C118FC-981E-4702-B22F-17BCDCA597CE>