Re: [RFC] Alignment of fields in struct dentry

From: Jörn Engel
Date: Mon Sep 18 2006 - 17:24:38 EST


On Fri, 15 September 2006 22:44:07 +0200, Arnd Bergmann wrote:
> Am Friday 15 September 2006 12:27 schrieb Jörn Engel:
> > -};
> > +}__attribute__((aligned(64))); /* make sure the dentry is 128/192 bytes
> > +                                  on 32/64 bit independently of config
> > +                                  options.  d_iname will vary in length
> > +                                  a bit. */

Wow. Mutt really didn't like quoting this.

> I'd guess that a 32 byte alignment is much better here, 64 byte sounds
> excessive. It should have the same effect with the current dentry layout
> and default config options, but would keep the d_iname length in the
> 16-44 byte range instead of 16-76 byte as your patch does.
>
> Since all important fields are supposed to be kept in 32 bytes anyway,
> they are still either at the start or the end of a given cache line,
> but never cross two.

Another take would be to use a cacheline. But I guess the difference
between 32/64/cacheline is mostly academic, given the rate of changes
to struct dentry.

Unless the minimum length for inline names is configurable, as in the
patch below. (Note to self: I really should finish my test setup
before posting any further patches without performance numbers.)

Jörn

--
Don't worry about people stealing your ideas. If your ideas are any good,
you'll have to ram them down people's throats.
-- Howard Aiken quoted by Ken Iverson quoted by Jim Horning quoted by
Raph Levien, 1979

--- slab/include/linux/dcache.h~dentry_size 2006-09-14 22:19:51.000000000 +0200
+++ slab/include/linux/dcache.h 2006-09-18 23:22:44.000000000 +0200
@@ -77,8 +77,6 @@ full_name_hash(const unsigned char *name

struct dcookie_struct;

-#define DNAME_INLINE_LEN_MIN 36
-
struct dentry {
atomic_t d_count;
unsigned int d_flags; /* protected by d_lock */
@@ -111,8 +109,8 @@ struct dentry {
struct dcookie_struct *d_cookie; /* cookie, if any */
#endif
int d_mounted;
- unsigned char d_iname[DNAME_INLINE_LEN_MIN]; /* small names */
-};
+ unsigned char d_iname[CONFIG_DCACHE_INLINE_LEN]; /* small names */
+}__cacheline_aligned;

/*
* dentry->d_lock spinlock nesting subclasses:
--- slab/fs/Kconfig~dentry_size 2006-08-11 13:03:43.000000000 +0200
+++ slab/fs/Kconfig 2006-09-18 23:22:44.000000000 +0200
@@ -1931,5 +1931,18 @@ endmenu

source "fs/nls/Kconfig"

+config DCACHE_INLINE_LEN
+ int "Length of inline filenames"
+ depends on EXPERIMENTAL
+ default 20
+ help
+ Filenames up to this length are stored inline in a struct dentry.
+ For most people something short like 20 is ok. Developers with
+ extensive use of git or ccache may want to set this to 38 or 44,
+ respectively, as those programs create and often access thousands
+ of files of that length.
+
+ If unsure, choose 20.
+
endmenu

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/