diff mbox

[3/3] VFS: call d_inode() from d_backing_inode()

Message ID 20180620193910.6804-4-rgoldwyn@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Goldwyn Rodrigues June 20, 2018, 7:39 p.m. UTC 
From: Goldwyn Rodrigues <rgoldwyn@suse.com>

d_backing_inode and d_inode perform the same task: return
dentry->d_inode

Introduced in df1a085af1f6 ("VFS: Add a fallthrough flag for marking
virtual dentries") These functions are being used by many but it
does not serve the purpose it was originally meant for. So,
changed the comments which are not relevant anymore and removed
d_backing_dentry() which is not used.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 include/linux/dcache.h | 37 ++++---------------------------------
 1 file changed, 4 insertions(+), 33 deletions(-)

Comments

Christoph Hellwig June 21, 2018, 12:22 p.m. UTC | #1 
On Wed, Jun 20, 2018 at 02:39:10PM -0500, Goldwyn Rodrigues wrote:
> -static inline struct dentry *d_backing_dentry(struct dentry *upper)
> +static inline struct inode *d_backing_inode(const struct dentry *dentry)
>  {
> -	return upper;
> +	return d_inode(dentry);
>  }

Why even keep both functions around then?
Goldwyn Rodrigues June 21, 2018, 12:58 p.m. UTC | #2 
On 06-21 05:22, Christoph Hellwig wrote:
> On Wed, Jun 20, 2018 at 02:39:10PM -0500, Goldwyn Rodrigues wrote:
> > -static inline struct dentry *d_backing_dentry(struct dentry *upper)
> > +static inline struct inode *d_backing_inode(const struct dentry *dentry)
> >  {
> > -	return upper;
> > +	return d_inode(dentry);
> >  }
> 
> Why even keep both functions around then?
> 

Yes, I would love to get rid of them. There are just too many users.
1332 users of d_inode
176 users of d_backing_inode

Similarly, file_inode() with 1020 users is another candidate.

Introduced in -
496ad9aa8ef4 ("new helper: file_inode(file)")
but made unnecessary in -
dd37978c50bc ("cache the value of file_inode() in struct file")

I will work on a patch to clean all.
Christoph Hellwig June 21, 2018, 1:01 p.m. UTC | #3 
On Thu, Jun 21, 2018 at 07:58:16AM -0500, Goldwyn Rodrigues wrote:
> Yes, I would love to get rid of them. There are just too many users.
> 1332 users of d_inode
> 176 users of d_backing_inode

Sounds like the latter is an easy candidate for a scriped removal.
Amir Goldstein June 21, 2018, 1:40 p.m. UTC | #4 
On Thu, Jun 21, 2018 at 3:58 PM, Goldwyn Rodrigues <rgoldwyn@suse.de> wrote:
> On 06-21 05:22, Christoph Hellwig wrote:
>> On Wed, Jun 20, 2018 at 02:39:10PM -0500, Goldwyn Rodrigues wrote:
>> > -static inline struct dentry *d_backing_dentry(struct dentry *upper)
>> > +static inline struct inode *d_backing_inode(const struct dentry *dentry)
>> >  {
>> > -   return upper;
>> > +   return d_inode(dentry);
>> >  }
>>
>> Why even keep both functions around then?
>>
>
> Yes, I would love to get rid of them. There are just too many users.
> 1332 users of d_inode
> 176 users of d_backing_inode
>

If you had an idea to get rid of d_inode() please resist the temptation.
s/d_backing_inode/d_inode is no harm.

> Similarly, file_inode() with 1020 users is another candidate.
>

Similarly, please resist the temptation to remove this harmless
wrapper, especially, not before this discussion ends with
an understanding about the required VFS abstractions:
https://marc.info/?l=linux-fsdevel&m=152904165622462&w=2

Thanks,
Amir.
diff mbox

Patch

diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 6bb1ba14af8d..5abb0866dca5 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -491,9 +491,6 @@  static inline unsigned long vfs_pressure_ratio(unsigned long val)
 /**
  * d_inode - Get the actual inode of this dentry
  * @dentry: The dentry to query
- *
- * This is the helper normal filesystems should use to get at their own inodes
- * in their own dentries and ignore the layering superimposed upon them.
  */
 static inline struct inode *d_inode(const struct dentry *dentry)
 {
@@ -503,9 +500,6 @@  static inline struct inode *d_inode(const struct dentry *dentry)
 /**
  * d_inode_rcu - Get the actual inode of this dentry with READ_ONCE()
  * @dentry: The dentry to query
- *
- * This is the helper normal filesystems should use to get at their own inodes
- * in their own dentries and ignore the layering superimposed upon them.
  */
 static inline struct inode *d_inode_rcu(const struct dentry *dentry)
 {
@@ -513,35 +507,12 @@  static inline struct inode *d_inode_rcu(const struct dentry *dentry)
 }
 
 /**
- * d_backing_inode - Get upper or lower inode we should be using
- * @upper: The upper layer
- *
- * This is the helper that should be used to get at the inode that will be used
- * if this dentry were to be opened as a file.  The inode may be on the upper
- * dentry or it may be on a lower dentry pinned by the upper.
- *
- * Normal filesystems should not use this to access their own inodes.
- */
-static inline struct inode *d_backing_inode(const struct dentry *upper)
-{
-	struct inode *inode = upper->d_inode;
-
-	return inode;
-}
-
-/**
- * d_backing_dentry - Get upper or lower dentry we should be using
- * @upper: The upper layer
- *
- * This is the helper that should be used to get the dentry of the inode that
- * will be used if this dentry were opened as a file.  It may be the upper
- * dentry or it may be a lower dentry pinned by the upper.
- *
- * Normal filesystems should not use this to access their own dentries.
+ * d_backing_inode - same as d_inode(). Use d_inode() instead.
+ * @dentry: dentry to query
  */
-static inline struct dentry *d_backing_dentry(struct dentry *upper)
+static inline struct inode *d_backing_inode(const struct dentry *dentry)
 {
-	return upper;
+	return d_inode(dentry);
 }
 
 /* d_real() flags */