lkml.org 
[lkml]   [2006]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [RFC] Alignment of fields in struct dentry
    On Thu, 14 September 2006 15:55:45 -0600, Andreas Dilger wrote:
    > On Sep 14, 2006 23:02 +0200, J???rn Engel wrote:
    > >
    > > Using your scheme (slightly reduced) we now have:
    > > size32 size64 funky?
    > > d_count 4 4
    > > d_flags 4 4
    > > d_lock 4 4_ y
    > > d_inode 4_ 8
    > > d_hash 8 16--
    > > d_parent 4 8_
    > > d_name 12-- 16___
    > > d_lru 8_ 16_
    > > d_rcu/d_child 8 16__
    > > d_subdirs 8___ 16_
    > > d_alias 8 16____
    > > d_time 4 8
    > > d_op 4_ 8_
    > > d_sb 4 8
    > > d_fsdata 4 8__
    > > d_cookie 0 0 y
    > > d_mounted 4 4
    > > d_iname 36____ 36
    > >
    > > With the two funky fields possibly growing, depending on kernel
    > > config. [_-] mark 16-, 32- 64- and 128-byte boundaries, depending on
    > > len. What really frightens me is that a 32-byte boundary goes right
    > > through d_name on 32bit machines.
    >
    > Actually, splitting d_name like this is not so bad (as long as the
    > compiler doesn't add padding) because the important fields (hash
    > and len) are first and are compared for all non-matching dentries
    > in __d_lookup().

    Except that d_name is split between hash and len, not between len and
    name. You said d_lock was added later? Looks like it broke careful
    tuning for 32-byte cacheline machines. And it could also have caused
    the misalignment on 64bit.

    > > Now d_lookup() should use a single cacheline, even on my aged
    > > notebook, and the other hot fields remain at the top. d_mounted is
    > > also moved up to remove the misalignment on 64bit. Might be worth
    > > a benchmark or two to see whether it makes a difference...
    >
    > Might not be too hard (even if it temporarily kills performance)
    > to add atomic counters for each of these fields where they are
    > referenced in dcache.c, namei.c and e.g. fs/ext3/*.c (which is
    > only using d_inode, d_name, and d_parent). Run a find and a
    > kernel compile and dump the counters at shutdown.

    Might be even simpler by running gcov/lcov. That would not show which
    fields are used at similar times, but it is a start.

    Btw, I already mentioned how reducing the d_iname fields by a few
    bytes could save a cacheline. And whenever I have a good idea, Arnd
    usually comes up with a better one. How about the patch below to fix
    dentries to 128/192 bytes on 32/64 bit machines, independently of
    config options? It would shrink them from 132 bytes to 128 bytes in
    my current config (although I don't quite remember why I turned
    CONFIG_PROFILING on).

    Jörn

    --
    Joern's library part 14:
    http://www.sandpile.org/

    --- slab/include/linux/dcache.h~dentry_size 2006-09-14 22:19:51.000000000 +0200
    +++ slab/include/linux/dcache.h 2006-09-15 12:25:27.000000000 +0200
    @@ -77,7 +77,7 @@ full_name_hash(const unsigned char *name

    struct dcookie_struct;

    -#define DNAME_INLINE_LEN_MIN 36
    +#define DNAME_INLINE_LEN_MIN 16

    struct dentry {
    atomic_t d_count;
    @@ -112,7 +112,10 @@ struct dentry {
    #endif
    int d_mounted;
    unsigned char d_iname[DNAME_INLINE_LEN_MIN]; /* small names */
    -};
    +}__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. */

    /*
    * dentry->d_lock spinlock nesting subclasses:
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/
    \
     
     \ /
      Last update: 2006-09-15 12:31    [W:0.024 / U:30.240 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site