[PATCH 0/8] remove d_time from dentry

[PATCH 0/8] remove d_time from dentry

[Miklos Szeredi]

dentry->d_time is not used by the VFS anymore, it's essentially a fs-private
data.  And it just wastes space in the dentry for the vast majority of
filesystems.

This series moves the few uses to ->d_fsdata.  Introduce ->d_allocate() method
to make it easier to allocate fs specific structure together with the dentry.

---
Miklos Szeredi (8):
  vfs: new d_allocate method
  ceph: don't use ->d_time
  cifs: don't use ->d_time
  vfat: don't use ->d_time
  fuse: don't use ->d_time
  nfs: don't use ->d_time
  ncpfs: don't use ->d_time
  vfs: remove ->d_time

 Documentation/filesystems/Locking |  2 ++
 Documentation/filesystems/vfs.txt |  3 +++
 fs/ceph/dir.c                     |  6 +++---
 fs/ceph/inode.c                   |  4 ++--
 fs/ceph/mds_client.c              |  4 ++--
 fs/ceph/super.h                   |  2 +-
 fs/cifs/cifsfs.h                  | 10 ++++++++++
 fs/cifs/dir.c                     |  6 +++---
 fs/cifs/inode.c                   |  2 +-
 fs/dcache.c                       | 11 +++++++++++
 fs/fat/namei_vfat.c               | 19 +++++++++++++++----
 fs/fuse/dir.c                     | 36 ++++++++++++++++--------------------
 fs/ncpfs/dir.c                    | 21 ++++++++++++++++++---
 fs/ncpfs/ncplib_kernel.h          | 16 +++++++++++++---
 fs/nfs/dir.c                      | 18 ++++++++++++++----
 fs/nfs/getroot.c                  |  4 ++--
 fs/nfs/namespace.c                |  2 +-
 fs/nfs/unlink.c                   | 16 ++++++++--------
 include/linux/dcache.h            |  8 ++++----
 include/linux/nfs_fs.h            | 18 +++++++++++++++++-
 20 files changed, 146 insertions(+), 62 deletions(-)

-- 
2.5.5

[PATCH 1/8] vfs: new d_allocate method

[Miklos Szeredi]

Allow filesystem to initialize dentry (->d_fsdata to be explicit) at
allocation time.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 Documentation/filesystems/Locking |  2 ++
 Documentation/filesystems/vfs.txt |  3 +++
 fs/dcache.c                       | 11 +++++++++++
 include/linux/dcache.h            |  1 +
 4 files changed, 17 insertions(+)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 75eea7ce3d7c..6da7d7363709 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -15,6 +15,7 @@ prototypes:
 	int (*d_compare)(const struct dentry *, const struct dentry *,
 			unsigned int, const char *, const struct qstr *);
 	int (*d_delete)(struct dentry *);
