[PATCH v2 0/2] Reduce direct access of nfsd to inode->i_mtime

[PATCH v2 0/2] Reduce direct access of nfsd to inode->i_mtime

[Amir Goldstein]

Bruce, Jeff,

These patches fix issues I found when running nfstest_posix
over NFS exported overlayfs. The issues stem from the fact that
the overlay inode i_mtime is often out of date w.r.t stat->mtime.

I am posting these separately from (soon to be posted) overlayfs NFS
export support patches to get your ACK, or, since those changes are
independent from overlayfs, you could carry them through your tree.

Please note that this does not eliminate all direct access of nfsd
to inode->i_mtime, but only the low hanging access sites, which also
happen to be the cases that matter to overlayfs.

Changes since v1 (RFC):
- Use stat->mtime times instead of inode->i_mtime where relevant,
  so overlayfs doesn't need to be special cased

Thanks,
Amir.

Amir Goldstein (2):
  nfsd: encode stat->mtime for getattr instead of inode->i_mtime
  nfsd: store stat times in fill_pre_wcc() instead of inode times

 fs/locks.c        |  6 ++----
 fs/nfsd/nfs3xdr.c | 31 ++++++++++++++++++++++++++++++-
 fs/nfsd/nfs4xdr.c |  2 +-
 fs/nfsd/nfsfh.h   | 28 ++++++----------------------
 fs/nfsd/nfsxdr.c  |  1 +
 5 files changed, 40 insertions(+), 28 deletions(-)

-- 
2.7.4

[PATCH v2 1/2] nfsd: encode stat->mtime for getattr instead of inode->i_mtime

[Amir Goldstein]

The values of stat->mtime and inode->i_mtime may differ for overlayfs
and stat->mtime is the correct value to use when encoding getattr.
This is also consistent with the fact that other attr times are also
encoded from stat values.

Both callers of lease_get_mtime() already have the value of stat->mtime,
so the only needed change is that lease_get_mtime() will not overwrite
this value with inode->i_mtime in case the inode does not have an
exclusive lease.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/locks.c       | 6 ++----
 fs/nfsd/nfsxdr.c | 1 +
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 21b4dfa289ee..d6ff4beb70ce 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1554,9 +1554,9 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
 EXPORT_SYMBOL(__break_lease);
 
 /**
- *	lease_get_mtime - get the last modified time of an inode
+ *	lease_get_mtime - update modified time of an inode with exclusive lease
  *	@inode: the inode
- *      @time:  pointer to a timespec which will contain the last modified time
+ *      @time:  pointer to a timespec which contains the last modified time
  *
  * This is to force NFS clients to flush their caches for files with
  * exclusive leases.  The justification is that if someone has an
@@ -1580,8 +1580,6 @@ void lease_get_mtime(struct inode *inode, struct timespec *time)
 
 	if (has_lease)
 		*time = current_time(inode);
-	else
-		*time = inode->i_mtime;
 }
 
 EXPORT_SYMBOL(lease_get_mtime);
diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
index 644a0342f0e0..79b6064f8977 100644
--- a/fs/nfsd/nfsxdr.c
+++ b/fs/nfsd/nfsxdr.c
@@ -188,6 +188,7 @@ encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp,
 	*p++ = htonl((u32) stat->ino);
 	*p++ = htonl((u32) stat->atime.tv_sec);
 	*p++ = htonl(stat->atime.tv_nsec ? stat->atime.tv_nsec / 1000 : 0);
+	time = stat->mtime;
 	lease_get_mtime(d_inode(dentry), &time); 
 	*p++ = htonl((u32) time.tv_sec);
 	*p++ = htonl(time.tv_nsec ? time.tv_nsec / 1000 : 0); 
-- 
2.7.4

[PATCH v2 2/2] nfsd: store stat times in fill_pre_wcc() instead of inode times

[Amir Goldstein]

The time values in stat and inode may differ for overlayfs and stat time
values are the correct ones to use. This is also consistent with the fact
that fill_post_wcc() also stores stat time values.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/nfsd/nfs3xdr.c | 31 ++++++++++++++++++++++++++++++-
 fs/nfsd/nfs4xdr.c |  2 +-
 fs/nfsd/nfsfh.h   | 28 ++++++----------------------
 3 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index 2758480555fa..1a70581e1cb2 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -251,6 +251,34 @@ encode_wcc_data(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
 }
 
 /*
+ * Fill in the pre_op attr for the wcc data
+ */
+void fill_pre_wcc(struct svc_fh *fhp)
+{
+	struct inode    *inode;
+	struct kstat	stat;
+	__be32 err;
+
+	if (fhp->fh_pre_saved)
+		return;
+
+	inode = d_inode(fhp->fh_dentry);
+	err = fh_getattr(fhp, &stat);
+	if (err) {
+		/* Grab the times from inode anyway */
+		stat.mtime = inode->i_mtime;
+		stat.ctime = inode->i_ctime;
+		stat.size  = inode->i_size;
+	}
+
+	fhp->fh_pre_mtime = stat.mtime;
+	fhp->fh_pre_ctime = stat.ctime;
+	fhp->fh_pre_size  = stat.size;
+	fhp->fh_pre_change = nfsd4_change_attribute(&stat, inode);
+	fhp->fh_pre_saved = true;
+}
+
+/*
  * Fill in the post_op attr for the wcc data
  */
 void fill_post_wcc(struct svc_fh *fhp)
@@ -261,7 +289,8 @@ void fill_post_wcc(struct svc_fh *fhp)
 		printk("nfsd: inode locked twice during operation.\n");
 
 	err = fh_getattr(fhp, &fhp->fh_post_attr);
