On Mon, Nov 28, 2016 at 12:56 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > On Mon, Nov 28, 2016 at 12:35 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: >> On Mon, Nov 28, 2016 at 10:10 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: [...] >>> I may be way off here, but why do you need to lookup entry and get ino >>> from xattr at all? Wouldn't it be easier to not add the entry to the list if >>> it was copied up and rely on the fact that it will be added to list in iter >>> of lower layer with original ino with no extra effort? >> >> What about renamed entries? > > Right. I completely missed out on the rename case.. > >> What about opaque ones? > > That's exactly the point of OVL_XATTR_INO IFF !OVL_XATTR_OPAQUE > If you have OVL_XATTR_INO means entry cannot be opaque, so it should > be safe to skip it So that means if OVL_XATTR_OPAQUE is set then we don't need to check OVL_XATTR_INO and if OVL_XATTR_INO is set we don't need to check OVL_XATTR_OPAQUE. But that doesn't help optimize readdir, because we really want to check neither. >> I do hope we can optimize directory reading, because doing lookup and >> getxattr for all entries is going to hurt... >> > > Possibly silly question: > Do you know if programs really rely of d_ino from getdents to be sane/non-zero? > And what are the implications of overlayfs readdir not exporting the real d_ino? I don't know of any precedent, so it's a big unknown. >> We do keep ino stable across rename. We don't keep ino stable across >> copy-up. That's what this patch is trying to address. >> >> You are saying that we should have redirects for non-dir and drop >> OVL_XATTR_INO? That's another option, but it doesn't look like it >> would simplify things... There *is* actually a case where both 'opaque' and 'ino' make sense: when an empty merged dir is exchanged for an empty opaque one in ovl_clear_empty(). > Well, not sure if you noticed my redirect_fh (rediect by file handle) work. > If differs from redirect by path in 2 major ways: > 1. Like OVL_XATTR_INO, redirect is set on copy up (but only for dirs) > 2. Lookup is much simpler (and most likely faster) then full path lookup > > It would be trivial to set oe->ino of merged dir from lower most entry in > ovl_lookup(). Okay. In fact always using the handle looks like a better option overall. File handle should be unique for the lifetime of the filesystem, while inode numbers may be reused. Biggest drawback of the file handle based redirects is exactly that: makes backing up the overlay basically impossible, since file handles won't work after a backup + restore. But as an optimization, in addition to path based redirects it would work fine and provide a good way to get the stable ino. Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
