There are very few places that legitimitely can do stores to
dentry->d_name. Basically, it's dentry creation and d_move() guts.
Unforutunately, there is a _lot_ of places where we access ->d_name -
every ->lookup(), ->create(), ->unlink(), etc. instances has to do
just that - that's how the directory entry name is passed to those.
Verifying that nobody is playing silly buggers with ->d_name
contents take, IME, several hours of code audit. And that has to
be repeated every cycle. Compiler is no help there - it's grep over
the tree, exclude the irrelevant hits (struct dirent has a field
with that name, so does e.g. UFS directory entry, etc.) and looking
through the 900-odd places that remain after that. Not fun, to
put it politely.
One way to do that would be to replace d_name with
union {
const struct qstr d_name;
struct qstr __d_name;
};
and let the the places that want to modify it use __d_name.
Tempting, but the thing that makes me rather nervous about this
approach is that such games with unions are straying into
the nasal demon country (C99 6.7.3[5]), inviting the optimizers
to play. Matt Wilcox pointed out that mainline already has
i_nlink/__i_nlink pair, but... there are fewer users of those
and the damage from miscompiles would be less sensitive.
Patch along those lines would be pretty simple, though.
There's an alternative way to get rid of that headache
that ought to be safer:
* add an inlined helper,
static inline const struct qstr *d_name(const struct dentry *d)
{
return &d->d_name;
}
* gradually convert the accesses to ->d_name to calls of that helper -
e.g.
ret = udf_fiiter_find_entry(dir, &dentry->d_name, &iter);
becoming
ret = udf_fiiter_find_entry(dir, d_name(dentry), &iter);
while
err = msdos_find(dir, dentry->d_name.name, dentry->d_name.len, &sinfo);
becomes either
err = msdos_find(dir, d_name(dentry)->name, d_name(dentry)->len, &sinfo);
or, if msdos_find() gets converted to passing const struct qstr *
instead of passing name and length as separate arguments,
err = msdos_find(dir, d_name(dentry), &sinfo);
I am *NOT* suggesting doing that step in a single conversion - that
would cause too many conflicts, wouldn't be easy to automate and there's
a considerable pile of places that would be better off with conversion
like above and those should be decided on case-by-case basis. However,
it's not hard to do as a patch series, with very few dependencies other
than "need to have that helper in place". And it's not hard to keep
track of unconverted instances. Some of the existing functions would
need struct qstr * argument constified - 4 of those in current mainline
(the same functions would need that treatment with anon union approach
as well; again, there are very few of them and it's not hard to do).
* once everything is converted (with the exception of several places in
fs/dcache.c and this d_name() inline itself), we can declared the use
of d_name() mandatory and rename the field to e.g. __d_name.
At that point audits become as easy as grep for new name of the field
and for casts to struct qstr * - compiler will take care of the rest.
That variant is longer, but it's not too large either. And it feels
safer wrt optimizer behaviour...
Preferences, comments?