-	fhp->fh_post_change = nfsd4_change_attribute(d_inode(fhp->fh_dentry));
+	fhp->fh_post_change = nfsd4_change_attribute(&fhp->fh_post_attr,
+						     d_inode(fhp->fh_dentry));
 	if (err) {
 		fhp->fh_post_saved = false;
 		/* Grab the ctime anyway - set_change_info might use it */
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 2c61c6b8ae09..fbe11ad9b9ff 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1991,7 +1991,7 @@ static __be32 *encode_change(__be32 *p, struct kstat *stat, struct inode *inode,
 		*p++ = cpu_to_be32(convert_to_wallclock(exp->cd->flush_time));
 		*p++ = 0;
 	} else if (IS_I_VERSION(inode)) {
-		p = xdr_encode_hyper(p, nfsd4_change_attribute(inode));
+		p = xdr_encode_hyper(p, nfsd4_change_attribute(stat, inode));
 	} else {
 		*p++ = cpu_to_be32(stat->ctime.tv_sec);
 		*p++ = cpu_to_be32(stat->ctime.tv_nsec);
diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index 43f31cf49bae..99be87b50ebe 100644
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h
@@ -252,36 +252,20 @@ fh_clear_wcc(struct svc_fh *fhp)
  * By using both ctime and the i_version counter we guarantee that as
  * long as time doesn't go backwards we never reuse an old value.
  */
-static inline u64 nfsd4_change_attribute(struct inode *inode)
+static inline u64 nfsd4_change_attribute(struct kstat *stat,
+					 struct inode *inode)
 {
 	u64 chattr;
 
-	chattr =  inode->i_ctime.tv_sec;
+	chattr =  stat->ctime.tv_sec;
 	chattr <<= 30;
-	chattr += inode->i_ctime.tv_nsec;
+	chattr += stat->ctime.tv_nsec;
 	chattr += inode->i_version;
 	return chattr;
 }
 
-/*
- * Fill in the pre_op attr for the wcc data
- */
-static inline void
-fill_pre_wcc(struct svc_fh *fhp)
-{
-	struct inode    *inode;
-
-	inode = d_inode(fhp->fh_dentry);
-	if (!fhp->fh_pre_saved) {
-		fhp->fh_pre_mtime = inode->i_mtime;
-		fhp->fh_pre_ctime = inode->i_ctime;
-		fhp->fh_pre_size  = inode->i_size;
-		fhp->fh_pre_change = nfsd4_change_attribute(inode);
-		fhp->fh_pre_saved = true;
-	}
-}
-
-extern void fill_post_wcc(struct svc_fh *);
+extern void fill_pre_wcc(struct svc_fh *fhp);
+extern void fill_post_wcc(struct svc_fh *fhp);
 #else
 #define fh_clear_wcc(ignored)
 #define fill_pre_wcc(ignored)
-- 
2.7.4

Re: [PATCH v2 2/2] nfsd: store stat times in fill_pre_wcc() instead of inode times

[Jeff Layton]

On Wed, 2018-01-03 at 17:14 +0200, Amir Goldstein wrote:
> The time values in stat and inode may differ for overlayfs and stat time
> values are the correct ones to use. This is also consistent with the fact
> that fill_post_wcc() also stores stat time values.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/nfsd/nfs3xdr.c | 31 ++++++++++++++++++++++++++++++-
>  fs/nfsd/nfs4xdr.c |  2 +-
>  fs/nfsd/nfsfh.h   | 28 ++++++----------------------
>  3 files changed, 37 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> index 2758480555fa..1a70581e1cb2 100644
> --- a/fs/nfsd/nfs3xdr.c
> +++ b/fs/nfsd/nfs3xdr.c
> @@ -251,6 +251,34 @@ encode_wcc_data(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
>  }
>  
>  /*
> + * Fill in the pre_op attr for the wcc data
> + */
> +void fill_pre_wcc(struct svc_fh *fhp)
> +{
> +	struct inode    *inode;
> +	struct kstat	stat;
> +	__be32 err;
> +
> +	if (fhp->fh_pre_saved)
> +		return;
> +
> +	inode = d_inode(fhp->fh_dentry);
> +	err = fh_getattr(fhp, &stat);
> +	if (err) {
> +		/* Grab the times from inode anyway */
> +		stat.mtime = inode->i_mtime;
> +		stat.ctime = inode->i_ctime;
> +		stat.size  = inode->i_size;
> +	}
> +

Might be best to instead just not supply pre/post op attrs if the
getattr fails? They are technically optional with v3 -- we can just set
the attributes_follow bit to false there.

For v4, it's a little more complicated. Scraping it out of the inode
might be necessary for the cases where we need a change_info4.

Either way, it'd be good to know that these getattrs are failing if this
occurs. Maybe a KERN_WARNING printk or something might be good there?


> +	fhp->fh_pre_mtime = stat.mtime;
> +	fhp->fh_pre_ctime = stat.ctime;
> +	fhp->fh_pre_size  = stat.size;
> +	fhp->fh_pre_change = nfsd4_change_attribute(&stat, inode);
> +	fhp->fh_pre_saved = true;
> +}
> +
> +/*
>   * Fill in the post_op attr for the wcc data
>   */
>  void fill_post_wcc(struct svc_fh *fhp)
> @@ -261,7 +289,8 @@ void fill_post_wcc(struct svc_fh *fhp)
>  		printk("nfsd: inode locked twice during operation.\n");
>  
>  	err = fh_getattr(fhp, &fhp->fh_post_attr);
> -	fhp->fh_post_change = nfsd4_change_attribute(d_inode(fhp->fh_dentry));
> +	fhp->fh_post_change = nfsd4_change_attribute(&fhp->fh_post_attr,
> +						     d_inode(fhp->fh_dentry));
>  	if (err) {
>  		fhp->fh_post_saved = false;
>  		/* Grab the ctime anyway - set_change_info might use it */
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 2c61c6b8ae09..fbe11ad9b9ff 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1991,7 +1991,7 @@ static __be32 *encode_change(__be32 *p, struct kstat *stat, struct inode *inode,
>  		*p++ = cpu_to_be32(convert_to_wallclock(exp->cd->flush_time));
>  		*p++ = 0;
>  	} else if (IS_I_VERSION(inode)) {
> -		p = xdr_encode_hyper(p, nfsd4_change_attribute(inode));
> +		p = xdr_encode_hyper(p, nfsd4_change_attribute(stat, inode));
>  	} else {
>  		*p++ = cpu_to_be32(stat->ctime.tv_sec);
>  		*p++ = cpu_to_be32(stat->ctime.tv_nsec);
> diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
> index 43f31cf49bae..99be87b50ebe 100644
> --- a/fs/nfsd/nfsfh.h
> +++ b/fs/nfsd/nfsfh.h
> @@ -252,36 +252,20 @@ fh_clear_wcc(struct svc_fh *fhp)
>   * By using both ctime and the i_version counter we guarantee that as
>   * long as time doesn't go backwards we never reuse an old value.
>   */
> -static inline u64 nfsd4_change_attribute(struct inode *inode)
> +static inline u64 nfsd4_change_attribute(struct kstat *stat,
> +					 struct inode *inode)
>  {
>  	u64 chattr;
>  
> -	chattr =  inode->i_ctime.tv_sec;
> +	chattr =  stat->ctime.tv_sec;
>  	chattr <<= 30;
> -	chattr += inode->i_ctime.tv_nsec;
> +	chattr += stat->ctime.tv_nsec;
>  	chattr += inode->i_version;
>  	return chattr;
>  }
>  
> -/*
> - * Fill in the pre_op attr for the wcc data
> - */
> -static inline void
> -fill_pre_wcc(struct svc_fh *fhp)
> -{
> -	struct inode    *inode;
> -
> -	inode = d_inode(fhp->fh_dentry);
> -	if (!fhp->fh_pre_saved) {
> -		fhp->fh_pre_mtime = inode->i_mtime;
> -		fhp->fh_pre_ctime = inode->i_ctime;
> -		fhp->fh_pre_size  = inode->i_size;
> -		fhp->fh_pre_change = nfsd4_change_attribute(inode);
> -		fhp->fh_pre_saved = true;
> -	}
> -}
> -
> -extern void fill_post_wcc(struct svc_fh *);
> +extern void fill_pre_wcc(struct svc_fh *fhp);
> +extern void fill_post_wcc(struct svc_fh *fhp);
>  #else
>  #define fh_clear_wcc(ignored)
>  #define fill_pre_wcc(ignored)

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 2/2] nfsd: store stat times in fill_pre_wcc() instead of inode times

[Amir Goldstein]

On Wed, Jan 3, 2018 at 5:41 PM, Jeff Layton <jlayton@kernel.org> wrote:
> On Wed, 2018-01-03 at 17:14 +0200, Amir Goldstein wrote:
>> The time values in stat and inode may differ for overlayfs and stat time
>> values are the correct ones to use. This is also consistent with the fact
>> that fill_post_wcc() also stores stat time values.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  fs/nfsd/nfs3xdr.c | 31 ++++++++++++++++++++++++++++++-
>>  fs/nfsd/nfs4xdr.c |  2 +-
>>  fs/nfsd/nfsfh.h   | 28 ++++++----------------------
>>  3 files changed, 37 insertions(+), 24 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
>> index 2758480555fa..1a70581e1cb2 100644
>> --- a/fs/nfsd/nfs3xdr.c
>> +++ b/fs/nfsd/nfs3xdr.c
>> @@ -251,6 +251,34 @@ encode_wcc_data(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
>>  }
>>
>>  /*
>> + * Fill in the pre_op attr for the wcc data
>> + */
>> +void fill_pre_wcc(struct svc_fh *fhp)
>> +{
>> +     struct inode    *inode;
>> +     struct kstat    stat;
>> +     __be32 err;
>> +
>> +     if (fhp->fh_pre_saved)
>> +             return;
>> +
>> +     inode = d_inode(fhp->fh_dentry);
>> +     err = fh_getattr(fhp, &stat);
>> +     if (err) {
>> +             /* Grab the times from inode anyway */
>> +             stat.mtime = inode->i_mtime;
>> +             stat.ctime = inode->i_ctime;
>> +             stat.size  = inode->i_size;
>> +     }
>> +
>
> Might be best to instead just not supply pre/post op attrs if the
> getattr fails? They are technically optional with v3 -- we can just set
> the attributes_follow bit to false there.

I considered to set fh_pre_saved = false on error just like
fill_post_wcc() does, but wasn't sure of the consequences or how to test
for that matter, so I chose a more delicate fallback instead.


>
> For v4, it's a little more complicated. Scraping it out of the inode
> might be necessary for the cases where we need a change_info4.
>
> Either way, it'd be good to know that these getattrs are failing if this
> occurs. Maybe a KERN_WARNING printk or something might be good there?
>

As you wish, but fill_post_wcc() does not emit a warning, so...

I'll wait for feedback from Bruce as well.
Let me know the error handing of your choice.

Thanks,
Amir.

Re: [PATCH v2 2/2] nfsd: store stat times in fill_pre_wcc() instead of inode times

[Jeff Layton]

On Wed, 2018-01-03 at 17:48 +0200, Amir Goldstein wrote:
> On Wed, Jan 3, 2018 at 5:41 PM, Jeff Layton <jlayton@kernel.org> wrote:
> > On Wed, 2018-01-03 at 17:14 +0200, Amir Goldstein wrote:
> > > The time values in stat and inode may differ for overlayfs and stat time
> > > values are the correct ones to use. This is also consistent with the fact
> > > that fill_post_wcc() also stores stat time values.
> > > 
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > >  fs/nfsd/nfs3xdr.c | 31 ++++++++++++++++++++++++++++++-
> > >  fs/nfsd/nfs4xdr.c |  2 +-
> > >  fs/nfsd/nfsfh.h   | 28 ++++++----------------------
> > >  3 files changed, 37 insertions(+), 24 deletions(-)
> > > 
> > > diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> > > index 2758480555fa..1a70581e1cb2 100644
> > > --- a/fs/nfsd/nfs3xdr.c
> > > +++ b/fs/nfsd/nfs3xdr.c
> > > @@ -251,6 +251,34 @@ encode_wcc_data(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
> > >  }
> > > 
> > >  /*
> > > + * Fill in the pre_op attr for the wcc data
> > > + */
> > > +void fill_pre_wcc(struct svc_fh *fhp)
> > > +{
> > > +     struct inode    *inode;
> > > +     struct kstat    stat;
> > > +     __be32 err;
> > > +
> > > +     if (fhp->fh_pre_saved)
> > > +             return;
> > > +
> > > +     inode = d_inode(fhp->fh_dentry);
> > > +     err = fh_getattr(fhp, &stat);
> > > +     if (err) {
> > > +             /* Grab the times from inode anyway */
> > > +             stat.mtime = inode->i_mtime;
> > > +             stat.ctime = inode->i_ctime;
> > > +             stat.size  = inode->i_size;
> > > +     }
> > > +
> > 
> > Might be best to instead just not supply pre/post op attrs if the
> > getattr fails? They are technically optional with v3 -- we can just set
> > the attributes_follow bit to false there.
> 
> I considered to set fh_pre_saved = false on error just like
> fill_post_wcc() does, but wasn't sure of the consequences or how to test
> for that matter, so I chose a more delicate fallback instead.
> 

Take care with the BUG_ON in set_change_info if you do that. 

Note that all of this is really just to handle weak cache consistency in
v3, and the change_info4 value in v4. When the info is not reliable, the
client doesn't trust its cache, for the most part. Getting it wrong
shouldn't be fatal in most cases.

For v3 you can just clear the attributes_follow bit when you fill out
the info, and leave it zeroed out. I had a patch a few years ago that
did this on a per-export basis that you're welcome to take an run with:

    https://patchwork.kernel.org/patch/7159311/

Obviously, the conditions for doing this here are different.

For v4, I think we can just try to scrape what we have like you're doing
here, and simply ensure that the "atomic" field being encoded in
encode_cinfo is false when this occurs.

> > 
> > For v4, it's a little more complicated. Scraping it out of the inode
> > might be necessary for the cases where we need a change_info4.
> > 
> > Either way, it'd be good to know that these getattrs are failing if this
> > occurs. Maybe a KERN_WARNING printk or something might be good there?
> > 
> 
> As you wish, but fill_post_wcc() does not emit a warning, so...
> 
> I'll wait for feedback from Bruce as well.
> Let me know the error handing of your choice.
> 

Yeah, it doesn't. I think that's good info to know though.

All of this is to support client-server cache coherency, and bugs in
those areas can be subtle and hard to detect. A stack trace is probably
not terribly helpful, but doing a one-time printk to show that you got
an error there may be.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 2/2] nfsd: store stat times in fill_pre_wcc() instead of inode times

[Amir Goldstein]

On Wed, Jan 3, 2018 at 8:45 PM, Jeff Layton <jlayton@kernel.org> wrote:
> On Wed, 2018-01-03 at 17:48 +0200, Amir Goldstein wrote:
>> On Wed, Jan 3, 2018 at 5:41 PM, Jeff Layton <jlayton@kernel.org> wrote:
>> > On Wed, 2018-01-03 at 17:14 +0200, Amir Goldstein wrote:
>> > > The time values in stat and inode may differ for overlayfs and stat time
>> > > values are the correct ones to use. This is also consistent with the fact
>> > > that fill_post_wcc() also stores stat time values.
>> > >
>> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> > > ---
>> > >  fs/nfsd/nfs3xdr.c | 31 ++++++++++++++++++++++++++++++-
>> > >  fs/nfsd/nfs4xdr.c |  2 +-
>> > >  fs/nfsd/nfsfh.h   | 28 ++++++----------------------
>> > >  3 files changed, 37 insertions(+), 24 deletions(-)
>> > >
>> > > diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
>> > > index 2758480555fa..1a70581e1cb2 100644
>> > > --- a/fs/nfsd/nfs3xdr.c
>> > > +++ b/fs/nfsd/nfs3xdr.c
>> > > @@ -251,6 +251,34 @@ encode_wcc_data(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
>> > >  }
>> > >
>> > >  /*
>> > > + * Fill in the pre_op attr for the wcc data
>> > > + */
>> > > +void fill_pre_wcc(struct svc_fh *fhp)
>> > > +{
>> > > +     struct inode    *inode;
>> > > +     struct kstat    stat;
>> > > +     __be32 err;
>> > > +
>> > > +     if (fhp->fh_pre_saved)
>> > > +             return;
>> > > +
>> > > +     inode = d_inode(fhp->fh_dentry);
>> > > +     err = fh_getattr(fhp, &stat);
>> > > +     if (err) {
>> > > +             /* Grab the times from inode anyway */
>> > > +             stat.mtime = inode->i_mtime;
>> > > +             stat.ctime = inode->i_ctime;
>> > > +             stat.size  = inode->i_size;
>> > > +     }
>> > > +
>> >
>> > Might be best to instead just not supply pre/post op attrs if the
>> > getattr fails? They are technically optional with v3 -- we can just set
>> > the attributes_follow bit to false there.
>>
>> I considered to set fh_pre_saved = false on error just like
>> fill_post_wcc() does, but wasn't sure of the consequences or how to test
>> for that matter, so I chose a more delicate fallback instead.
>>
>
> Take care with the BUG_ON in set_change_info if you do that.
>
> Note that all of this is really just to handle weak cache consistency in
> v3, and the change_info4 value in v4. When the info is not reliable, the
> client doesn't trust its cache, for the most part. Getting it wrong
> shouldn't be fatal in most cases.
>
> For v3 you can just clear the attributes_follow bit when you fill out
> the info, and leave it zeroed out. I had a patch a few years ago that
> did this on a per-export basis that you're welcome to take an run with:
>
>     https://patchwork.kernel.org/patch/7159311/
>
> Obviously, the conditions for doing this here are different.
>
> For v4, I think we can just try to scrape what we have like you're doing
> here, and simply ensure that the "atomic" field being encoded in
> encode_cinfo is false when this occurs.
>

Honestly, Jeff, at this point I am so far out into the woods with overlay NFS
export, that I would like to remain focused on correctness and leave
performance for later time.

So if I understand you correctly, patch 2/2 is not needed for correctness?
Meaning that if overlay inode times are not uptodate, nothing fatal will
happen? Or did you mean that I must take care of signalling the client
that time values are not reliable for overlayfs?

If patch 2/2 is indeed not a must, then I would like to ask you to ACK
patch 1/2. It seems quite simple, trivial and harmless to me even  without
diving deep into NFS protocols. I think patch 1/2 should be enough for
first implementation - it certainly is enough to fix the nfstest_posix failures.

Thanks!
Amir.

Re: [PATCH v2 1/2] nfsd: encode stat->mtime for getattr instead of inode->i_mtime

[Jeff Layton]

On Wed, 2018-01-03 at 17:14 +0200, Amir Goldstein wrote:
> The values of stat->mtime and inode->i_mtime may differ for overlayfs
> and stat->mtime is the correct value to use when encoding getattr.
> This is also consistent with the fact that other attr times are also
> encoded from stat values.
> 
> Both callers of lease_get_mtime() already have the value of stat->mtime,
> so the only needed change is that lease_get_mtime() will not overwrite
> this value with inode->i_mtime in case the inode does not have an
> exclusive lease.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/locks.c       | 6 ++----
>  fs/nfsd/nfsxdr.c | 1 +
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index 21b4dfa289ee..d6ff4beb70ce 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -1554,9 +1554,9 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
>  EXPORT_SYMBOL(__break_lease);
>  
>  /**
> - *	lease_get_mtime - get the last modified time of an inode
> + *	lease_get_mtime - update modified time of an inode with exclusive lease
>   *	@inode: the inode
> - *      @time:  pointer to a timespec which will contain the last modified time
> + *      @time:  pointer to a timespec which contains the last modified time
>   *
>   * This is to force NFS clients to flush their caches for files with
>   * exclusive leases.  The justification is that if someone has an
> @@ -1580,8 +1580,6 @@ void lease_get_mtime(struct inode *inode, struct timespec *time)
>  
>  	if (has_lease)
>  		*time = current_time(inode);
> -	else
> -		*time = inode->i_mtime;
>  }
>  
>  EXPORT_SYMBOL(lease_get_mtime);
> diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
> index 644a0342f0e0..79b6064f8977 100644
> --- a/fs/nfsd/nfsxdr.c
> +++ b/fs/nfsd/nfsxdr.c
> @@ -188,6 +188,7 @@ encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp,
>  	*p++ = htonl((u32) stat->ino);
>  	*p++ = htonl((u32) stat->atime.tv_sec);
>  	*p++ = htonl(stat->atime.tv_nsec ? stat->atime.tv_nsec / 1000 : 0);
> +	time = stat->mtime;
>  	lease_get_mtime(d_inode(dentry), &time); 
>  	*p++ = htonl((u32) time.tv_sec);
>  	*p++ = htonl(time.tv_nsec ? time.tv_nsec / 1000 : 0); 

This looks like a clear improvement. AFAICT, in all cases we pass
lease_get_mtime a value from a previous getattr operation. We should
prefer that one to inode->i_mtime.

The lease_get_mtime prototype is now rather awkward though. It might be
nice to clean that up, eventually, but that can be done later.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 2/2] nfsd: store stat times in fill_pre_wcc() instead of inode times

[Jeff Layton]

On Wed, 2018-01-03 at 23:03 +0200, Amir Goldstein wrote:
> On Wed, Jan 3, 2018 at 8:45 PM, Jeff Layton <jlayton@kernel.org> wrote:
> > On Wed, 2018-01-03 at 17:48 +0200, Amir Goldstein wrote:
> > > On Wed, Jan 3, 2018 at 5:41 PM, Jeff Layton <jlayton@kernel.org> wrote:
> > > > On Wed, 2018-01-03 at 17:14 +0200, Amir Goldstein wrote:
> > > > > The time values in stat and inode may differ for overlayfs and stat time
> > > > > values are the correct ones to use. This is also consistent with the fact
> > > > > that fill_post_wcc() also stores stat time values.
> > > > > 
> > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > > ---
> > > > >  fs/nfsd/nfs3xdr.c | 31 ++++++++++++++++++++++++++++++-
> > > > >  fs/nfsd/nfs4xdr.c |  2 +-
> > > > >  fs/nfsd/nfsfh.h   | 28 ++++++----------------------
> > > > >  3 files changed, 37 insertions(+), 24 deletions(-)
> > > > > 
> > > > > diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> > > > > index 2758480555fa..1a70581e1cb2 100644
> > > > > --- a/fs/nfsd/nfs3xdr.c
> > > > > +++ b/fs/nfsd/nfs3xdr.c
> > > > > @@ -251,6 +251,34 @@ encode_wcc_data(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
> > > > >  }
> > > > > 
> > > > >  /*
> > > > > + * Fill in the pre_op attr for the wcc data
> > > > > + */
> > > > > +void fill_pre_wcc(struct svc_fh *fhp)
> > > > > +{
> > > > > +     struct inode    *inode;
> > > > > +     struct kstat    stat;
> > > > > +     __be32 err;
> > > > > +
> > > > > +     if (fhp->fh_pre_saved)
> > > > > +             return;
> > > > > +
> > > > > +     inode = d_inode(fhp->fh_dentry);
> > > > > +     err = fh_getattr(fhp, &stat);
> > > > > +     if (err) {
> > > > > +             /* Grab the times from inode anyway */
> > > > > +             stat.mtime = inode->i_mtime;
> > > > > +             stat.ctime = inode->i_ctime;
> > > > > +             stat.size  = inode->i_size;
> > > > > +     }
> > > > > +
> > > > 
> > > > Might be best to instead just not supply pre/post op attrs if the
> > > > getattr fails? They are technically optional with v3 -- we can just set
> > > > the attributes_follow bit to false there.
> > > 
> > > I considered to set fh_pre_saved = false on error just like
> > > fill_post_wcc() does, but wasn't sure of the consequences or how to test
> > > for that matter, so I chose a more delicate fallback instead.
> > > 
> > 
> > Take care with the BUG_ON in set_change_info if you do that.
> > 
> > Note that all of this is really just to handle weak cache consistency in
> > v3, and the change_info4 value in v4. When the info is not reliable, the
> > client doesn't trust its cache, for the most part. Getting it wrong
> > shouldn't be fatal in most cases.
> > 
> > For v3 you can just clear the attributes_follow bit when you fill out
> > the info, and leave it zeroed out. I had a patch a few years ago that
> > did this on a per-export basis that you're welcome to take an run with:
> > 
> >     https://patchwork.kernel.org/patch/7159311/
> > 
> > Obviously, the conditions for doing this here are different.
> > 
> > For v4, I think we can just try to scrape what we have like you're doing
> > here, and simply ensure that the "atomic" field being encoded in
> > encode_cinfo is false when this occurs.
> > 
> 
> Honestly, Jeff, at this point I am so far out into the woods with overlay NFS
> export, that I would like to remain focused on correctness and leave
> performance for later time.
> 

:)

> So if I understand you correctly, patch 2/2 is not needed for correctness?
> Meaning that if overlay inode times are not uptodate, nothing fatal will
> happen? Or did you mean that I must take care of signalling the client
> that time values are not reliable for overlayfs?
> 
> If patch 2/2 is indeed not a must, then I would like to ask you to ACK
> patch 1/2. It seems quite simple, trivial and harmless to me even  without
> diving deep into NFS protocols. I think patch 1/2 should be enough for
> first implementation - it certainly is enough to fix the nfstest_posix failures.
> 
> Thanks!
> Amir.

Patch #1 looks fine. I think we ought to wait on #2.

We really should be doing getattrs like this, but when that fails we
should probably just zero out the wcc / change_info4 at the end rather
than pretending that it's valid.

I think Bruce or I can take care of that bit after patch #1 is merged.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 2/2] nfsd: store stat times in fill_pre_wcc() instead of inode times

[Amir Goldstein]

On Thu, Jan 4, 2018 at 3:26 PM, Jeff Layton <jlayton@kernel.org> wrote:
> On Wed, 2018-01-03 at 23:03 +0200, Amir Goldstein wrote:
[...]

>
>> So if I understand you correctly, patch 2/2 is not needed for correctness?
>> Meaning that if overlay inode times are not uptodate, nothing fatal will
>> happen? Or did you mean that I must take care of signalling the client
>> that time values are not reliable for overlayfs?
>>
>> If patch 2/2 is indeed not a must, then I would like to ask you to ACK
>> patch 1/2. It seems quite simple, trivial and harmless to me even  without
>> diving deep into NFS protocols. I think patch 1/2 should be enough for
>> first implementation - it certainly is enough to fix the nfstest_posix failures.
>>
>> Thanks!
>> Amir.
>
> Patch #1 looks fine. I think we ought to wait on #2.
>
> We really should be doing getattrs like this, but when that fails we
> should probably just zero out the wcc / change_info4 at the end rather
> than pretending that it's valid.
>
> I think Bruce or I can take care of that bit after patch #1 is merged.

Great!

Then I'll stack patch #1 along with my overlay nfs export series
with your ACK.

Thanks!
Amir.

Re: [PATCH v2 2/2] nfsd: store stat times in fill_pre_wcc() instead of inode times

[J . Bruce Fields]

On Thu, Jan 04, 2018 at 08:26:43AM -0500, Jeff Layton wrote:
> On Wed, 2018-01-03 at 23:03 +0200, Amir Goldstein wrote:
> > On Wed, Jan 3, 2018 at 8:45 PM, Jeff Layton <jlayton@kernel.org> wrote:
> > > On Wed, 2018-01-03 at 17:48 +0200, Amir Goldstein wrote:
> > > > On Wed, Jan 3, 2018 at 5:41 PM, Jeff Layton <jlayton@kernel.org> wrote:
> > > > > On Wed, 2018-01-03 at 17:14 +0200, Amir Goldstein wrote:
> > > > > > The time values in stat and inode may differ for overlayfs and stat time
> > > > > > values are the correct ones to use. This is also consistent with the fact
> > > > > > that fill_post_wcc() also stores stat time values.
> > > > > > 
> > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > > > ---
> > > > > >  fs/nfsd/nfs3xdr.c | 31 ++++++++++++++++++++++++++++++-
> > > > > >  fs/nfsd/nfs4xdr.c |  2 +-
> > > > > >  fs/nfsd/nfsfh.h   | 28 ++++++----------------------
> > > > > >  3 files changed, 37 insertions(+), 24 deletions(-)
> > > > > > 
> > > > > > diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> > > > > > index 2758480555fa..1a70581e1cb2 100644
> > > > > > --- a/fs/nfsd/nfs3xdr.c
> > > > > > +++ b/fs/nfsd/nfs3xdr.c
> > > > > > @@ -251,6 +251,34 @@ encode_wcc_data(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
> > > > > >  }
> > > > > > 
> > > > > >  /*
> > > > > > + * Fill in the pre_op attr for the wcc data
> > > > > > + */
> > > > > > +void fill_pre_wcc(struct svc_fh *fhp)
> > > > > > +{
> > > > > > +     struct inode    *inode;
> > > > > > +     struct kstat    stat;
> > > > > > +     __be32 err;
> > > > > > +
> > > > > > +     if (fhp->fh_pre_saved)
> > > > > > +             return;
> > > > > > +
> > > > > > +     inode = d_inode(fhp->fh_dentry);
> > > > > > +     err = fh_getattr(fhp, &stat);
> > > > > > +     if (err) {
> > > > > > +             /* Grab the times from inode anyway */
> > > > > > +             stat.mtime = inode->i_mtime;
> > > > > > +             stat.ctime = inode->i_ctime;
> > > > > > +             stat.size  = inode->i_size;
> > > > > > +     }
> > > > > > +
> > > > > 
> > > > > Might be best to instead just not supply pre/post op attrs if the
> > > > > getattr fails? They are technically optional with v3 -- we can just set
> > > > > the attributes_follow bit to false there.
> > > > 
> > > > I considered to set fh_pre_saved = false on error just like
> > > > fill_post_wcc() does, but wasn't sure of the consequences or how to test
> > > > for that matter, so I chose a more delicate fallback instead.
> > > > 
> > > 
> > > Take care with the BUG_ON in set_change_info if you do that.
> > > 
> > > Note that all of this is really just to handle weak cache consistency in
> > > v3, and the change_info4 value in v4. When the info is not reliable, the
> > > client doesn't trust its cache, for the most part. Getting it wrong
> > > shouldn't be fatal in most cases.
> > > 
> > > For v3 you can just clear the attributes_follow bit when you fill out
> > > the info, and leave it zeroed out. I had a patch a few years ago that
> > > did this on a per-export basis that you're welcome to take an run with:
> > > 
> > >     https://patchwork.kernel.org/patch/7159311/
> > > 
> > > Obviously, the conditions for doing this here are different.
> > > 
> > > For v4, I think we can just try to scrape what we have like you're doing
> > > here, and simply ensure that the "atomic" field being encoded in
> > > encode_cinfo is false when this occurs.
> > > 
> > 
> > Honestly, Jeff, at this point I am so far out into the woods with overlay NFS
> > export, that I would like to remain focused on correctness and leave
> > performance for later time.
> > 
> 
> :)
> 
> > So if I understand you correctly, patch 2/2 is not needed for correctness?
> > Meaning that if overlay inode times are not uptodate, nothing fatal will
> > happen? Or did you mean that I must take care of signalling the client
> > that time values are not reliable for overlayfs?
> > 
> > If patch 2/2 is indeed not a must, then I would like to ask you to ACK
> > patch 1/2. It seems quite simple, trivial and harmless to me even  without
> > diving deep into NFS protocols. I think patch 1/2 should be enough for
> > first implementation - it certainly is enough to fix the nfstest_posix failures.
> > 
> > Thanks!
> > Amir.
> 
> Patch #1 looks fine. I think we ought to wait on #2.
> 
> We really should be doing getattrs like this, but when that fails we
> should probably just zero out the wcc / change_info4 at the end rather
> than pretending that it's valid.

I can believe that, but that's what the current behavior is, so I don't
think this patch needs to (or should) make that change too.  I'd rather
fix the one overlayfs problem (as this patch does) and then handle the
change in behavior in the stat failure case separately.

(Though maybe a comment there to remind future selves of this dicussion
would be helpful.)

--b.

Re: [PATCH v2 2/2] nfsd: store stat times in fill_pre_wcc() instead of inode times

[Amir Goldstein]

On Fri, Jan 5, 2018 at 1:05 AM, J . Bruce Fields <bfields@fieldses.org> wrote:
> On Thu, Jan 04, 2018 at 08:26:43AM -0500, Jeff Layton wrote:
>> On Wed, 2018-01-03 at 23:03 +0200, Amir Goldstein wrote:
>> > On Wed, Jan 3, 2018 at 8:45 PM, Jeff Layton <jlayton@kernel.org> wrote:
>> > > On Wed, 2018-01-03 at 17:48 +0200, Amir Goldstein wrote:
>> > > > On Wed, Jan 3, 2018 at 5:41 PM, Jeff Layton <jlayton@kernel.org> wrote:
>> > > > > On Wed, 2018-01-03 at 17:14 +0200, Amir Goldstein wrote:
>> > > > > > The time values in stat and inode may differ for overlayfs and stat time
>> > > > > > values are the correct ones to use. This is also consistent with the fact
>> > > > > > that fill_post_wcc() also stores stat time values.
>> > > > > >
>> > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> > > > > > ---
>> > > > > >  fs/nfsd/nfs3xdr.c | 31 ++++++++++++++++++++++++++++++-
>> > > > > >  fs/nfsd/nfs4xdr.c |  2 +-
>> > > > > >  fs/nfsd/nfsfh.h   | 28 ++++++----------------------
>> > > > > >  3 files changed, 37 insertions(+), 24 deletions(-)
>> > > > > >
>> > > > > > diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
>> > > > > > index 2758480555fa..1a70581e1cb2 100644
>> > > > > > --- a/fs/nfsd/nfs3xdr.c
>> > > > > > +++ b/fs/nfsd/nfs3xdr.c
>> > > > > > @@ -251,6 +251,34 @@ encode_wcc_data(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
>> > > > > >  }
>> > > > > >
>> > > > > >  /*
>> > > > > > + * Fill in the pre_op attr for the wcc data
>> > > > > > + */
>> > > > > > +void fill_pre_wcc(struct svc_fh *fhp)
>> > > > > > +{
>> > > > > > +     struct inode    *inode;
>> > > > > > +     struct kstat    stat;
>> > > > > > +     __be32 err;
>> > > > > > +
>> > > > > > +     if (fhp->fh_pre_saved)
>> > > > > > +             return;
>> > > > > > +
>> > > > > > +     inode = d_inode(fhp->fh_dentry);
>> > > > > > +     err = fh_getattr(fhp, &stat);
>> > > > > > +     if (err) {
>> > > > > > +             /* Grab the times from inode anyway */
>> > > > > > +             stat.mtime = inode->i_mtime;
>> > > > > > +             stat.ctime = inode->i_ctime;
>> > > > > > +             stat.size  = inode->i_size;
>> > > > > > +     }
>> > > > > > +
>> > > > >
>> > > > > Might be best to instead just not supply pre/post op attrs if the
>> > > > > getattr fails? They are technically optional with v3 -- we can just set
>> > > > > the attributes_follow bit to false there.
>> > > >
>> > > > I considered to set fh_pre_saved = false on error just like
>> > > > fill_post_wcc() does, but wasn't sure of the consequences or how to test
>> > > > for that matter, so I chose a more delicate fallback instead.
>> > > >
>> > >
>> > > Take care with the BUG_ON in set_change_info if you do that.
>> > >
>> > > Note that all of this is really just to handle weak cache consistency in
>> > > v3, and the change_info4 value in v4. When the info is not reliable, the
>> > > client doesn't trust its cache, for the most part. Getting it wrong
>> > > shouldn't be fatal in most cases.
>> > >
>> > > For v3 you can just clear the attributes_follow bit when you fill out
>> > > the info, and leave it zeroed out. I had a patch a few years ago that
>> > > did this on a per-export basis that you're welcome to take an run with:
>> > >
>> > >     https://patchwork.kernel.org/patch/7159311/
>> > >
>> > > Obviously, the conditions for doing this here are different.
>> > >
>> > > For v4, I think we can just try to scrape what we have like you're doing
>> > > here, and simply ensure that the "atomic" field being encoded in
>> > > encode_cinfo is false when this occurs.
>> > >
>> >
>> > Honestly, Jeff, at this point I am so far out into the woods with overlay NFS
>> > export, that I would like to remain focused on correctness and leave
>> > performance for later time.
>> >
>>
>> :)
>>
>> > So if I understand you correctly, patch 2/2 is not needed for correctness?
>> > Meaning that if overlay inode times are not uptodate, nothing fatal will
>> > happen? Or did you mean that I must take care of signalling the client
>> > that time values are not reliable for overlayfs?
>> >
>> > If patch 2/2 is indeed not a must, then I would like to ask you to ACK
>> > patch 1/2. It seems quite simple, trivial and harmless to me even  without
>> > diving deep into NFS protocols. I think patch 1/2 should be enough for
>> > first implementation - it certainly is enough to fix the nfstest_posix failures.
>> >
>> > Thanks!
>> > Amir.
>>
>> Patch #1 looks fine. I think we ought to wait on #2.
>>
>> We really should be doing getattrs like this, but when that fails we
>> should probably just zero out the wcc / change_info4 at the end rather
>> than pretending that it's valid.
>
> I can believe that, but that's what the current behavior is, so I don't
> think this patch needs to (or should) make that change too.  I'd rather
> fix the one overlayfs problem (as this patch does) and then handle the
> change in behavior in the stat failure case separately.
>
> (Though maybe a comment there to remind future selves of this dicussion
> would be helpful.)
>

So can I assume you will be taking both patches through your tree?
and add said comment to your future self?
Since patches are independent of overlayfs work, it does not matter if they
get merged before or after overlayfs work, just please let us know what you
intend to do, because right now I posted patch 1/2 at the end of the
overlayfs series for Miklos to take.

Which reminds me... I never got any feedback from tmpfs/vfs folks about the
tmpfs patch: https://patchwork.kernel.org/patch/10055177/
but IIRC, both you and Jeff where in favor of the change.
Do you feel confident enough about this patch to carry it though your tree?
Or should I nudge someone else about it?
The test for decoding a file handle of an unlinked file has already been
merged to xfstest generic/467 and the test is failing with tmpfs without this
change.

Thanks!
Amir.

Re: [PATCH v2 2/2] nfsd: store stat times in fill_pre_wcc() instead of inode times

[J . Bruce Fields]

On Fri, Jan 05, 2018 at 04:45:46PM +0200, Amir Goldstein wrote:
> So can I assume you will be taking both patches through your tree?
> and add said comment to your future self?

I'm OK with that (if Jeff is).

> Since patches are independent of overlayfs work, it does not matter if they
> get merged before or after overlayfs work, just please let us know what you
> intend to do, because right now I posted patch 1/2 at the end of the
> overlayfs series for Miklos to take.

I'd be fine with that or with Miklos taking either or both (feel free to
add an Acked-by: tag for me if you take that route).

But I'll assume I'm taking them unless I hear otherwise.

> Which reminds me... I never got any feedback from tmpfs/vfs folks about the
> tmpfs patch: https://patchwork.kernel.org/patch/10055177/
> but IIRC, both you and Jeff where in favor of the change.
> Do you feel confident enough about this patch to carry it though your tree?

No, I'd recommend resending.  Looks like it should go to Hugh Dickins
<hughd@google.com> and linux-mm@kvack.org ?

--b.

> Or should I nudge someone else about it?
> The test for decoding a file handle of an unlinked file has already been
> merged to xfstest generic/467 and the test is failing with tmpfs without this
> change.
> 
> Thanks!
> Amir.

Re: [PATCH v2 2/2] nfsd: store stat times in fill_pre_wcc() instead of inode times

[J . Bruce Fields]

On Fri, Jan 05, 2018 at 10:30:18AM -0500, J . Bruce Fields wrote:
> On Fri, Jan 05, 2018 at 04:45:46PM +0200, Amir Goldstein wrote:
> > So can I assume you will be taking both patches through your tree?
> > and add said comment to your future self?
> 
> I'm OK with that (if Jeff is).
> 
> > Since patches are independent of overlayfs work, it does not matter if they
> > get merged before or after overlayfs work, just please let us know what you
> > intend to do, because right now I posted patch 1/2 at the end of the
> > overlayfs series for Miklos to take.
> 
> I'd be fine with that or with Miklos taking either or both (feel free to
> add an Acked-by: tag for me if you take that route).
> 
> But I'll assume I'm taking them unless I hear otherwise.

By the way, I just got back to this: committed to nfsd-next at
git://linux-nfs.org/~bfields/linux.git, with just one extra paragraph on
#2's changelog.

--b.