+	int (*d_allocate)(struct dentry *);
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)((struct dentry *dentry, char *buffer, int buflen);
@@ -28,6 +29,7 @@ d_weak_revalidate:no		no		yes	 	no
 d_hash		no		no		no		maybe
 d_compare:	yes		no		no		maybe
 d_delete:	no		yes		no		no
+d_allocate:	no		no		yes		no
 d_release:	no		no		yes		no
 d_prune:        no              yes             no              no
 d_iput:		no		no		yes		no
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index c61a223ef3ff..4d8f1a5d8936 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -933,6 +933,7 @@ struct dentry_operations {
 	int (*d_compare)(const struct dentry *, const struct dentry *,
 			unsigned int, const char *, const struct qstr *);
 	int (*d_delete)(const struct dentry *);
+	int (*d_allocate)(struct dentry *);
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
@@ -1003,6 +1004,8 @@ struct dentry_operations {
 	always cache a reachable dentry. d_delete must be constant and
 	idempotent.
 
+  d_allocate: called when a dentry is really allocated
+
   d_release: called when a dentry is really deallocated
 
   d_iput: called when a dentry loses its inode (just prior to its
diff --git a/fs/dcache.c b/fs/dcache.c
index 817c243c1ff1..dc573af50edc 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1559,6 +1559,7 @@ struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
 {
 	struct dentry *dentry;
 	char *dname;
+	int err;
 
 	dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL);
 	if (!dentry)
@@ -1617,6 +1618,16 @@ struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
 	INIT_LIST_HEAD(&dentry->d_child);
 	d_set_d_op(dentry, dentry->d_sb->s_d_op);
 
+	if (dentry->d_op && dentry->d_op->d_allocate) {
+		err = dentry->d_op->d_allocate(dentry);
+		if (err) {
+			if (dname_external(dentry))
+				kfree(external_name(dentry));
+			kmem_cache_free(dentry_cache, dentry);
+			return NULL;
+		}
+	}
+
 	this_cpu_inc(nr_dentry);
 
 	return dentry;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index f28100f6b556..a2f01284b277 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -133,6 +133,7 @@ struct dentry_operations {
 	int (*d_compare)(const struct dentry *, const struct dentry *,
 			unsigned int, const char *, const struct qstr *);
 	int (*d_delete)(const struct dentry *);
+	int (*d_allocate)(struct dentry *);
 	void (*d_release)(struct dentry *);
 	void (*d_prune)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
-- 
2.5.5

Re: [PATCH 1/8] vfs: new d_allocate method

[Al Viro]

On Wed, Jun 22, 2016 at 04:35:03PM +0200, Miklos Szeredi wrote:
> Allow filesystem to initialize dentry (->d_fsdata to be explicit) at
> allocation time.

Something similar had been discussed a while ago (I don't remember whether
you'd been on Cc, though).  E.g. ceph and lustre would benefit from having
such method (proposed name was ->d_init(), seeing that it's not always
just an allocation).  The subtle part is barriers and I would really like
to see a proof that you don't need any.

Re: [PATCH 1/8] vfs: new d_allocate method

[Miklos Szeredi]

On Wed, Jun 22, 2016 at 7:02 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Wed, Jun 22, 2016 at 04:35:03PM +0200, Miklos Szeredi wrote:
>> Allow filesystem to initialize dentry (->d_fsdata to be explicit) at
>> allocation time.
>
> Something similar had been discussed a while ago (I don't remember whether
> you'd been on Cc, though).  E.g. ceph and lustre would benefit from having
> such method (proposed name was ->d_init(), seeing that it's not always
> just an allocation).  The subtle part is barriers and I would really like
> to see a proof that you don't need any.

Thanks for the review.  Yeah, I missed the rcu concequences.  And I
also didn't benchmark anything, it's obviously going to hurt somewhat
for fs that need to alloc an fsdata due to this.  I'm waiting to hear
better ideas.

One, rather ugly, is to make the max internal name length fs depenent,
and allow fs to use that space if it wants to trading it for a longer
d_iname.

Thaks,
Miklos

[PATCH 2/8] ceph: don't use ->d_time

[Miklos Szeredi]

Pretty simple: just use ceph_dentry_info.time instead (which was already
there, unused).

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Yan, Zheng <zyan@redhat.com>
---
 fs/ceph/dir.c        | 6 +++---
 fs/ceph/inode.c      | 4 ++--
 fs/ceph/mds_client.c | 4 ++--
 fs/ceph/super.h      | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 6e0fedf6713b..8ff7bcc7fc88 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -59,7 +59,7 @@ int ceph_init_dentry(struct dentry *dentry)
 
 	di->dentry = dentry;
 	di->lease_session = NULL;
-	dentry->d_time = jiffies;
+	di->time = jiffies;
 	/* avoid reordering d_fsdata setup so that the check above is safe */
 	smp_mb();
 	dentry->d_fsdata = di;
@@ -1124,7 +1124,7 @@ static int ceph_rename(struct inode *old_dir, struct dentry *old_dentry,
 void ceph_invalidate_dentry_lease(struct dentry *dentry)
 {
 	spin_lock(&dentry->d_lock);
-	dentry->d_time = jiffies;
+	ceph_dentry(dentry)->time = jiffies;
 	ceph_dentry(dentry)->lease_shared_gen = 0;
 	spin_unlock(&dentry->d_lock);
 }
@@ -1154,7 +1154,7 @@ static int dentry_lease_is_valid(struct dentry *dentry)
 		spin_unlock(&s->s_gen_ttl_lock);
 
 		if (di->lease_gen == gen &&
-		    time_before(jiffies, dentry->d_time) &&
+		    time_before(jiffies, di->time) &&
 		    time_before(jiffies, ttl)) {
 			valid = 1;
 			if (di->lease_renew_after &&
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index f059b5997072..7a33178ef850 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -1018,7 +1018,7 @@ static void update_dentry_lease(struct dentry *dentry,
 		goto out_unlock;
 
 	if (di->lease_gen == session->s_cap_gen &&
-	    time_before(ttl, dentry->d_time))
+	    time_before(ttl, di->time))
 		goto out_unlock;  /* we already have a newer lease. */
 
 	if (di->lease_session && di->lease_session != session)
@@ -1032,7 +1032,7 @@ static void update_dentry_lease(struct dentry *dentry,
 	di->lease_seq = le32_to_cpu(lease->seq);
 	di->lease_renew_after = half_ttl;
 	di->lease_renew_from = 0;
-	dentry->d_time = ttl;
+	di->time = ttl;
 out_unlock:
 	spin_unlock(&dentry->d_lock);
 	return;
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 2103b823bec0..db9c654d42cd 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3231,7 +3231,7 @@ static void handle_lease(struct ceph_mds_client *mdsc,
 				msecs_to_jiffies(le32_to_cpu(h->duration_ms));
 
 			di->lease_seq = seq;
-			dentry->d_time = di->lease_renew_from + duration;
+			di->time = di->lease_renew_from + duration;
 			di->lease_renew_after = di->lease_renew_from +
 				(duration >> 1);
 			di->lease_renew_from = 0;
@@ -3316,7 +3316,7 @@ void ceph_mdsc_lease_release(struct ceph_mds_client *mdsc, struct inode *inode,
 	if (!di || !di->lease_session ||
 	    di->lease_session->s_mds < 0 ||
 	    di->lease_gen != di->lease_session->s_cap_gen ||
-	    !time_before(jiffies, dentry->d_time)) {
+	    !time_before(jiffies, di->time)) {
 		dout("lease_release inode %p dentry %p -- "
 		     "no lease\n",
 		     inode, dentry);
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 0168b49fb6ad..10776db93143 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -246,7 +246,7 @@ struct ceph_dentry_info {
 	unsigned long lease_renew_after, lease_renew_from;
 	struct list_head lru;
 	struct dentry *dentry;
-	u64 time;
+	unsigned long time;
 	u64 offset;
 };
 
-- 
2.5.5

Re: [PATCH 2/8] ceph: don't use ->d_time

[Yan, Zheng]

> On Jun 22, 2016, at 22:35, Miklos Szeredi <mszeredi@redhat.com> wrote:
> 
> Pretty simple: just use ceph_dentry_info.time instead (which was already
> there, unused).
> 
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> Cc: Yan, Zheng <zyan@redhat.com>
> ---
> fs/ceph/dir.c        | 6 +++---
> fs/ceph/inode.c      | 4 ++--
> fs/ceph/mds_client.c | 4 ++--
> fs/ceph/super.h      | 2 +-
> 4 files changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Yan, Zheng <zyan@redhat.com>

> 
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 6e0fedf6713b..8ff7bcc7fc88 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -59,7 +59,7 @@ int ceph_init_dentry(struct dentry *dentry)
> 
> 	di->dentry = dentry;
> 	di->lease_session = NULL;
> -	dentry->d_time = jiffies;
> +	di->time = jiffies;
> 	/* avoid reordering d_fsdata setup so that the check above is safe */
> 	smp_mb();
> 	dentry->d_fsdata = di;
> @@ -1124,7 +1124,7 @@ static int ceph_rename(struct inode *old_dir, struct dentry *old_dentry,
> void ceph_invalidate_dentry_lease(struct dentry *dentry)
> {
> 	spin_lock(&dentry->d_lock);
> -	dentry->d_time = jiffies;
> +	ceph_dentry(dentry)->time = jiffies;
> 	ceph_dentry(dentry)->lease_shared_gen = 0;
> 	spin_unlock(&dentry->d_lock);
> }
> @@ -1154,7 +1154,7 @@ static int dentry_lease_is_valid(struct dentry *dentry)
> 		spin_unlock(&s->s_gen_ttl_lock);
> 
> 		if (di->lease_gen == gen &&
> -		    time_before(jiffies, dentry->d_time) &&
> +		    time_before(jiffies, di->time) &&
> 		    time_before(jiffies, ttl)) {
> 			valid = 1;
> 			if (di->lease_renew_after &&
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index f059b5997072..7a33178ef850 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -1018,7 +1018,7 @@ static void update_dentry_lease(struct dentry *dentry,
> 		goto out_unlock;
> 
> 	if (di->lease_gen == session->s_cap_gen &&
> -	    time_before(ttl, dentry->d_time))
> +	    time_before(ttl, di->time))
> 		goto out_unlock;  /* we already have a newer lease. */
> 
> 	if (di->lease_session && di->lease_session != session)
> @@ -1032,7 +1032,7 @@ static void update_dentry_lease(struct dentry *dentry,
> 	di->lease_seq = le32_to_cpu(lease->seq);
> 	di->lease_renew_after = half_ttl;
> 	di->lease_renew_from = 0;
> -	dentry->d_time = ttl;
> +	di->time = ttl;
> out_unlock:
> 	spin_unlock(&dentry->d_lock);
> 	return;
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 2103b823bec0..db9c654d42cd 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -3231,7 +3231,7 @@ static void handle_lease(struct ceph_mds_client *mdsc,
> 				msecs_to_jiffies(le32_to_cpu(h->duration_ms));
> 
> 			di->lease_seq = seq;
> -			dentry->d_time = di->lease_renew_from + duration;
> +			di->time = di->lease_renew_from + duration;
> 			di->lease_renew_after = di->lease_renew_from +
> 				(duration >> 1);
> 			di->lease_renew_from = 0;
> @@ -3316,7 +3316,7 @@ void ceph_mdsc_lease_release(struct ceph_mds_client *mdsc, struct inode *inode,
> 	if (!di || !di->lease_session ||
> 	    di->lease_session->s_mds < 0 ||
> 	    di->lease_gen != di->lease_session->s_cap_gen ||
> -	    !time_before(jiffies, dentry->d_time)) {
> +	    !time_before(jiffies, di->time)) {
> 		dout("lease_release inode %p dentry %p -- "
> 		     "no lease\n",
> 		     inode, dentry);
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index 0168b49fb6ad..10776db93143 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -246,7 +246,7 @@ struct ceph_dentry_info {
> 	unsigned long lease_renew_after, lease_renew_from;
> 	struct list_head lru;
> 	struct dentry *dentry;
> -	u64 time;
> +	unsigned long time;
> 	u64 offset;
> };
> 
> -- 
> 2.5.5
> 

Re: [PATCH 2/8] ceph: don't use ->d_time

[Miklos Szeredi]

On Thu, Jun 23, 2016 at 8:21 AM, Yan, Zheng <zyan@redhat.com> wrote:
>
>> On Jun 22, 2016, at 22:35, Miklos Szeredi <mszeredi@redhat.com> wrote:
>>
>> Pretty simple: just use ceph_dentry_info.time instead (which was already
>> there, unused).
>>
>> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
>> Cc: Yan, Zheng <zyan@redhat.com>
>> ---
>> fs/ceph/dir.c        | 6 +++---
>> fs/ceph/inode.c      | 4 ++--
>> fs/ceph/mds_client.c | 4 ++--
>> fs/ceph/super.h      | 2 +-
>> 4 files changed, 8 insertions(+), 8 deletions(-)
>
> Reviewed-by: Yan, Zheng <zyan@redhat.com>

Can you please take this through your tree?

Thanks,
Miklos

Re: [PATCH 2/8] ceph: don't use ->d_time

[Yan, Zheng]

> On Jun 28, 2016, at 16:09, Miklos Szeredi <mszeredi@redhat.com> wrote:
> 
> On Thu, Jun 23, 2016 at 8:21 AM, Yan, Zheng <zyan@redhat.com> wrote:
>> 
>>> On Jun 22, 2016, at 22:35, Miklos Szeredi <mszeredi@redhat.com> wrote:
>>> 
>>> Pretty simple: just use ceph_dentry_info.time instead (which was already
>>> there, unused).
>>> 
>>> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
>>> Cc: Yan, Zheng <zyan@redhat.com>
>>> ---
>>> fs/ceph/dir.c        | 6 +++---
>>> fs/ceph/inode.c      | 4 ++--
>>> fs/ceph/mds_client.c | 4 ++--
>>> fs/ceph/super.h      | 2 +-
>>> 4 files changed, 8 insertions(+), 8 deletions(-)
>> 
>> Reviewed-by: Yan, Zheng <zyan@redhat.com>
> 
> Can you please take this through your tree?

applied, thanks

Yan, Zheng


> 
> Thanks,
> Miklos

[PATCH 3/8] cifs: don't use ->d_time

[Miklos Szeredi]

Use d_fsdata instead, which is the same size.  Introduce helpers to hide
the typecasts.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Steve French <sfrench@samba.org>
---
 fs/cifs/cifsfs.h | 10 ++++++++++
 fs/cifs/dir.c    |  6 +++---
 fs/cifs/inode.c  |  2 +-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 9dcf974acc47..c9c00a862036 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -41,6 +41,16 @@ cifs_uniqueid_to_ino_t(u64 fileid)
 
 }
 
+static inline void cifs_set_time(struct dentry *dentry, unsigned long time)
+{
+	dentry->d_fsdata = (void *) time;
+}
+
+static inline unsigned long cifs_get_time(struct dentry *dentry)
+{
+	return (unsigned long) dentry->d_fsdata;
+}
+
 extern struct file_system_type cifs_fs_type;
 extern const struct address_space_operations cifs_addr_ops;
 extern const struct address_space_operations cifs_addr_ops_smallbuf;
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index c3eb998a99bd..49e13ecbdca9 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -40,7 +40,7 @@ renew_parental_timestamps(struct dentry *direntry)
 	/* BB check if there is a way to get the kernel to do this or if we
 	   really need this */
 	do {
-		direntry->d_time = jiffies;
+		cifs_set_time(direntry, jiffies);
 		direntry = direntry->d_parent;
 	} while (!IS_ROOT(direntry));
 }
@@ -768,7 +768,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
 
 	} else if (rc == -ENOENT) {
 		rc = 0;
-		direntry->d_time = jiffies;
+		cifs_set_time(direntry, jiffies);
 		d_add(direntry, NULL);
 	/*	if it was once a directory (but how can we tell?) we could do
 		shrink_dcache_parent(direntry); */
@@ -828,7 +828,7 @@ cifs_d_revalidate(struct dentry *direntry, unsigned int flags)
 	if (flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET))
 		return 0;
 
-	if (time_after(jiffies, direntry->d_time + HZ) || !lookupCacheEnabled)
+	if (time_after(jiffies, cifs_get_time(direntry) + HZ) || !lookupCacheEnabled)
 		return 0;
 
 	return 1;
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 514dadb0575d..cc916adb49f0 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1933,7 +1933,7 @@ int cifs_revalidate_dentry_attr(struct dentry *dentry)
 
 	cifs_dbg(FYI, "Update attributes: %s inode 0x%p count %d dentry: 0x%p d_time %ld jiffies %ld\n",
 		 full_path, inode, inode->i_count.counter,
-		 dentry, dentry->d_time, jiffies);
+		 dentry, cifs_get_time(dentry), jiffies);
 
 	if (cifs_sb_master_tcon(CIFS_SB(sb))->unix_ext)
 		rc = cifs_get_inode_info_unix(&inode, full_path, sb, xid);
-- 
2.5.5

[PATCH 4/8] vfat: don't use ->d_time

[Miklos Szeredi]

Use d_fsdata instead, which is the same size.  Introduce helpers to hide
the typecasts.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
 fs/fat/namei_vfat.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index 7092584f424a..cb27cd818572 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -21,6 +21,17 @@
 #include <linux/namei.h>
 #include "fat.h"
 
+static inline unsigned long vfat_d_version(struct dentry *dentry)
+{
+	return (unsigned long) dentry->d_fsdata;
+}
+
+static inline void vfat_d_version_set(struct dentry *dentry,
+				      unsigned long version)
+{
+	dentry->d_fsdata = (void *) version;
+}
+
 /*
  * If new entry was created in the parent, it could create the 8.3
  * alias (the shortname of logname).  So, the parent may have the
@@ -33,7 +44,7 @@ static int vfat_revalidate_shortname(struct dentry *dentry)
 {
 	int ret = 1;
 	spin_lock(&dentry->d_lock);
-	if (dentry->d_time != d_inode(dentry->d_parent)->i_version)
+	if (vfat_d_version(dentry) != d_inode(dentry->d_parent)->i_version)
 		ret = 0;
 	spin_unlock(&dentry->d_lock);
 	return ret;
@@ -759,7 +770,7 @@ static struct dentry *vfat_lookup(struct inode *dir, struct dentry *dentry,
 out:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
 	if (!inode)
-		dentry->d_time = dir->i_version;
+		vfat_d_version_set(dentry, dir->i_version);
 	return d_splice_alias(inode, dentry);
 error:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
@@ -823,7 +834,7 @@ static int vfat_rmdir(struct inode *dir, struct dentry *dentry)
 	clear_nlink(inode);
 	inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
 	fat_detach(inode);
-	dentry->d_time = dir->i_version;
+	vfat_d_version_set(dentry, dir->i_version);
 out:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
 
@@ -849,7 +860,7 @@ static int vfat_unlink(struct inode *dir, struct dentry *dentry)
 	clear_nlink(inode);
 	inode->i_mtime = inode->i_atime = CURRENT_TIME_SEC;
 	fat_detach(inode);
-	dentry->d_time = dir->i_version;
+	vfat_d_version_set(dentry, dir->i_version);
 out:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
 
-- 
2.5.5

[PATCH 5/8] fuse: don't use ->d_time

[Miklos Szeredi]

Store in memory pointed to by ->d_fsdata.  Use ->d_allocate() to allocate
the storage.

We could cast ->d_fsdata directly on 64bit archs, but I don't think this is
worth the extra complexity.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/dir.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ccd4971cc6c1..1a1dabb72036 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -37,37 +37,20 @@ static void fuse_advise_use_readdirplus(struct inode *dir)
 	set_bit(FUSE_I_ADVISE_RDPLUS, &fi->state);
 }
 
-#if BITS_PER_LONG >= 64
 static inline void fuse_dentry_settime(struct dentry *entry, u64 time)
 {
-	entry->d_time = time;
+	*(u64 *) entry->d_fsdata = time;
 }
 
 static inline u64 fuse_dentry_time(struct dentry *entry)
 {
-	return entry->d_time;
+	return *(u64 *) entry->d_fsdata;
 }
-#else
-/*
- * On 32 bit archs store the high 32 bits of time in d_fsdata
- */
-static void fuse_dentry_settime(struct dentry *entry, u64 time)
-{
-	entry->d_time = time;
-	entry->d_fsdata = (void *) (unsigned long) (time >> 32);
-}
-
-static u64 fuse_dentry_time(struct dentry *entry)
-{
-	return (u64) entry->d_time +
-		((u64) (unsigned long) entry->d_fsdata << 32);
-}
-#endif
 
 /*
  * FUSE caches dentries and attributes with separate timeout.  The
  * time in jiffies until the dentry/attributes are valid is stored in
- * dentry->d_time and fuse_inode->i_time respectively.
+ * dentry->d_fsdata and fuse_inode->i_time respectively.
  */
 
 /*
@@ -272,8 +255,21 @@ static int invalid_nodeid(u64 nodeid)
 	return !nodeid || nodeid == FUSE_ROOT_ID;
 }
 
+static int fuse_dentry_allocate(struct dentry *dentry)
+{
+	dentry->d_fsdata = kzalloc(sizeof(u64), GFP_KERNEL);
+
+	return dentry->d_fsdata ? 0 : -ENOMEM;
+}
+static void fuse_dentry_release(struct dentry *dentry)
+{
+	kfree(dentry->d_fsdata);
+}
+
 const struct dentry_operations fuse_dentry_operations = {
 	.d_revalidate	= fuse_dentry_revalidate,
+	.d_allocate	= fuse_dentry_allocate,
+	.d_release	= fuse_dentry_release,
 };
 
 int fuse_valid_type(int m)
-- 
2.5.5

Re: [PATCH 5/8] fuse: don't use ->d_time

[Al Viro]

On Wed, Jun 22, 2016 at 04:35:07PM +0200, Miklos Szeredi wrote:
> Store in memory pointed to by ->d_fsdata.  Use ->d_allocate() to allocate
> the storage.
> 
> We could cast ->d_fsdata directly on 64bit archs, but I don't think this is
> worth the extra complexity.

Now, _that_ is interesting:

> +static void fuse_dentry_release(struct dentry *dentry)
> +{
> +	kfree(dentry->d_fsdata);
> +}

What happens to fuse_dentry_revalidate() called on dentry in process of
getting dropped?  Unlike freeing struct dentry itself, ->d_release() is
not RCU-delayed.  So you are risking dereference of ->d_fsdata after
kfree(); at the very least, it needs RCU-delayed freeing...

[PATCH 6/8] nfs: don't use ->d_time

[Miklos Szeredi]

NFS is the most complicated of the lot.  It uses ->d_fsdata to

 1) store devname in the root dentry, and
 2) store nfs_unlinkdata for sillyrenames.

In addition it stores a verifier in ->d_time.

Introduce nfs_dentry structure that to store all of the above and make
d_fsdata point to it.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/dir.c           | 18 ++++++++++++++----
 fs/nfs/getroot.c       |  4 ++--
 fs/nfs/namespace.c     |  2 +-
 fs/nfs/unlink.c        | 16 ++++++++--------
 include/linux/nfs_fs.h | 18 +++++++++++++++++-
 5 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index aaf7bd0cbae2..ebb536c23b19 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1022,7 +1022,7 @@ static int nfs_check_verifier(struct inode *dir, struct dentry *dentry,
 		return 1;
 	if (NFS_SERVER(dir)->flags & NFS_MOUNT_LOOKUP_CACHE_NONE)
 		return 0;
-	if (!nfs_verify_change_attribute(dir, dentry->d_time))
+	if (!nfs_verify_change_attribute(dir, NFS_D(dentry)->verf))
 		return 0;
 	/* Revalidate nfsi->cache_change_attribute before we declare a match */
 	if (rcu_walk)
@@ -1031,7 +1031,7 @@ static int nfs_check_verifier(struct inode *dir, struct dentry *dentry,
 		ret = nfs_revalidate_inode(NFS_SERVER(dir), dir);
 	if (ret < 0)
 		return 0;
-	if (!nfs_verify_change_attribute(dir, dentry->d_time))
+	if (!nfs_verify_change_attribute(dir, NFS_D(dentry)->verf))
 		return 0;
 	return 1;
 }
@@ -1339,15 +1339,23 @@ static void nfs_dentry_iput(struct dentry *dentry, struct inode *inode)
 	iput(inode);
 }
 
+static int nfs_d_allocate(struct dentry *dentry)
+{
+	dentry->d_fsdata = kzalloc(sizeof(struct nfs_dentry), GFP_KERNEL);
+
+	return dentry->d_fsdata ? 0 : -ENOMEM;
+}
+
 static void nfs_d_release(struct dentry *dentry)
 {
 	/* free cached devname value, if it survived that far */
-	if (unlikely(dentry->d_fsdata)) {
+	if (unlikely(NFS_D(dentry)->devname)) {
 		if (dentry->d_flags & DCACHE_NFSFS_RENAMED)
 			WARN_ON(1);
 		else
-			kfree(dentry->d_fsdata);
+			kfree(NFS_D(dentry)->devname);
 	}
+	kfree(dentry->d_fsdata);
 }
 
 const struct dentry_operations nfs_dentry_operations = {
@@ -1356,6 +1364,7 @@ const struct dentry_operations nfs_dentry_operations = {
 	.d_delete	= nfs_dentry_delete,
 	.d_iput		= nfs_dentry_iput,
 	.d_automount	= nfs_d_automount,
+	.d_allocate	= nfs_d_allocate,
 	.d_release	= nfs_d_release,
 };
 EXPORT_SYMBOL_GPL(nfs_dentry_operations);
@@ -1437,6 +1446,7 @@ const struct dentry_operations nfs4_dentry_operations = {
 	.d_delete	= nfs_dentry_delete,
 	.d_iput		= nfs_dentry_iput,
 	.d_automount	= nfs_d_automount,
+	.d_allocate	= nfs_d_allocate,
 	.d_release	= nfs_d_release,
 };
 EXPORT_SYMBOL_GPL(nfs4_dentry_operations);
diff --git a/fs/nfs/getroot.c b/fs/nfs/getroot.c
index a608ffd28acc..84482c656e05 100644
--- a/fs/nfs/getroot.c
+++ b/fs/nfs/getroot.c
@@ -120,9 +120,9 @@ struct dentry *nfs_get_root(struct super_block *sb, struct nfs_fh *mntfh,
 
 	security_d_instantiate(ret, inode);
 	spin_lock(&ret->d_lock);
-	if (IS_ROOT(ret) && !ret->d_fsdata &&
+	if (IS_ROOT(ret) && !NFS_D(ret)->devname &&
 	    !(ret->d_flags & DCACHE_NFSFS_RENAMED)) {
-		ret->d_fsdata = name;
+		NFS_D(ret)->devname = name;
 		name = NULL;
 	}
 	spin_unlock(&ret->d_lock);
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index c8162c660c44..0060806d047b 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -90,7 +90,7 @@ rename_retry:
 		*--end = '/';
 	}
 	*p = end;
-	base = dentry->d_fsdata;
+	base = NFS_D(dentry)->devname;
 	if (!base) {
 		spin_unlock(&dentry->d_lock);
 		rcu_read_unlock();
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 1868246f56e6..d4c442a5f56a 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -134,8 +134,8 @@ static int nfs_call_unlink(struct dentry *dentry, struct nfs_unlinkdata *data)
 		spin_lock(&alias->d_lock);
 		if (d_really_is_positive(alias) &&
 		    !(alias->d_flags & DCACHE_NFSFS_RENAMED)) {
-			devname_garbage = alias->d_fsdata;
-			alias->d_fsdata = data;
+			devname_garbage = NFS_D(alias)->devname;
+			NFS_D(alias)->data = data;
 			alias->d_flags |= DCACHE_NFSFS_RENAMED;
 			ret = 1;
 		} else
@@ -189,8 +189,8 @@ nfs_async_unlink(struct dentry *dentry, struct qstr *name)
 	if (dentry->d_flags & DCACHE_NFSFS_RENAMED)
 		goto out_unlock;
 	dentry->d_flags |= DCACHE_NFSFS_RENAMED;
-	devname_garbage = dentry->d_fsdata;
-	dentry->d_fsdata = data;
+	devname_garbage = NFS_D(dentry)->devname;
+	NFS_D(dentry)->data = data;
 	spin_unlock(&dentry->d_lock);
 	/*
 	 * If we'd displaced old cached devname, free it.  At that
@@ -226,8 +226,8 @@ nfs_complete_unlink(struct dentry *dentry, struct inode *inode)
 
 	spin_lock(&dentry->d_lock);
 	dentry->d_flags &= ~DCACHE_NFSFS_RENAMED;
-	data = dentry->d_fsdata;
-	dentry->d_fsdata = NULL;
+	data = NFS_D(dentry)->data;
+	NFS_D(dentry)->data = NULL;
 	spin_unlock(&dentry->d_lock);
 
 	if (NFS_STALE(inode) || !nfs_call_unlink(dentry, data))
@@ -240,10 +240,10 @@ nfs_cancel_async_unlink(struct dentry *dentry)
 {
 	spin_lock(&dentry->d_lock);
 	if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
-		struct nfs_unlinkdata *data = dentry->d_fsdata;
+		struct nfs_unlinkdata *data = NFS_D(dentry)->data;
 
 		dentry->d_flags &= ~DCACHE_NFSFS_RENAMED;
-		dentry->d_fsdata = NULL;
+		NFS_D(dentry)->data = NULL;
 		spin_unlock(&dentry->d_lock);
 		nfs_free_unlinkdata(data);
 		return;
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index d71278c3c5bd..a33fb67ddd1c 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -187,6 +187,17 @@ struct nfs_inode {
 };
 
 /*
+ * NFS specific dentry data
+ */
+struct nfs_dentry {
+	union {
+		char *devname;
+		struct nfs_unlinkdata *data;
+	};
+	unsigned long verf;
+};
+
+/*
  * Cache validity bit flags
  */
 #define NFS_INO_INVALID_ATTR	0x0001		/* cached attrs are invalid */
@@ -217,6 +228,11 @@ static inline struct nfs_inode *NFS_I(const struct inode *inode)
 	return container_of(inode, struct nfs_inode, vfs_inode);
 }
 
+static inline struct nfs_dentry *NFS_D(struct dentry *dentry)
+{
+	return (struct nfs_dentry *) dentry->d_fsdata;
+}
+
 static inline struct nfs_server *NFS_SB(const struct super_block *s)
 {
 	return (struct nfs_server *)(s->s_fs_info);
@@ -299,7 +315,7 @@ static inline int nfs_server_capable(struct inode *inode, int cap)
 
 static inline void nfs_set_verifier(struct dentry * dentry, unsigned long verf)
 {
-	dentry->d_time = verf;
+	NFS_D(dentry)->verf = verf;
 }
 
 /**
-- 
2.5.5

Re: [PATCH 6/8] nfs: don't use ->d_time

[Al Viro]

On Wed, Jun 22, 2016 at 04:35:08PM +0200, Miklos Szeredi wrote:
>  static void nfs_d_release(struct dentry *dentry)
>  {
>  	/* free cached devname value, if it survived that far */
> -	if (unlikely(dentry->d_fsdata)) {
> +	if (unlikely(NFS_D(dentry)->devname)) {
>  		if (dentry->d_flags & DCACHE_NFSFS_RENAMED)
>  			WARN_ON(1);
>  		else
> -			kfree(dentry->d_fsdata);
> +			kfree(NFS_D(dentry)->devname);
>  	}
> +	kfree(dentry->d_fsdata);

Again, is that safe wrt RCU delays?

[PATCH 7/8] ncpfs: don't use ->d_time

[Miklos Szeredi]

Ncpfs stores a boolean value in ->d_fsdata as well as using d_time.
Introcude ncp_dentry to store both of these and make d_fsdata point to it.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/ncpfs/dir.c           | 21 ++++++++++++++++++---
 fs/ncpfs/ncplib_kernel.h | 16 +++++++++++++---
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/fs/ncpfs/dir.c b/fs/ncpfs/dir.c
index bfdad003ee56..805d76dba426 100644
--- a/fs/ncpfs/dir.c
+++ b/fs/ncpfs/dir.c
@@ -72,6 +72,8 @@ const struct inode_operations ncp_dir_inode_operations =
 /*
  * Dentry operations routines
  */
+static int ncp_d_allocate(struct dentry *);
+static void ncp_d_release(struct dentry *);
 static int ncp_lookup_validate(struct dentry *, unsigned int);
 static int ncp_hash_dentry(const struct dentry *, struct qstr *);
 static int ncp_compare_dentry(const struct dentry *, const struct dentry *,
@@ -81,6 +83,8 @@ static void ncp_d_prune(struct dentry *dentry);
 
 const struct dentry_operations ncp_dentry_operations =
 {
+	.d_allocate	= ncp_d_allocate,
+	.d_release	= ncp_d_release,
 	.d_revalidate	= ncp_lookup_validate,
 	.d_hash		= ncp_hash_dentry,
 	.d_compare	= ncp_compare_dentry,
@@ -306,6 +310,17 @@ leave_me:;
 }
 #endif	/* CONFIG_NCPFS_STRONG */
 
+static int ncp_d_allocate(struct dentry *dentry)
+{
+	dentry->d_fsdata = kzalloc(sizeof(struct ncp_dentry), GFP_KERNEL);
+
+	return dentry->d_fsdata ? 0 : -ENOMEM;
+}
+
+static void ncp_d_release(struct dentry *dentry)
+{
+	kfree(dentry->d_fsdata);
+}
 
 static int
 ncp_lookup_validate(struct dentry *dentry, unsigned int flags)
@@ -409,7 +424,7 @@ ncp_invalidate_dircache_entries(struct dentry *parent)
 
 	spin_lock(&parent->d_lock);
 	list_for_each_entry(dentry, &parent->d_subdirs, d_child) {
-		dentry->d_fsdata = NULL;
+		NCP_DENTRY(dentry)->cached = false;
 		ncp_age_dentry(server, dentry);
 	}
 	spin_unlock(&parent->d_lock);
@@ -569,7 +584,7 @@ out:
 
 static void ncp_d_prune(struct dentry *dentry)
 {
-	if (!dentry->d_fsdata)	/* not referenced from page cache */
+	if (!NCP_DENTRY(dentry)->cached) /* not referenced from page cache */
 		return;
 	NCP_FINFO(d_inode(dentry->d_parent))->flags &= ~NCPI_DIR_CACHE;
 }
@@ -660,7 +675,7 @@ ncp_fill_cache(struct file *file, struct dir_context *ctx,
 	}
 	if (ctl.cache) {
 		if (d_really_is_positive(newdent)) {
-			newdent->d_fsdata = newdent;
+			NCP_DENTRY(newdent)->cached = true;
 			ctl.cache->dentry[ctl.idx] = newdent;
 			ino = d_inode(newdent)->i_ino;
 			ncp_new_dentry(newdent);
diff --git a/fs/ncpfs/ncplib_kernel.h b/fs/ncpfs/ncplib_kernel.h
index 17cfb743b5bf..76459e4f6b64 100644
--- a/fs/ncpfs/ncplib_kernel.h
+++ b/fs/ncpfs/ncplib_kernel.h
@@ -168,20 +168,30 @@ static inline int ncp_strnicmp(const struct nls_table *t,
 
 #endif /* CONFIG_NCPFS_NLS */
 
-#define NCP_GET_AGE(dentry)	(jiffies - (dentry)->d_time)
+struct ncp_dentry {
+	unsigned long time;
+	bool cached;
+};
+
+static inline struct ncp_dentry *NCP_DENTRY(struct dentry *dentry)
+{
+	return (struct ncp_dentry *) dentry->d_fsdata;
+}
+
+#define NCP_GET_AGE(dentry)	(jiffies - NCP_DENTRY(dentry)->time)
 #define NCP_MAX_AGE(server)	atomic_read(&(server)->dentry_ttl)
 #define NCP_TEST_AGE(server,dentry)	(NCP_GET_AGE(dentry) < NCP_MAX_AGE(server))
 
 static inline void
 ncp_age_dentry(struct ncp_server* server, struct dentry* dentry)
 {
-	dentry->d_time = jiffies - NCP_MAX_AGE(server);
+	NCP_DENTRY(dentry)->time = jiffies - NCP_MAX_AGE(server);
 }
 
 static inline void
 ncp_new_dentry(struct dentry* dentry)
 {
-	dentry->d_time = jiffies;
+	NCP_DENTRY(dentry)->time = jiffies;
 }
 
 struct ncp_cache_head {
-- 
2.5.5

Re: [PATCH 7/8] ncpfs: don't use ->d_time

[Al Viro]

On Wed, Jun 22, 2016 at 04:35:09PM +0200, Miklos Szeredi wrote:
> Ncpfs stores a boolean value in ->d_fsdata as well as using d_time.
> Introcude ncp_dentry to store both of these and make d_fsdata point to it.

Umm...  I'd ask if you've profiled that change (you are adding a bunch of
dereferencing), but it's ncpfs we are talking about...

[PATCH 8/8] vfs: remove ->d_time

[Miklos Szeredi]

Remove dentry->d_time field, which is now unused.  This makes space for
longer inline names.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 include/linux/dcache.h | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index a2f01284b277..c2d43c5a8e29 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -69,12 +69,12 @@ extern struct dentry_stat_t dentry_stat;
  * large memory footprint increase).
  */
 #ifdef CONFIG_64BIT
-# define DNAME_INLINE_LEN 32 /* 192 bytes */
+# define DNAME_INLINE_LEN 40 /* 192 bytes */
 #else
 # ifdef CONFIG_SMP
-#  define DNAME_INLINE_LEN 36 /* 128 bytes */
-# else
 #  define DNAME_INLINE_LEN 40 /* 128 bytes */
+# else
+#  define DNAME_INLINE_LEN 44 /* 128 bytes */
 # endif
 #endif
 
@@ -95,7 +95,6 @@ struct dentry {
 	struct lockref d_lockref;	/* per-dentry lock and refcount */
 	const struct dentry_operations *d_op;
 	struct super_block *d_sb;	/* The root of the dentry tree */
-	unsigned long d_time;		/* used by d_revalidate */
 	void *d_fsdata;			/* fs-specific data */
 
 	union {
-- 
2.5.5

Re: [PATCH 0/8] remove d_time from dentry

[Miklos Szeredi]

Pushed an updated one to

  git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git d_time

Changes:

 - rename d_allocate to d_init
 - use kfree_rcu() for fuse and nfs dentry->d_fsdata

Thanks,
Miklos

---
Miklos Szeredi (8):
      vfs: new d_init method
      ceph: don't use ->d_time
      cifs: don't use ->d_time
      vfat: don't use ->d_time
      fuse: don't use ->d_time
      nfs: don't use ->d_time
      ncpfs: don't use ->d_time
      vfs: remove ->d_time

---
 Documentation/filesystems/Locking |  2 ++
 Documentation/filesystems/vfs.txt |  3 +++
 fs/ceph/dir.c                     |  6 +++---
 fs/ceph/inode.c                   |  4 ++--
 fs/ceph/mds_client.c              |  4 ++--
 fs/ceph/super.h                   |  2 +-
 fs/cifs/cifsfs.h                  | 10 +++++++++
 fs/cifs/dir.c                     |  6 +++---
 fs/cifs/inode.c                   |  2 +-
 fs/dcache.c                       | 11 ++++++++++
 fs/fat/namei_vfat.c               | 19 +++++++++++++----
 fs/fuse/dir.c                     | 43 +++++++++++++++++++++------------------
 fs/ncpfs/dir.c                    | 21 ++++++++++++++++---
 fs/ncpfs/ncplib_kernel.h          | 16 ++++++++++++---
 fs/nfs/dir.c                      | 18 ++++++++++++----
 fs/nfs/getroot.c                  |  4 ++--
 fs/nfs/namespace.c                |  2 +-
 fs/nfs/unlink.c                   | 16 +++++++--------
 include/linux/dcache.h            |  8 ++++----
 include/linux/nfs_fs.h            | 19 ++++++++++++++++-
 20 files changed, 154 insertions(+), 62 deletions(-)