From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <1463935173.2257.6.camel@HansenPartnership.com> References: <20160517215326.GA1589@veci.piliscsaba.szeredi.hu> <1463935173.2257.6.camel@HansenPartnership.com> Date: Fri, 27 May 2016 10:14:34 +0200 Message-ID: Subject: Re: [PATCH 1/3] vfs: merge .d_select_inode() into .d_real() From: Miklos Szeredi To: James Bottomley Cc: Al Viro , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, David Howells Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: On Sun, May 22, 2016 at 6:39 PM, James Bottomley wrote: > On Tue, 2016-05-17 at 23:53 +0200, Miklos Szeredi wrote: >> The two methods essentially do the same: find the real dentry/inode >> belonging to an overlay dentry. The difference is in the usage: >> >> vfs_open() uses ->d_select_inode() and expects the function to >> perform copy-up if necessary based on the open flags argument. >> >> file_dentry() uses ->d_real() passing in the overlay dentry as well >> as the underlying inode. >> >> vfs_rename() uses ->d_select_inode() but passes zero flags. >> ->d_real() with a zero inode would have worked just as well here. >> >> This patch merges the functionality of ->d_select_inode() into >> ->d_real() by adding an 'open_flags' argument to the latter. > > Actually, before you do this, I have a use case for keeping them > separate: The shiftfs prototype I've posted cannot use d_select_inode > because it's shifting the uid/gids, so the inode permissions have to be > done on the upper inode. However, several filesystems make use of > file_dentry() and for them, I need to supply the lower dentry, so I end > up setting d_real but not d_select_inode. > > for me, then vfs_open and vfs_rename have to operate on the upper inode > but all uses of file_dentry should return the lower dentry. The calls in vfs_rename() deal with rename to hard-linked counterpart. Unless you want to add lower to upper mapping for inodes, you'll need this too. Same goes for hard linked sockets: https://git.kernel.org/cgit/linux/kernel/git/mszeredi/vfs.git/commit/?h=overlayfs-next&id=eb0a4a47ae89aaa0674ab3180de6a162f3be2ddf So looks like the only thing you don't want this is for vfs_open(). We pass open flags into the merged ->d_real() function, but that may be zero. Could pass an internal O_OPEN flag to indicate this is open. Thanks, Miklos