On Thu, Jan 16, 2025 at 10:38:53AM -0500, Gabriel Krisman Bertazi wrote:
> > * If the dentry name is stored in-line, then it may be concurrently
> > * modified by a rename. If this happens, the VFS will eventually retry
> > * the lookup, so it doesn't matter what ->d_compare() returns.
> > * However, it's unsafe to call utf8_strncasecmp() with an unstable
> > * string. Therefore, we have to copy the name into a temporary buffer.
>
> This part of the comment needs updating since there is no more copying.
>
> > + * As above, len is guaranteed to match str, so the shortname case
> > + * is exactly when str points to ->d_shortname.
> > */
> > - if (len <= DNAME_INLINE_LEN - 1) {
> > - memcpy(strbuf, str, len);
> > - strbuf[len] = 0;
> > - str = strbuf;
> > + if (qstr.name == dentry->d_shortname.string) {
> > + strbuf = dentry->d_shortname; // NUL is guaranteed to be in there
> > + qstr.name = strbuf.string;
> > /* prevent compiler from optimizing out the temporary buffer */
> > barrier();
>
> If I read the code correctly, I admit I don't understand how this
> guarantees the stability. Aren't you just assigning qstr.name back the
> same value it had in case of an inlined name through a bounce pointer?
> The previous implementation made sense to me, since the memcpy only
> accessed each character once, and we guaranteed the terminating
> character explicitly, but I'm having a hard time with this version.
This
strbuf = dentry->d_shortname; // NUL is guaranteed to be in there
copies the entire array. No bounce pointers of any sort; we copy
the array contents, all 40 bytes of it. And yes, struct (or union,
in this case) assignment generates better code than manual memcpy()
here.
