On Mon, Dec 09, 2024 at 10:27:04AM -0800, Linus Torvalds wrote:
> The name consistency issue is really annoying. Do we really need it
> here? Because honestly, what you actually *really* care about here is
> whether it's inline or not, and you do that test right afterwards:
>
> > + // ->name and ->len are at least consistent with each other, so if
> > + // ->name points to dentry->d_iname, ->len is below DNAME_INLINE_LEN
> > + if (likely(name->name.name == dentry->d_iname)) {
> > + memcpy(name->inline_name, dentry->d_iname, name->name.len + 1);
>
> and here it would actually be more efficient to just use a
> constant-sized memcpy with DNAME_INLINE_LEN, and never care about
> 'len' at all.
Actually, taking a look at what's generated for that memcpy()... *ow*
amd64 is fine, but anything that doesn't like unaligned accesses is
ending up with really awful code.
gcc does not realize that pointers are word-aligned. What's more,
even
unsigned long v[5];
void f(unsigned long *w)
{
memcpu(v, w, sizeof(v));
}
is not enough to convince the damn thing - try it for e.g. alpha and you'll
see arseloads of extq/insq/mskq, all inlined.
And yes, they are aligned - d_iname follows a pointer, inline_name follows
struct qstr, i.e. u64 + pointer. How about we add struct inlined_name {
unsigned char name[DNAME_INLINE_LEN];}; and turn d_iname and inline_name
into anon unions with that? Hell, might even make it an array of unsigned
long and use that to deal with this
} else {
/*
* Both are internal.
*/
unsigned int i;
BUILD_BUG_ON(!IS_ALIGNED(DNAME_INLINE_LEN, sizeof(long)));
for (i = 0; i < DNAME_INLINE_LEN / sizeof(long); i++) {
swap(((long *) &dentry->d_iname)[i],
((long *) &target->d_iname)[i]);
}
}
in swap_names(). With struct assignment in the corresponding case in
copy_name() and in take_dentry_name_snapshot() - that does generate sane
code...
