From mboxrd@z Thu Jan 1 00:00:00 1970 From: Miklos Szeredi Subject: Re: [PATCH v2 05/11] ovl: lookup redirect by file handle Date: Wed, 26 Apr 2017 14:15:11 +0200 Message-ID: References: <1493025256-27188-1-git-send-email-amir73il@gmail.com> <1493025256-27188-6-git-send-email-amir73il@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org To: Amir Goldstein Cc: Vivek Goyal , Al Viro , "linux-unionfs@vger.kernel.org" , linux-fsdevel List-Id: linux-unionfs@vger.kernel.org On Wed, Apr 26, 2017 at 12:17 PM, Amir Goldstein wrote: > On Wed, Apr 26, 2017 at 12:55 PM, Miklos Szeredi wrote: >> On Wed, Apr 26, 2017 at 11:40 AM, Amir Goldstein wrote: >> >>> Just to see that I understand you correctly. >>> >>> I am now working on storing the following: >>> >>> /* >>> * The tuple origin.{fh,layer,uuid} is a universal unique identifier >>> * for a copy up origin, where: >>> * origin.fh - exported file handle of the lower file >>> * origin.root - exported file handle of the lower layer root >>> * origin.uuid - uuid of the lower filesystem >> >> I wouldn't even store origin.root. >> >>> * >>> * origin.{fh,root} are stored in format of a variable length binary blob >>> * with struct ovl_fh header (total blob size up to 20 bytes). >>> * uuid is stored in raw format (16 bytes) as published by sb->s_uuid. >>> */ >>> >>> I intend to implement lookup as follows: >>> - compare(origin.uuid, same_lower_sb->s_uuid) >>> # layer root dentries cannot be DCACHE_DISCONNECTED, so >>> # exportfs_decode_fh ignores mnt arg and returns the cached dentry >>> - root = exportfs_decode_fh(lowerstack[0].mnt, origin.root) >>> - find layer where lowerstack[layer].dentry == root >>> - this = exportfs_decode_fh(lowerstack[layer].mnt, origin.fh) >>> >>> is_subdir() is NOT needed for decoding the layer root >>> is_subdir() is optional for decoding the lower file, because >>> it is not needed to identify the layer >> >> Hmm, we can just force exportfs_decode_fh() to return a connected >> dentry (return false from *acceptable() if the dentry is disconnected) >> before going on to iterate the layers to see which one contains it. >> > > Hmm, this might work, but to quote from exportfs_decode_fh(): > "It's not a directory. Life is a little more complicated." > > IIUC, 'connected' means 'connected to sb root', and not > 'connected to mnt root', so in the optimal case where > all lower dentries are cached, exportfs_decode_fh() will return > a connected dentry for every fh we give it regardless of the > mnt argument, so we will have to use is_subdir() to find the > right layer, which brings us back to O(numlower*depth) It just means that we might have to make up an artificial mount which has its root at the sb root to be able to decode the handle into a connected one. > > With the extra cost of storing the deducible information origin.root, > we will have less complex and more efficient lookup code. > > Let me try and implement it and see if I am right. > We can always discard origin.root from v4 if it turns > out to be unhelpful. I don't have good feelings about storing the root fh just because we don't special case the layer root anywhere yet, and I wouldn't want to do that unless there's a good reason. Thanks, Miklos