All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zan Lynx <zlynx@acm.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	linux-kernel@vger.kernel.org, Hugh Dickins <hughd@google.com>
Subject: Re: [PATCH 1/2] msync: support syncing a small part of the file
Date: Wed, 13 Jun 2012 15:51:33 -0600	[thread overview]
Message-ID: <1339624293.2080.24.camel@knife> (raw)
In-Reply-To: <20120613142614.6af42ef5.akpm@linux-foundation.org>

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

On Wed, 2012-06-13 at 14:26 -0700, Andrew Morton wrote:
> On Thu, 31 May 2012 22:43:54 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> > msync does not need to flush changes to the entire file, even with MS_SYNC.
> > Instead, it can use vfs_fsync_range to only synchronize a part of the file.
> > 
> > In addition, not all metadata has to be synced; msync is closer to
> > fdatasync than it is to msync.  So, pass 1 to vfs_fsync_range.
> 
> Would be nice, but if applications were previously assuming that an
> msync() was syncing the whole file, this patch will secretly and subtly
> break them.  Convince me that this change won't weaken anyone's data
> integrity ;)

As an interested observer and a programmer who uses msync()...

I never assumed msync() did the whole file. It has an address and length
argument. I always assumed it only looked for dirty pages within that
range. That is also what the msync() documentation claims.

As for weakening data integrity because of assumptions programmers *may*
have made, I think this is a bad argument which followed to its logical
conclusion can only lead to requiring an implicit sync() before and
after every system call. :-)

Asking programmers to use sync_file_range() is not a replacement for
improving msync() because msync() is POSIX and can be used everywhere.
sync_file_range() requires Linux specific code.

-- 
Knowledge Is Power
Power Corrupts
Study Hard
Be Evil

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

  reply	other threads:[~2012-06-13 22:15 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-31 20:43 [PATCH 0/2] msync improvements Paolo Bonzini
2012-05-31 20:43 ` [PATCH 1/2] msync: support syncing a small part of the file Paolo Bonzini
2012-06-13 21:26   ` Andrew Morton
2012-06-13 21:51     ` Zan Lynx [this message]
2012-06-13 22:08       ` Andrew Morton
2012-06-14  8:57         ` Paolo Bonzini
2012-05-31 20:43 ` [PATCH 2/2] msync: start async writeout when MS_ASYNC Paolo Bonzini
2012-06-13 21:29   ` Andrew Morton
2012-06-14  9:02     ` Paolo Bonzini
2012-06-14 10:07       ` Andrew Morton
2012-06-14 10:19         ` Paolo Bonzini
2012-06-14 12:24     ` Paolo Bonzini
2012-06-12 15:38 ` [PATCH 0/2] msync improvements Paolo Bonzini

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=1339624293.2080.24.camel@knife \
    --to=zlynx@acm.org \
    --cc=akpm@linux-foundation.org \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.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.