Re: [PATCH][RFC] %pd - for printing dentry name

[Linus Torvalds]

On Thu, 4 Feb 2010, Paul E. McKenney wrote:
> 
> Ah, good point on the hash size.  And given that DNAME_INLINE_LEN_MIN
> is 40 characters on 32-bit systems and 32 characters on 64-bit systems,
> I agree that while a four-character increase might be nice, it cannot be
> said to be an emergency.

Well, what we _could_ do is to make the 'length' field be part of the name 
itself, and just keep the hash separate. The hash (and parenthood) is what 
we check most in the hot inner loop, and don't want to have any extra 
indirection (or cache misses) for. The name length we check only later, 
after we've done all other checks (and after we've gotten the spinlock, 
which is the big thing).

So qstr->len is _not_ performance critical in the same way that qstr->hash 
is.

So we might not save 4 bytes, but we _could_ try to move the length field 
into the name with something like

	struct qstr_name {
		unsigned short len;
		char name[];
	};

	struct qstr {
		unsigned int hash;
		const struct qstr_name *name;
	};

but the problem then is one of alignment (ie that pointer generally wants 
8-byte alignment on 64-bit architectures, so none of this _helps_ unless 
we also move some other 4-byte field into the qstr (we could look at 
making 'd_time' be a 32-bit entry, for example, and move it in there).

Or we could do really crazy things, and put the four first characters of 
d_iname inside the qstr etc etc. It's just that it all gets totally 
unnatural very quickly.

So 'struct dentry' is one of the most important data structures we have 
(just because you easily end up with millions of them), but since we 
already control its size by varying the inline name length, I doubt that 
playing really fancy games is worth it.

			Linus
--
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/