Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1605152 > unrolled thread

Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization bfields@fieldses.org (J. Bruce Fields) - 2017-03-20 22:50 +0100
    Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization Christoph Hellwig <hch@infradead.org> - 2017-03-21 14:50 +0100
      Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization "J. Bruce Fields" <bfields@fieldses.org> - 2017-03-21 17:40 +0100
        Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and  optimization Jeff Layton <jlayton@redhat.com> - 2017-03-21 18:30 +0100
          Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization "J. Bruce Fields" <bfields@fieldses.org> - 2017-03-21 18:40 +0100
            Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization "J. Bruce Fields" <bfields@fieldses.org> - 2017-03-21 19:00 +0100
          Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and  optimization Jeff Layton <jlayton@redhat.com> - 2017-03-21 19:50 +0100
            Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization "J. Bruce Fields" <bfields@fieldses.org> - 2017-03-21 20:20 +0100
              Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and  optimization Jeff Layton <jlayton@redhat.com> - 2017-03-21 23:10 +0100
          Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization "J. Bruce Fields" <bfields@fieldses.org> - 2017-03-21 19:50 +0100
          Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization Dave Chinner <david@fromorbit.com> - 2017-03-21 22:50 +0100
            Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and  optimization Jeff Layton <jlayton@redhat.com> - 2017-03-22 21:00 +0100

#1605152 — Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization

On Thu, Dec 22, 2016 at 09:42:04AM -0500, Jeff Layton wrote:
> On Thu, 2016-12-22 at 00:45 -0800, Christoph Hellwig wrote:
> > On Wed, Dec 21, 2016 at 12:03:17PM -0500, Jeff Layton wrote:
> > > 
> > > Only btrfs, ext4, and xfs implement it for data changes. Because of
> > > this, these filesystems must log the inode to disk whenever the
> > > i_version counter changes. That has a non-zero performance impact,
> > > especially on write-heavy workloads, because we end up dirtying the
> > > inode metadata on every write, not just when the times change. [1]
> > 
> > Do you have numbers to justify these changes?
> 
> I have numbers. As to whether they justify the changes, I'm not sure.
> This helps a lot on a (admittedly nonsensical) 1-byte write workload. On
> XFS, with this fio jobfile:

To me, the interesting question is whether this allows us to turn on
i_version updates by default on xfs and ext4.

When Josef looked at doing that previously he withdrew the patch due to
performance regressions.  I think the most useful thread started here:

	http://lkml.kernel.org/r/1337092396-3272-1-git-send-email-josef@redhat.com

Skimming quickly....  I think the regression was also in the small-write
case.  So apparently that was thought to reveal a real problem?

So if you've mostly eliminated that regression, then that's good
motivation for your patches.  (Though I think in addition to comparing
the patched and unpatched i_version case, we need to compare to the
unpatched not-i_version case.  I'm not clear whether you did that.)

--b.

> 
> --------------------8<------------------
> [global]
> direct=0
> size=2g
> filesize=512m
> bsrange=1-1
> timeout=60
> numjobs=1
> directory=/mnt/scratch
> 
> [f1]
> filename=randwrite
> rw=randwrite
> --------------------8<------------------
> 
> Unpatched kernel:
>   WRITE: io=7707KB, aggrb=128KB/s, minb=128KB/s, maxb=128KB/s, mint=60000msec, maxt=60000msec
> 
> Patched kernel:
>   WRITE: io=12701KB, aggrb=211KB/s, minb=211KB/s, maxb=211KB/s, mint=60000msec, maxt=60000msec
> 
> So quite a difference there and it's pretty consistent across runs. If I
> change the jobfile to have "direct=1" and "bsrange=4k-4k", then any
> variation between the two doesn't seem to be significant (numbers vary
> as much between runs on the same kernels and are roughly the same).
> 
> Playing with buffered I/O sizes between 1 byte and 4k shows that as the
> I/O sizes get larger, this makes less difference (which is what I'd
> expect).
> 
> Previous testing with ext4 shows roughly the same results. btrfs shows
> some benefit here but significantly less than with ext4 or xfs. Not sure
> why that is yet -- maybe CoW effects?
> 
> That said, I don't have a great test rig for this. I'm using VMs with a
> dedicated LVM volume that's on a random SSD I had laying around. It
> could use testing on a wider set of configurations and workloads.
> 
> I was also hoping that others may have workloads that they think might
> be (postively or negatively) affected by these changes. If you can think
> of any in particular, then I'm interested to hear about them.
> 
> -- 
> Jeff Layton <jlayton@redhat.com>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [next] | [standalone]

