[PATCH 0/3] VFS: Revert irrelevant dcache patches

[PATCH 0/3] VFS: Revert irrelevant dcache patches

[Goldwyn Rodrigues]

These are some patches which were included in dcache during
unionfs development but are not used, or partially used.
These do not seem to be relevant to overlayfs either because
overlay uses it's own flags to represent whiteouts. Fallthru
was never used.

Regards,

-- 
Goldwyn

[PATCH 1/3] Revert: "VFS: Add a fallthrough flag for marking virtual dentries"

[Goldwyn Rodrigues]

From: Goldwyn Rodrigues <rgoldwyn@suse.com>

Reverts: df1a085af1f6 ("VFS: Add a fallthrough flag for marking
virtual dentries")

No users of DCACHE_FALLTHRU.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/dcache.c            | 20 ++------------------
 include/linux/dcache.h |  9 ---------
 2 files changed, 2 insertions(+), 27 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 0e8e5de3c48a..710afa5d9364 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -318,7 +318,7 @@ static inline void __d_set_inode_and_type(struct dentry *dentry,
 
 	dentry->d_inode = inode;
 	flags = READ_ONCE(dentry->d_flags);
-	flags &= ~(DCACHE_ENTRY_TYPE | DCACHE_FALLTHRU);
+	flags &= ~DCACHE_ENTRY_TYPE;
 	flags |= type_flags;
 	WRITE_ONCE(dentry->d_flags, flags);
 }
@@ -327,7 +327,7 @@ static inline void __d_clear_type_and_inode(struct dentry *dentry)
 {
 	unsigned flags = READ_ONCE(dentry->d_flags);
 
-	flags &= ~(DCACHE_ENTRY_TYPE | DCACHE_FALLTHRU);
+	flags &= ~DCACHE_ENTRY_TYPE;
 	WRITE_ONCE(dentry->d_flags, flags);
 	dentry->d_inode = NULL;
 }
@@ -1785,22 +1785,6 @@ void d_set_d_op(struct dentry *dentry, const struct dentry_operations *op)
 }
 EXPORT_SYMBOL(d_set_d_op);
 
-
-/*
- * d_set_fallthru - Mark a dentry as falling through to a lower layer
- * @dentry - The dentry to mark
- *
- * Mark a dentry as falling through to the lower layer (as set with
- * d_pin_lower()).  This flag may be recorded on the medium.
- */
-void d_set_fallthru(struct dentry *dentry)
-{
-	spin_lock(&dentry->d_lock);
-	dentry->d_flags |= DCACHE_FALLTHRU;
-	spin_unlock(&dentry->d_lock);
-}
-EXPORT_SYMBOL(d_set_fallthru);
-
 static unsigned d_flags_for_inode(struct inode *inode)
 {
 	unsigned add_flags = DCACHE_REGULAR_TYPE;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 66c6e17e61e5..39d377ba4208 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -211,7 +211,6 @@ struct dentry_operations {
 #define DCACHE_SYMLINK_TYPE		0x00600000 /* Symlink (or fallthru to such) */
 
 #define DCACHE_MAY_FREE			0x00800000
-#define DCACHE_FALLTHRU			0x01000000 /* Fall through to lower layer */
 #define DCACHE_ENCRYPTED_WITH_KEY	0x02000000 /* dir is encrypted with a valid key */
 #define DCACHE_OP_REAL			0x04000000
 
@@ -493,14 +492,6 @@ static inline int simple_positive(const struct dentry *dentry)
 	return d_really_is_positive(dentry) && !d_unhashed(dentry);
 }
 
-extern void d_set_fallthru(struct dentry *dentry);
-
-static inline bool d_is_fallthru(const struct dentry *dentry)
-{
-	return dentry->d_flags & DCACHE_FALLTHRU;
-}
-
-
 extern int sysctl_vfs_cache_pressure;
 
 static inline unsigned long vfs_pressure_ratio(unsigned long val)
-- 
2.16.4

[PATCH 2/3] Revert: "VFS: Add a whiteout dentry type"

[Goldwyn Rodrigues]

From: Goldwyn Rodrigues <rgoldwyn@suse.com>

Reverts e7f7d2253c05 ("VFS: Add a whiteout dentry type")
No users of DCACHE_WHITEOUT_TYPE. Overlay uses its own
flags to represent whiteouts.

d_is_miss() is not used.

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

diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 39d377ba4208..6bb1ba14af8d 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -202,13 +202,13 @@ struct dentry_operations {
 #define DCACHE_LRU_LIST			0x00080000
 
 #define DCACHE_ENTRY_TYPE		0x00700000
-#define DCACHE_MISS_TYPE		0x00000000 /* Negative dentry (maybe fallthru to nowhere) */
+#define DCACHE_MISS_TYPE		0x00000000 /* Negative dentry */
 #define DCACHE_WHITEOUT_TYPE		0x00100000 /* Whiteout dentry (stop pathwalk) */
 #define DCACHE_DIRECTORY_TYPE		0x00200000 /* Normal directory */
 #define DCACHE_AUTODIR_TYPE		0x00300000 /* Lookupless directory (presumed automount) */
-#define DCACHE_REGULAR_TYPE		0x00400000 /* Regular file type (or fallthru to such) */
-#define DCACHE_SPECIAL_TYPE		0x00500000 /* Other file type (or fallthru to such) */
-#define DCACHE_SYMLINK_TYPE		0x00600000 /* Symlink (or fallthru to such) */
+#define DCACHE_REGULAR_TYPE		0x00400000 /* Regular file type */
+#define DCACHE_SPECIAL_TYPE		0x00500000 /* Other file type */
+#define DCACHE_SYMLINK_TYPE		0x00600000 /* Symlink */
 
 #define DCACHE_MAY_FREE			0x00800000
 #define DCACHE_ENCRYPTED_WITH_KEY	0x02000000 /* dir is encrypted with a valid key */
@@ -393,16 +393,6 @@ static inline unsigned __d_entry_type(const struct dentry *dentry)
 	return dentry->d_flags & DCACHE_ENTRY_TYPE;
 }
 
-static inline bool d_is_miss(const struct dentry *dentry)
-{
-	return __d_entry_type(dentry) == DCACHE_MISS_TYPE;
-}
-
-static inline bool d_is_whiteout(const struct dentry *dentry)
-{
-	return __d_entry_type(dentry) == DCACHE_WHITEOUT_TYPE;
-}
-
 static inline bool d_can_lookup(const struct dentry *dentry)
 {
 	return __d_entry_type(dentry) == DCACHE_DIRECTORY_TYPE;
@@ -440,8 +430,7 @@ static inline bool d_is_file(const struct dentry *dentry)
 
 static inline bool d_is_negative(const struct dentry *dentry)
 {
-	// TODO: check d_is_whiteout(dentry) also.
-	return d_is_miss(dentry);
+	return __d_entry_type(dentry) == DCACHE_MISS_TYPE;
 }
 
 static inline bool d_is_positive(const struct dentry *dentry)
-- 
2.16.4

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

[Goldwyn Rodrigues]

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(-)

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 */
-- 
2.16.4

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

[Christoph Hellwig]

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?

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

[Goldwyn Rodrigues]

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.

-- 
Goldwyn

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

[Christoph Hellwig]

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.

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

[Amir Goldstein]

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.