All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Christoph Hellwig <hch@lst.de>,
	"J. Bruce Fields" <bfields@fieldses.org>,
	Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 2/2] nfsd: implement chage_attr_type attribute
Date: Fri, 14 Nov 2014 15:35:05 +1100	[thread overview]
Message-ID: <20141114043505.GA29950@dastard> (raw)
In-Reply-To: <CAHQdGtTzs+K1dUo=9W9sp45-qGxFp5pDwXJgdHY=6UV+Myr68Q@mail.gmail.com>

On Thu, Nov 13, 2014 at 07:43:33PM -0500, Trond Myklebust wrote:
> On Thu, Nov 13, 2014 at 6:54 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Thu, Nov 13, 2014 at 08:02:43AM -0500, Trond Myklebust wrote:
> >> On Wed, Nov 12, 2014 at 7:28 PM, Dave Chinner <david@fromorbit.com> wrote:
> >> > On Wed, Nov 12, 2014 at 09:26:16AM -0500, Trond Myklebust wrote:
> >> >> On Wed, Nov 12, 2014 at 5:24 AM, Christoph Hellwig <hch@lst.de> wrote:
> >> >> > On Wed, Nov 12, 2014 at 09:27:10AM +1100, Dave Chinner wrote:
> >> >> >> To clarify what Christoph wrote, XFS updates i_version is updated
> >> >> >> once per transaction that modifies the inode. So if a VFS level
> >> >> >> operation results in multiple transactions then each transaction
> >> >> >> will but the version.
> >> >> >>
> >> >> >> It was implemented that way because nobody could tell me what the
> >> >> >> actual granularity requirement for change detection was.  Hence what
> >> >> >> I implemented was "be able to detect any persistent change that is
> >> >> >> made" to cover all bases.
> >
> > FWIW, ext4 takes the same approach.  See Ted's post today:
> >
> > http://www.spinics.net/lists/linux-ext4/msg46194.html
> >
> > "The inode_inc_iversion() in mark4_ext4_iloc_dirty() is probably not
> > necessary, since we already should be incrementing i_version whenever
> > ctime and mtime gets updated.  The inode_inc_iversion() there is more
> > of a "belt and suspenders" safety thing, on the theory that the extra
> > bump in i_version won't hurt anything."
> >
> 
> It will hurt if it causes all the NFS clients to blow their caches
> unnecessarily.

Not my problem. We've just implemented what we were asked to
implement.

> Who asked for this?

The only discussion where actual specifications were enumerated was
during a thread about using i_version in the integrity measurement
code (IMA subsystem). The NFSv4 requirements for the change counter
were expressed here:

https://lkml.org/lkml/2012/1/5/408

Don't blame us for implementing the vague "changes every time"
requirements in a way that results in no chance of a persistent
change to either data or metadata being missed by the filesystem.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2014-11-14  4:35 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-08 12:11 nfsd: add support for the chage_attr_type attribute Christoph Hellwig
2014-11-08 12:11 ` [PATCH 1/2] nfsd: correctly define v4.2 support attributes Christoph Hellwig
2014-11-10 22:21   ` J. Bruce Fields
2014-11-11 10:22     ` Christoph Hellwig
2014-11-08 12:11 ` [PATCH 2/2] nfsd: implement chage_attr_type attribute Christoph Hellwig
2014-11-10 17:54   ` J. Bruce Fields
2014-11-11 10:27     ` Christoph Hellwig
2014-11-11 12:36       ` Trond Myklebust
2014-11-11 16:27         ` Christoph Hellwig
2014-11-11 22:27           ` Dave Chinner
2014-11-12 10:24             ` Christoph Hellwig
2014-11-12 14:26               ` Trond Myklebust
2014-11-13  0:28                 ` Dave Chinner
2014-11-13 13:02                   ` Trond Myklebust
2014-11-13 21:47                     ` Dave Chinner
2014-11-13 23:54                     ` Dave Chinner
2014-11-14  0:43                       ` Trond Myklebust
2014-11-14  4:35                         ` Dave Chinner [this message]
2014-11-14 14:22                           ` Trond Myklebust
2014-11-15  1:24                             ` Dave Chinner
2014-11-11 21:42       ` J. Bruce Fields
2014-12-01 19:50         ` J. Bruce Fields
2014-12-02 17:23           ` Christoph Hellwig
2014-11-10 21:42 ` nfsd: add support for the " J. Bruce Fields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141114043505.GA29950@dastard \
    --to=david@fromorbit.com \
    --cc=bfields@fieldses.org \
    --cc=hch@lst.de \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@primarydata.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.