#1605607

On Mon, Mar 20, 2017 at 05:43:27PM -0400, J. Bruce Fields wrote:
> To me, the interesting question is whether this allows us to turn on
> i_version updates by default on xfs and ext4.

XFS v5 file systems have it on by default.  Although we'll still need
to agree on the exact semantics of i_version before it's going to be
useful.

[toc] | [prev] | [next] | [standalone]

#1605789

On Tue, Mar 21, 2017 at 06:45:00AM -0700, Christoph Hellwig wrote:
> On Mon, Mar 20, 2017 at 05:43:27PM -0400, J. Bruce Fields wrote:
> > To me, the interesting question is whether this allows us to turn on
> > i_version updates by default on xfs and ext4.
> 
> XFS v5 file systems have it on by default.

Great, thanks.

> Although we'll still need to agree on the exact semantics of i_version
> before it's going to be useful.

Once it's figured out maybe we should write it up for a manpage that
could be used if statx starts exposing it to userspace.

A first attempt:

- It's a u64.

- It works for regular files and directories.  (What about symlinks or
  other special types?)

- It changes between two checks if and only if there were intervening
  data or metadata changes.  The change will always be an increase, but
  the amount of the increase is meaningless.
	- NFS doesn't actually require that it increases, but I think it
	  should.  I assume 64 bits means we don't need a discussion of
	  wraparound.
	- AFS wants an actual counter: if you get i_version X, then
	  write twice, then get i_version X+2, you're allowed to assume
	  your writes were the only modifications.  Let's ignore this
	  for now.  In the future if someone explains how to count
	  operations, then we can extend the interface to tell the
	  caller it can get those extra semantics.

- It's durable; the above comparison still works if there were reboots
  between the two i_version checks.
	- I don't know how realistic this is--we may need to figure out
	  if there's a weaker guarantee that's still useful.  Do
	  filesystems actually make ctime/mtime/i_version changes
	  atomically with the changes that caused them?  What if a
	  change attribute is exposed to an NFS client but doesn't make
	  it to disk, and then that value is reused after reboot?

Am I missing any issues?

--b.

[toc] | [prev] | [next] | [standalone]

#1605842 — Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization

On Tue, 2017-03-21 at 12:30 -0400, J. Bruce Fields wrote:
> On Tue, Mar 21, 2017 at 06:45:00AM -0700, Christoph Hellwig wrote:
> > On Mon, Mar 20, 2017 at 05:43:27PM -0400, J. Bruce Fields wrote:
> > > To me, the interesting question is whether this allows us to turn on
> > > i_version updates by default on xfs and ext4.
> > 
> > XFS v5 file systems have it on by default.
> 
> Great, thanks.
> 
> > Although we'll still need to agree on the exact semantics of i_version
> > before it's going to be useful.
> 
> Once it's figured out maybe we should write it up for a manpage that
> could be used if statx starts exposing it to userspace.
> 
> A first attempt:
> 
> - It's a u64.
> 
> - It works for regular files and directories.  (What about symlinks or
>   other special types?)
> 
> - It changes between two checks if and only if there were intervening
>   data or metadata changes.  The change will always be an increase, but
>   the amount of the increase is meaningless.
> 	- NFS doesn't actually require that it increases, but I think it
> 	  should.  I assume 64 bits means we don't need a discussion of
> 	  wraparound.

I thought NFS spec required that you be able to recognize old change
attributes so that they can be discarded. I could be wrong here though.
I'd have to go back and look through the spec to be sure.

> 	- AFS wants an actual counter: if you get i_version X, then
> 	  write twice, then get i_version X+2, you're allowed to assume
> 	  your writes were the only modifications.  Let's ignore this
> 	  for now.  In the future if someone explains how to count
> 	  operations, then we can extend the interface to tell the
> 	  caller it can get those extra semantics.
> 
> - It's durable; the above comparison still works if there were reboots
>   between the two i_version checks.
> 	- I don't know how realistic this is--we may need to figure out
> 	  if there's a weaker guarantee that's still useful.  Do
> 	  filesystems actually make ctime/mtime/i_version changes
> 	  atomically with the changes that caused them?  What if a
> 	  change attribute is exposed to an NFS client but doesn't make
> 	  it to disk, and then that value is reused after reboot?
> 

Yeah, there could be atomicity there. If we bump i_version, we'll mark
the inode dirty and I think that will end up with the new i_version at
least being journalled before __mark_inode_dirty returns.

