On Thu, Jun 8, 2017 at 4:48 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > On Thu, Jun 8, 2017 at 3:11 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: >> On Wed, Jun 7, 2017 at 9:51 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >>> When inodes index feature is enabled, lookup in indexdir for the index >>> entry of lower real inode or copy up origin inode. The index entry name >>> is the hex representation of the lower inode file handle. >>> >>> If the index dentry in negative, then either no lower aliases have been >>> copied up yet, or aliases have been copied up in older kernels and are >>> not indexed. >>> >>> If the index dentry for a copy up origin inode is positive, but points >>> to an inode different than the upper inode, then either the upper inode >>> has been copied up and not indexed or it was indexed, but since then >>> index dir was cleared. Either way, that index cannot be used to indentify >>> the overlay inode. >>> >>> If a negative index dentry is found or a positive dentry that matches the >>> upper inode, store it in the overlay dentry to be used later. A non-upper >>> overlay dentry may still have a positive index from copy up of another >>> lower hardlink. This situation can be tested with the path type macros >>> OVL_TYPE_INDEX(type) && !OVL_TYPE_UPPER(type). >>> >>> This is going to be used to prevent breaking hardlinks on copy up. >> >> Why is a negative index (or any index, for that matter) stored in the >> overlay dentry? > > One of the reasons it is stored in the overlay dentry is to be able to > test OVL_TYPE_INDEX() on the dentry, for example, in: > "ovl: adjust overlay inode nlink for indexed inodes". OVL_TYPE_INDEX() is just a bool. When we do the *first* copy up of a file with multiple links we need to update all the aliases anyway: i.e. do the hardlink-ups, update ->upperdentry, etc. > OVL_TYPE_INDEX() is only interested in positive index, so it may seem > like a better idea to save memory and store the index dentry on lookup > only if it is positive, but then OVL_TYPE_INDEX() will be wrong for lower > aliases that became lower indexed aliases due to another alias copy up. > This is important for reasons that are mentioned below. > > There may be an opportunity for a massive optimization though. > For all practical purpose, ovl_type_indexed_lower() should always be > false for lower with nlink == 1, so there is no reason so store the negative > index dentry in that case. > >> This seems a big waste, since the index dentry will >> be allocated for all lower files, yet never used unless copied up. >> >> Index is used: >> >> - at lookup need to find any copied up alias >> - at copyup need to set up new index > > So it has several more subtle uses: > - When whiteout a lower aliases, we need to count down nlink to > know when we can unlink the an orphan index (TODO) If we do the link-up early (at lookup) then whiteout won't need special casing. The link-up would be unnecessary in this case, but delaying it will just cause headaches. > - Same when renaming over a lower indexed alias And the same case here. > - The lower hardlinks copy up on read [1] is another big user Again, doing it on lookup will be earlier than at read, so no issue. Thanks, Miklos
