On 2023/6/16 19:33, Alexander Larsson wrote:
On Fri, Jun 16, 2023 at 11:28 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
On Fri, Jun 16, 2023 at 11:39 AM Alexander Larsson <alexl@xxxxxxxxxx> wrote:
On Fri, Jun 16, 2023 at 10:12 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
I don't believe this format will actually need to change much.
However, I do agree
with the general requirement for *some* ability to move forward with
this format,
so I'm gonna go with a single version byte.
I disagree.
If you present a real life use case why that really matters
then I can reconsider.
I disagree, but I'll add them.
Let's ask for a 3rd opinion.
don't add them for now, unless Miklos says that you should.
I added them to the branch anyway for now. However, if we're going
full header + flags anyway, I wonder if we really need the
"overlay.digest" xattr at all? We could just put the header + optional
digest in the "overlay.metacopy" xattr, and then just read/store one
xattr. Right now metacopy is zero size, but adding some data to it
would not break ovl_check_metacopy_xattr() in older kernels.
Basically, during the lookup we get the metacopy xattr anyway, and
when we do we could record in a flag that there is a digest in it,
then during open we don't have to look for a separate digest xattr,
just re-load the metacopy xattr if the flag is set. With this in place
we can also easily add other flags to overlay.metacopy, which imho
makes a ton more sense than adding flags to overlay.digest.
My own slight concern about this is that:
Previously, all metacopy inodes shares a common EROFS shared xattr
which can be cached in memory once when the first read and other
inodes won't trigger any I/Os for this.
If "overlay.metacopy" xattr is not empty thus it cannot be shared,
I guess at least you could place it into an EROFS inline xattr, that
may be good as well if you also keep "header + flags".
But I'm not sure if it's a good idea to keep the full fsverity
digest in "overlay.metacopy" honestly, especially overlayfs
fsverity feature is off. That will amplify I/Os (which could be
landed in EROFS shared xattrs as "overlay.digest" in the past and
now we need to read it immediately.)
Especially for the "ls -lR" workload, I guess you might need to
evaluate this way (only add flags vs flags + digest in
"overlay.metacopy") first.
Thanks,
Gao Xiang
I'll have a look at this.
