All of lore.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: "J. Bruce Fields" <bfields@redhat.com>,
	linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Peter Staubach <pstaubach@exagrid.com>
Subject: Re: [PATCH 0/2] fix nfsd stable write implementation
Date: Wed, 31 Oct 2012 07:30:27 +1100	[thread overview]
Message-ID: <20121031073027.254242ed@notabene.brown> (raw)
In-Reply-To: <20121030140725.GC24618@fieldses.org>

[-- Attachment #1: Type: text/plain, Size: 1950 bytes --]

On Tue, 30 Oct 2012 10:07:25 -0400 "J. Bruce Fields" <bfields@fieldses.org>
wrote:

> On Tue, Oct 30, 2012 at 10:28:33AM +1100, NeilBrown wrote:
> > On Fri, 26 Oct 2012 17:06:55 -0400 "J. Bruce Fields" <bfields@redhat.com>
> > wrote:
> > 
> > > From: "J. Bruce Fields" <bfields@redhat.com>
> > > 
> > > Peter pointed out to me that the nfs server is implementing stable
> > > writes by setting the O_SYNC flag.  I can't see why we couldn't write
> > > and then sync instead, but I don't know this stuff as well as I should;
> > > does the following look reasonable to people?
> > 
> > Bruce changed the code to implement stable writes by calling
> > vfs_fsync_range().  I can't see why we couldn't use O_SYNC instead.
> > 
> > It seems like you are making a change just for the sake of making a change.
> > Is there some reason that you think a separate 'sync' is more efficient than
> > using O_SYNC ?
> 
> Oh, sorry, see the changelog on the second patch: the problem is that
> the struct file can be shared across multiple writes in the NFSv4 case,
> so a single stable write could make all subsequent writes synchronous.
> 
> (And some day people would like filehandle caching for v2/v3, in which
> case we'll run into the same problem.)

Ahh, I missed that.
Makes sense then, thanks.

> 
> > As a general principle, I think it is best to give the file system as much
> > information as possible to that it can make whatever optimisation decisions
> > that it wants to.
> > 
> > Setting O_SYNC gives the filesystem more information than not, because it
> > allows it to change the behaviour of the 'write' request... though I don't
> > know if any filesystem actually uses the information.
> 
> I'm not sure how to figure out if that's a real problem or not.

Search all filesystems for special handling of O_SYNC or O_DSYNC?

I strongly suspect that it is not a real problem.

Thanks,
NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>
To: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
Cc: "J. Bruce Fields"
	<bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Peter Staubach
	<pstaubach-83r9SdEf25FBDgjK7y7TUQ@public.gmane.org>
Subject: Re: [PATCH 0/2] fix nfsd stable write implementation
Date: Wed, 31 Oct 2012 07:30:27 +1100	[thread overview]
Message-ID: <20121031073027.254242ed@notabene.brown> (raw)
In-Reply-To: <20121030140725.GC24618-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]

On Tue, 30 Oct 2012 10:07:25 -0400 "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
wrote:

> On Tue, Oct 30, 2012 at 10:28:33AM +1100, NeilBrown wrote:
> > On Fri, 26 Oct 2012 17:06:55 -0400 "J. Bruce Fields" <bfields-H+wXaHxf7aJhl2p70BpVqQ@public.gmane.orgm>
> > wrote:
> > 
> > > From: "J. Bruce Fields" <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > 
> > > Peter pointed out to me that the nfs server is implementing stable
> > > writes by setting the O_SYNC flag.  I can't see why we couldn't write
> > > and then sync instead, but I don't know this stuff as well as I should;
> > > does the following look reasonable to people?
> > 
> > Bruce changed the code to implement stable writes by calling
> > vfs_fsync_range().  I can't see why we couldn't use O_SYNC instead.
> > 
> > It seems like you are making a change just for the sake of making a change.
> > Is there some reason that you think a separate 'sync' is more efficient than
> > using O_SYNC ?
> 
> Oh, sorry, see the changelog on the second patch: the problem is that
> the struct file can be shared across multiple writes in the NFSv4 case,
> so a single stable write could make all subsequent writes synchronous.
> 
> (And some day people would like filehandle caching for v2/v3, in which
> case we'll run into the same problem.)

Ahh, I missed that.
Makes sense then, thanks.

> 
> > As a general principle, I think it is best to give the file system as much
> > information as possible to that it can make whatever optimisation decisions
> > that it wants to.
> > 
> > Setting O_SYNC gives the filesystem more information than not, because it
> > allows it to change the behaviour of the 'write' request... though I don't
> > know if any filesystem actually uses the information.
> 
> I'm not sure how to figure out if that's a real problem or not.

Search all filesystems for special handling of O_SYNC or O_DSYNC?

I strongly suspect that it is not a real problem.

Thanks,
NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

  reply	other threads:[~2012-10-30 20:30 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-26 21:06 [PATCH 0/2] fix nfsd stable write implementation J. Bruce Fields
2012-10-26 21:06 ` J. Bruce Fields
2012-10-26 21:06 ` [PATCH 1/2] nfsd: assume writeable exportabled filesystems have f_sync J. Bruce Fields
2012-10-26 21:06   ` J. Bruce Fields
2012-10-26 21:06 ` [PATCH 2/2] nfsd: use vfs_fsync_range(), not O_SYNC, for stable writes J. Bruce Fields
2012-10-26 21:06   ` J. Bruce Fields
2012-10-29 23:28 ` [PATCH 0/2] fix nfsd stable write implementation NeilBrown
2012-10-29 23:28   ` NeilBrown
2012-10-30 14:07   ` J. Bruce Fields
2012-10-30 20:30     ` NeilBrown [this message]
2012-10-30 20:30       ` NeilBrown
2012-11-08  0:20       ` 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=20121031073027.254242ed@notabene.brown \
    --to=neilb@suse.de \
    --cc=bfields@fieldses.org \
    --cc=bfields@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=pstaubach@exagrid.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.