Messages in this thread | | | Date | Wed, 13 Jun 2012 15:08:09 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 1/2] msync: support syncing a small part of the file |
| |
On Wed, 13 Jun 2012 15:51:33 -0600 Zan Lynx <zlynx@acm.org> wrote:
> 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.
OK, that's one user accounted for. 3 million to go.
Look, I'm not terribly averse to the change, but it does have this risk. And it's a nasty risk because anyone who is hit by it simply will not know that his applcation has lost some of its data integrity.
> 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. :-)
No, not at all. The issue is the *removal* of a side-effect upon which some apps/designers may have been depending. Perhaps unintentionally!
| |