On Fri, Feb 09, 2024 at 11:10:10AM -0800, Nick Desaulniers wrote: > I have 100% observed llvm throw out writes to objects declared as > const where folks tried to write via "casting away the const" (since > that's UB) which resulted in boot failures in the Linux kernel > affecting android devices. I can't find the commit in question at the > moment, but seemed to have made some kind of note in it in 2020. > https://android-review.git.corp.google.com/c/platform/prebuilts/clang/host/linux-x86/+/1201901/1/RELEASE_NOTES.md > > That said, I just tried Al's union, and don't observe such optimizations. > https://godbolt.org/z/zrj71E8W5 FWIW, see #work.qstr in git://git.kernel.org:/pub/scm/linux/kernel/git/viro/vfs.git First 4 commits are constifying the arguments that get &dentry->d_inode passed into, followed by the patch below. __d_alloc(), swap_names(), copy_name() and d_mark_tmpfile() - no other writers... diff --git a/fs/dcache.c b/fs/dcache.c index b813528fb147..d21df0fb967d 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1651,13 +1651,13 @@ static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name) dname = dentry->d_iname; } - dentry->d_name.len = name->len; - dentry->d_name.hash = name->hash; + dentry->__d_name.len = name->len; + dentry->__d_name.hash = name->hash; memcpy(dname, name->name, name->len); dname[name->len] = 0; /* Make sure we always see the terminating NUL character */ - smp_store_release(&dentry->d_name.name, dname); /* ^^^ */ + smp_store_release(&dentry->__d_name.name, dname); /* ^^^ */ dentry->d_lockref.count = 1; dentry->d_flags = 0; @@ -2695,16 +2695,16 @@ static void swap_names(struct dentry *dentry, struct dentry *target) /* * Both external: swap the pointers */ - swap(target->d_name.name, dentry->d_name.name); + swap(target->__d_name.name, dentry->__d_name.name); } else { /* * dentry:internal, target:external. Steal target's * storage and make target internal. */ - memcpy(target->d_iname, dentry->d_name.name, - dentry->d_name.len + 1); - dentry->d_name.name = target->d_name.name; - target->d_name.name = target->d_iname; + memcpy(target->d_iname, dentry->__d_name.name, + dentry->__d_name.len + 1); + dentry->__d_name.name = target->__d_name.name; + target->__d_name.name = target->d_iname; } } else { if (unlikely(dname_external(dentry))) { @@ -2712,10 +2712,10 @@ static void swap_names(struct dentry *dentry, struct dentry *target) * dentry:external, target:internal. Give dentry's * storage to target and make dentry internal */ - memcpy(dentry->d_iname, target->d_name.name, - target->d_name.len + 1); - target->d_name.name = dentry->d_name.name; - dentry->d_name.name = dentry->d_iname; + memcpy(dentry->d_iname, target->__d_name.name, + target->__d_name.len + 1); + target->__d_name.name = dentry->__d_name.name; + dentry->__d_name.name = dentry->d_iname; } else { /* * Both are internal. @@ -2728,7 +2728,7 @@ static void swap_names(struct dentry *dentry, struct dentry *target) } } } - swap(dentry->d_name.hash_len, target->d_name.hash_len); + swap(dentry->__d_name.hash_len, target->__d_name.hash_len); } static void copy_name(struct dentry *dentry, struct dentry *target) @@ -2738,12 +2738,12 @@ static void copy_name(struct dentry *dentry, struct dentry *target) old_name = external_name(dentry); if (unlikely(dname_external(target))) { atomic_inc(&external_name(target)->u.count); - dentry->d_name = target->d_name; + dentry->__d_name = target->__d_name; } else { - memcpy(dentry->d_iname, target->d_name.name, - target->d_name.len + 1); - dentry->d_name.name = dentry->d_iname; - dentry->d_name.hash_len = target->d_name.hash_len; + memcpy(dentry->d_iname, target->__d_name.name, + target->__d_name.len + 1); + dentry->__d_name.name = dentry->d_iname; + dentry->__d_name.hash_len = target->__d_name.hash_len; } if (old_name && likely(atomic_dec_and_test(&old_name->u.count))) kfree_rcu(old_name, u.head); @@ -3080,7 +3080,7 @@ void d_mark_tmpfile(struct file *file, struct inode *inode) !d_unlinked(dentry)); spin_lock(&dentry->d_parent->d_lock); spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED); - dentry->d_name.len = sprintf(dentry->d_iname, "#%llu", + dentry->__d_name.len = sprintf(dentry->d_iname, "#%llu", (unsigned long long)inode->i_ino); spin_unlock(&dentry->d_lock); spin_unlock(&dentry->d_parent->d_lock); diff --git a/include/linux/dcache.h b/include/linux/dcache.h index eb4ca8a1d948..a74370b1fe31 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -85,7 +85,10 @@ struct dentry { seqcount_spinlock_t d_seq; /* per dentry seqlock */ struct hlist_bl_node d_hash; /* lookup hash list */ struct dentry *d_parent; /* parent directory */ - struct qstr d_name; + union { + struct qstr __d_name; + const struct qstr d_name; + }; struct inode *d_inode; /* Where the name belongs to - NULL is * negative */ unsigned char d_iname[DNAME_INLINE_LEN]; /* small names */