That said, I suppose it is possible for us to bump the counter, hand
that new counter value out to a NFS client and then the box crashes
before it makes it to the journal.

Not sure how big a problem that really is.

> Am I missing any issues?
> 

No, I think you have it covered, and that's pretty much exactly what I
had in mind as far as semantics go. Thanks for writing it up!

-- 
Jeff Layton <jlayton@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1605853

On Tue, Mar 21, 2017 at 01:23:24PM -0400, Jeff Layton wrote:
> On Tue, 2017-03-21 at 12:30 -0400, J. Bruce Fields wrote:
> > 	- NFS doesn't actually require that it increases, but I think it
> > 	  should.  I assume 64 bits means we don't need a discussion of
> > 	  wraparound.
> 
> I thought NFS spec required that you be able to recognize old change
> attributes so that they can be discarded. I could be wrong here though.
> I'd have to go back and look through the spec to be sure.

https://tools.ietf.org/html/rfc7862#section-10

--b.

[toc] | [prev] | [next] | [standalone]

#1605860

On Tue, Mar 21, 2017 at 01:37:04PM -0400, J. Bruce Fields wrote:
> On Tue, Mar 21, 2017 at 01:23:24PM -0400, Jeff Layton wrote:
> > On Tue, 2017-03-21 at 12:30 -0400, J. Bruce Fields wrote:
> > > 	- NFS doesn't actually require that it increases, but I think it
> > > 	  should.  I assume 64 bits means we don't need a discussion of
> > > 	  wraparound.
> > 
> > I thought NFS spec required that you be able to recognize old change
> > attributes so that they can be discarded. I could be wrong here though.
> > I'd have to go back and look through the spec to be sure.
> 
> https://tools.ietf.org/html/rfc7862#section-10

So, I'm suggesting we implement this one:

	NFS4_CHANGE_TYPE_IS_MONOTONIC_INCR:  The change attribute value
	  MUST monotonically increase for every atomic change to the file
	  attributes, data, or directory contents.

It may be a slight lie--after your patches we wouldn't actually increase
"for every atomic change".  I think that's OK.

--b.

[toc] | [prev] | [next] | [standalone]

#1605904 — Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization

On Tue, 2017-03-21 at 14:30 -0400, J. Bruce Fields wrote:
> On Tue, Mar 21, 2017 at 01:23:24PM -0400, Jeff Layton wrote:
> > On Tue, 2017-03-21 at 12:30 -0400, J. Bruce Fields wrote:
> > > - It's durable; the above comparison still works if there were reboots
> > >   between the two i_version checks.
> > > 	- I don't know how realistic this is--we may need to figure out
> > > 	  if there's a weaker guarantee that's still useful.  Do
> > > 	  filesystems actually make ctime/mtime/i_version changes
> > > 	  atomically with the changes that caused them?  What if a
> > > 	  change attribute is exposed to an NFS client but doesn't make
> > > 	  it to disk, and then that value is reused after reboot?
> > > 
> > 
> > Yeah, there could be atomicity there. If we bump i_version, we'll mark
> > the inode dirty and I think that will end up with the new i_version at
> > least being journalled before __mark_inode_dirty returns.
> 
> So you think the filesystem can provide the atomicity?  In more detail:
> 

Sorry, I hit send too quickly. That should have read:

"Yeah, there could be atomicity issues there."

I think providing that level of atomicity may be difficult, though
maybe there's some way to make the querying of i_version block until
the inode update has been journalled?

