From: Al Viro <viro@zeniv.linux.org.uk>
To: linux-fsdevel@vger.kernel.org
Cc: linux-unionfs@vger.kernel.org,
Amir Goldstein <amir73il@gmail.com>,
Miklos Szeredi <miklos@szeredi.hu>
Subject: [RFC][overlayfs] do we still need d_instantiate_anon() and export of d_alloc_anon()?
Date: Sat, 11 Nov 2023 08:04:00 +0000 [thread overview]
Message-ID: <20231111080400.GO1957730@ZenIV> (raw)
AFAICS, the main reason for exposing those used to be the need
to store ovl_entry in allocated dentry; we needed to do that before it
gets attached to inode, so the guts of d_obtain_alias() had to be
exposed.
These days overlayfs is stashing ovl_entry in the inode, so
we are left with this:
dentry = d_find_any_alias(inode);
if (dentry)
goto out_iput;
dentry = d_alloc_anon(inode->i_sb);
if (unlikely(!dentry))
goto nomem;
if (upper_alias)
ovl_dentry_set_upper_alias(dentry);
ovl_dentry_init_reval(dentry, upper, OVL_I_E(inode));
return d_instantiate_anon(dentry, inode);
ovl_dentry_init_reval() can bloody well be skipped, AFAICS - all it does
is potentially clearing DCACHE_OP_{,WEAK_}REVALIDATE. That's also done
in ovl_lookup(), and in case we have d_splice_alias() return a non-NULL
dentry we can simply copy it there. Sure, somebody might race with
us, pick dentry from hash and call ->d_revalidate() before we notice that
DCACHE_OP_REVALIDATE could be cleaned. So what? That call of ->d_revalidate()
will find nothing to do and return 1. Which is the effect of having
DCACHE_OP_REVALIDATE cleared, except for pointless method call. Anyone
who finds that dentry after the flag is cleared will skip the call.
IOW, that race is harmless.
And as for the ovl_dentry_set_upper_alias()... that information used to
live in ovl_entry until the need to trim the thing down. These days
it's in a bit in dentry->d_fsdata.
How painful would it be to switch to storing that in LSB of ovl_entry::__numlower,
turning ovl_numlower() into
return oe ? oe->__numlower>>1 : 0
and ovl_lowerdata() into
return lowerstack ? &lowerstack[(oe->__numlower>>1) - 1] : NULL
with obvious adjustment to ovl_alloc_entry().
An entry is coallocated with an array of struct ovl_path, with
numlower elements. More than 2G layers doesn't seem to be plausible -
there are fat 64bit boxen, but 32Gb (kmalloc'ed, at that) just in
the root ovl_entry alone feels somewhat over the top ;-)
So stealing that bit shouldn't be a problem. Is there anything I'm
missing?
next reply other threads:[~2023-11-11 8:04 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-11 8:04 Al Viro [this message]
2023-11-11 18:31 ` [RFC][overlayfs] do we still need d_instantiate_anon() and export of d_alloc_anon()? Amir Goldstein
2023-11-11 18:50 ` Al Viro
2023-11-11 20:05 ` Amir Goldstein
2023-11-12 7:26 ` Amir Goldstein
2023-11-18 20:02 ` Al Viro
2023-11-18 21:17 ` Al Viro
2023-11-19 6:57 ` Amir Goldstein
2023-11-19 7:26 ` Al Viro
2023-11-19 8:19 ` Amir Goldstein
2023-11-20 11:39 ` Amir Goldstein
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231111080400.GO1957730@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=amir73il@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).