> 	- if I write to a file, a simultaneous reader should see either
> 	  (old data, old i_version) or (new data, new i_version), not a
> 	  combination of the two.
> 	- ditto for metadata modifications.
> 	- the same should be true if there's a crash.
> 
> (If that's not possible, then I think we could live with a brief window
> of (new data, old i_version) as long as it doesn't persist beyond sync?)
> 
> > That said, I suppose it is possible for us to bump the counter, hand
> > that new counter value out to a NFS client and then the box crashes
> > before it makes it to the journal.
> > 
> > Not sure how big a problem that really is.
> 
> The other case I was wondering about may have been unclear.  Represent
> the state of a file by a (data, i_version) pair.  Say:
> 
> 	- file is modified from (F, V) to (F', V+1).
> 	- client reads and caches (F', V+1).
> 	- server crashes before writeback, so disk still has (F, V).
> 	- after restart, someone else modifies file to (F'', V+1).
> 	- original client revalidates its cache, sees V+1, concludes
> 	  file data is still F'.
> 
> This may not cause a real problem for clients depending only on
> traditional NFS close-to-open semantics.
> 
> 

No, I think that is a legitimate problem.

That said, after F'', the mtime would almost certainly be different
from the time after F', and that would likely be enough to prevent
confusion in NFS clients.

Applications that are just looking at i_version via statx() could get
confused though.

-- 
Jeff Layton <jlayton@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1605931

On Tue, Mar 21, 2017 at 02:46:53PM -0400, Jeff Layton wrote:
> On Tue, 2017-03-21 at 14:30 -0400, J. Bruce Fields wrote:
> > On Tue, Mar 21, 2017 at 01:23:24PM -0400, Jeff Layton wrote:
> > > On Tue, 2017-03-21 at 12:30 -0400, J. Bruce Fields wrote:
> > > > - It's durable; the above comparison still works if there were reboots
> > > >   between the two i_version checks.
> > > > 	- I don't know how realistic this is--we may need to figure out
> > > > 	  if there's a weaker guarantee that's still useful.  Do
> > > > 	  filesystems actually make ctime/mtime/i_version changes
> > > > 	  atomically with the changes that caused them?  What if a
> > > > 	  change attribute is exposed to an NFS client but doesn't make
> > > > 	  it to disk, and then that value is reused after reboot?
> > > > 
> > > 
> > > Yeah, there could be atomicity there. If we bump i_version, we'll mark
> > > the inode dirty and I think that will end up with the new i_version at
> > > least being journalled before __mark_inode_dirty returns.
> > 
> > So you think the filesystem can provide the atomicity?  In more detail:
> > 
> 
> Sorry, I hit send too quickly. That should have read:
> 
> "Yeah, there could be atomicity issues there."
> 
> I think providing that level of atomicity may be difficult, though
> maybe there's some way to make the querying of i_version block until
> the inode update has been journalled?

No idea.  Anyway, I'd like to figure out some reasonable requirement
that we can document.

> 
> > 	- if I write to a file, a simultaneous reader should see either
> > 	  (old data, old i_version) or (new data, new i_version), not a
> > 	  combination of the two.
> > 	- ditto for metadata modifications.
> > 	- the same should be true if there's a crash.
> > 
> > (If that's not possible, then I think we could live with a brief window
> > of (new data, old i_version) as long as it doesn't persist beyond sync?)
> > 
> > > That said, I suppose it is possible for us to bump the counter, hand
> > > that new counter value out to a NFS client and then the box crashes
> > > before it makes it to the journal.
> > > 
> > > Not sure how big a problem that really is.
> > 
> > The other case I was wondering about may have been unclear.  Represent
> > the state of a file by a (data, i_version) pair.  Say:
> > 
> > 	- file is modified from (F, V) to (F', V+1).
> > 	- client reads and caches (F', V+1).
> > 	- server crashes before writeback, so disk still has (F, V).
> > 	- after restart, someone else modifies file to (F'', V+1).
> > 	- original client revalidates its cache, sees V+1, concludes
> > 	  file data is still F'.
> > 
> > This may not cause a real problem for clients depending only on
> > traditional NFS close-to-open semantics.
> > 
> > 
> 
> No, I think that is a legitimate problem.
> 
> That said, after F'', the mtime would almost certainly be different
> from the time after F', and that would likely be enough to prevent
> confusion in NFS clients.

Oh, good point.  So, may be worth saying that anyone wanting to make
sense of these across reboot should compare times as well (maybe that
should be in nfs rfc's too).  I think that should be ctime not mtime,
though?

--b.

[toc] | [prev] | [next] | [standalone]

#1606017 — Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization

On Tue, 2017-03-21 at 15:13 -0400, J. Bruce Fields wrote:
> On Tue, Mar 21, 2017 at 02:46:53PM -0400, Jeff Layton wrote:
> > On Tue, 2017-03-21 at 14:30 -0400, J. Bruce Fields wrote:
> > > On Tue, Mar 21, 2017 at 01:23:24PM -0400, Jeff Layton wrote:
> > > > On Tue, 2017-03-21 at 12:30 -0400, J. Bruce Fields wrote:
> > > > > - It's durable; the above comparison still works if there were reboots
> > > > >   between the two i_version checks.
> > > > > 	- I don't know how realistic this is--we may need to figure out
> > > > > 	  if there's a weaker guarantee that's still useful.  Do
> > > > > 	  filesystems actually make ctime/mtime/i_version changes
> > > > > 	  atomically with the changes that caused them?  What if a
> > > > > 	  change attribute is exposed to an NFS client but doesn't make
> > > > > 	  it to disk, and then that value is reused after reboot?
> > > > > 
> > > > 
> > > > Yeah, there could be atomicity there. If we bump i_version, we'll mark
> > > > the inode dirty and I think that will end up with the new i_version at
> > > > least being journalled before __mark_inode_dirty returns.
> > > 
> > > So you think the filesystem can provide the atomicity?  In more detail:
> > > 
> > 
> > Sorry, I hit send too quickly. That should have read:
> > 
> > "Yeah, there could be atomicity issues there."
> > 
> > I think providing that level of atomicity may be difficult, though
> > maybe there's some way to make the querying of i_version block until
> > the inode update has been journalled?
> 
> No idea.  Anyway, I'd like to figure out some reasonable requirement
> that we can document.
> 
> > 
> > > 	- if I write to a file, a simultaneous reader should see either
> > > 	  (old data, old i_version) or (new data, new i_version), not a
> > > 	  combination of the two.
> > > 	- ditto for metadata modifications.
> > > 	- the same should be true if there's a crash.
> > > 
> > > (If that's not possible, then I think we could live with a brief window
> > > of (new data, old i_version) as long as it doesn't persist beyond sync?)
> > > 
> > > > That said, I suppose it is possible for us to bump the counter, hand
> > > > that new counter value out to a NFS client and then the box crashes
> > > > before it makes it to the journal.
> > > > 
> > > > Not sure how big a problem that really is.
> > > 
> > > The other case I was wondering about may have been unclear.  Represent
> > > the state of a file by a (data, i_version) pair.  Say:
> > > 
> > > 	- file is modified from (F, V) to (F', V+1).
> > > 	- client reads and caches (F', V+1).
> > > 	- server crashes before writeback, so disk still has (F, V).
> > > 	- after restart, someone else modifies file to (F'', V+1).
> > > 	- original client revalidates its cache, sees V+1, concludes
> > > 	  file data is still F'.
> > > 
> > > This may not cause a real problem for clients depending only on
> > > traditional NFS close-to-open semantics.
> > > 
> > > 
> > 
> > No, I think that is a legitimate problem.
> > 
> > That said, after F'', the mtime would almost certainly be different
> > from the time after F', and that would likely be enough to prevent
> > confusion in NFS clients.
> 
> Oh, good point.  So, may be worth saying that anyone wanting to make
> sense of these across reboot should compare times as well (maybe that
> should be in nfs rfc's too).  I think that should be ctime not mtime,
> though?
> 

Yes, it might be worth a mention there. IIRC, it does mention that you
shouldn't just look at a single attribute for cache validation
purposes, but the wording is a bit vague. I can't find the section at
the moment though.

The more I think about it though, simply ensuring that we don't publish
 a new change attr until the inode update has hit the journal may be
the best we can do. I'd have to think about how to implement that
though.
-- 
Jeff Layton <jlayton@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1605905

On Tue, Mar 21, 2017 at 01:23:24PM -0400, Jeff Layton wrote:
> On Tue, 2017-03-21 at 12:30 -0400, J. Bruce Fields wrote:
> > - It's durable; the above comparison still works if there were reboots
> >   between the two i_version checks.
> > 	- I don't know how realistic this is--we may need to figure out
> > 	  if there's a weaker guarantee that's still useful.  Do
> > 	  filesystems actually make ctime/mtime/i_version changes
> > 	  atomically with the changes that caused them?  What if a
> > 	  change attribute is exposed to an NFS client but doesn't make
> > 	  it to disk, and then that value is reused after reboot?
> > 
> 
> Yeah, there could be atomicity there. If we bump i_version, we'll mark
> the inode dirty and I think that will end up with the new i_version at
> least being journalled before __mark_inode_dirty returns.

So you think the filesystem can provide the atomicity?  In more detail:

	- if I write to a file, a simultaneous reader should see either
	  (old data, old i_version) or (new data, new i_version), not a
	  combination of the two.
	- ditto for metadata modifications.
	- the same should be true if there's a crash.

(If that's not possible, then I think we could live with a brief window
of (new data, old i_version) as long as it doesn't persist beyond sync?)

> That said, I suppose it is possible for us to bump the counter, hand
> that new counter value out to a NFS client and then the box crashes
> before it makes it to the journal.
> 
> Not sure how big a problem that really is.

The other case I was wondering about may have been unclear.  Represent
the state of a file by a (data, i_version) pair.  Say:

	- file is modified from (F, V) to (F', V+1).
	- client reads and caches (F', V+1).
	- server crashes before writeback, so disk still has (F, V).
	- after restart, someone else modifies file to (F'', V+1).
	- original client revalidates its cache, sees V+1, concludes
	  file data is still F'.

This may not cause a real problem for clients depending only on
traditional NFS close-to-open semantics.

--b.

[toc] | [prev] | [next] | [standalone]

#1606012

On Tue, Mar 21, 2017 at 01:23:24PM -0400, Jeff Layton wrote:
> On Tue, 2017-03-21 at 12:30 -0400, J. Bruce Fields wrote:
> > - It's durable; the above comparison still works if there were reboots
> >   between the two i_version checks.
> > 	- I don't know how realistic this is--we may need to figure out
> > 	  if there's a weaker guarantee that's still useful.  Do
> > 	  filesystems actually make ctime/mtime/i_version changes
> > 	  atomically with the changes that caused them?  What if a
> > 	  change attribute is exposed to an NFS client but doesn't make
> > 	  it to disk, and then that value is reused after reboot?
> > 
> 
> Yeah, there could be atomicity there. If we bump i_version, we'll mark
> the inode dirty and I think that will end up with the new i_version at
> least being journalled before __mark_inode_dirty returns.

The change may be journalled, but it isn't guaranteed stable until
fsync is run on the inode.

NFS server operations commit the metadata changed by a modification
through ->commit_metadata or sync_inode_metadata() before the
response is sent back to the client, hence guaranteeing that
i_version changes through the NFS server are stable and durable.

This is not the case for normal operations done through the POSIX
API - the journalling is asynchronous and the only durability
guarantees are provided by fsync()....

> That said, I suppose it is possible for us to bump the counter, hand
> that new counter value out to a NFS client and then the box crashes
> before it makes it to the journal.

Yup, this has aways been a problem when you mix posix applications
running on the NFS server modifying the same files as the NFS
clients are accessing and requiring synchronisation.

> Not sure how big a problem that really is.

This coherency problem has always existed on the server side...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

#1606953 — Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization

On Wed, 2017-03-22 at 08:45 +1100, Dave Chinner wrote:
> On Tue, Mar 21, 2017 at 01:23:24PM -0400, Jeff Layton wrote:
> > On Tue, 2017-03-21 at 12:30 -0400, J. Bruce Fields wrote:
> > > - It's durable; the above comparison still works if there were reboots
> > >   between the two i_version checks.
> > > 	- I don't know how realistic this is--we may need to figure out
> > > 	  if there's a weaker guarantee that's still useful.  Do
> > > 	  filesystems actually make ctime/mtime/i_version changes
> > > 	  atomically with the changes that caused them?  What if a
> > > 	  change attribute is exposed to an NFS client but doesn't make
> > > 	  it to disk, and then that value is reused after reboot?
> > > 
> > 
> > Yeah, there could be atomicity there. If we bump i_version, we'll mark
> > the inode dirty and I think that will end up with the new i_version at
> > least being journalled before __mark_inode_dirty returns.
> 
> The change may be journalled, but it isn't guaranteed stable until
> fsync is run on the inode.
> 
> NFS server operations commit the metadata changed by a modification
> through ->commit_metadata or sync_inode_metadata() before the
> response is sent back to the client, hence guaranteeing that
> i_version changes through the NFS server are stable and durable.
> 
> This is not the case for normal operations done through the POSIX
> API - the journalling is asynchronous and the only durability
> guarantees are provided by fsync()....
> 

Ahh ok, I missed that...thanks.

I think we'll have a hard time making this fully atomic. We may end up
having to settle for something less (and doing our best to warn users
of that possibility).

One idea might be to tie the behavior to AT_FORCE/DONT_SYNC. In the
don't sync case, allow the kernel to hand out the i_version without
syncing it to disk. In the FORCE_SYNC case, do an fsync internally
before returning.

> > That said, I suppose it is possible for us to bump the counter, hand
> > that new counter value out to a NFS client and then the box crashes
> > before it makes it to the journal.
> 
> Yup, this has aways been a problem when you mix posix applications
> running on the NFS server modifying the same files as the NFS
> clients are accessing and requiring synchronisation.
> 
> > Not sure how big a problem that really is.
> 
> This coherency problem has always existed on the server side...
> 

Yes. I don't think this patchset makes anything worse in this regard.
We will need well-defined semantics here before i_version can be
exposed to userland via statx however.
-- 
Jeff Layton <jlayton@redhat.com>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web