LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Rik van Riel <riel@redhat.com>
To: Anton Salikhmetov <salikhmetov@gmail.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH][RFC][BUG] updating the ctime and mtime time stamps in msync()
Date: Wed, 9 Jan 2008 15:50:15 -0500 [thread overview]
Message-ID: <20080109155015.4d2d4c1d@cuia.boston.redhat.com> (raw)
In-Reply-To: <1199728459.26463.11.camel@codedot>
On Mon, 07 Jan 2008 20:54:19 +0300
Anton Salikhmetov <salikhmetov@gmail.com> wrote:
> This program showed that the msync() function had a bug:
> it did not update the st_mtime and st_ctime fields.
>
> The program shows appropriate behavior of the msync()
> function using the kernel with the proposed patch applied.
> Specifically, the ctime and mtime time stamps do change
> when modifying the mapped memory and do not change when
> there have been no write references between the mmap()
> and msync() system calls.
As long as the ctime and mtime stamps change when the memory is
written to, what exactly is the problem?
Is it that the ctime and mtime does not change again when the memory
is written to again?
Is there a way for backup programs to miss file modification times?
Could you explain (using short words and simple sentences) what the
exact problem is?
Eg.
1) program mmaps file
2) program writes to mmaped area
3) ??? <=== this part, in equally simple words :)
4) data loss
An explanation like that will help people understand exactly what the
bug is, and why the patch should be applied ASAP.
> The patch adds a call to the file_update_time() function to change
> the file metadata before syncing. The patch also contains
> substantial code cleanup: consolidated error check
> for function parameters, using the PAGE_ALIGN() macro instead of
> "manual" alignment, improved readability of the loop,
> which traverses the process memory regions, updated comments.
Due to the various cleanups all being in one file, it took me a while
to understand the patch. In an area of code this subtle, it may be
better to submit the cleanups in (a) separate patch(es) from the patch
that adds the call to file_update_time().
> - (vma->vm_flags & VM_SHARED)) {
> + if (file && (vma->vm_flags & VM_SHARED)) {
> get_file(file);
> - up_read(&mm->mmap_sem);
> - error = do_fsync(file, 0);
> - fput(file);
> - if (error || start >= end)
> - goto out;
> - down_read(&mm->mmap_sem);
> - vma = find_vma(mm, start);
> - } else {
> - if (start >= end) {
> - error = 0;
> - goto out_unlock;
> + if (file->f_mapping->host->i_state & I_DIRTY_PAGES)
> + file_update_time(file);
I wonder if calling file_update_time() from inside the loop is the
best idea. Why not call that function just once, after msync breaks
from the loop?
thanks,
Rik
--
All Rights Reversed
next prev parent reply other threads:[~2008-01-09 20:50 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-01-07 17:54 Anton Salikhmetov
2008-01-09 11:32 ` Anton Salikhmetov
2008-01-09 11:47 ` Jakob Oestergaard
2008-01-09 12:22 ` Jakob Oestergaard
2008-01-09 14:41 ` Jesper Juhl
2008-01-09 15:31 ` Anton Salikhmetov
2008-01-09 21:28 ` Peter Staubach
2008-01-09 20:50 ` Rik van Riel [this message]
2008-01-09 21:01 ` Klaus S. Madsen
2008-01-09 21:06 ` Valdis.Kletnieks
2008-01-09 22:06 ` Rik van Riel
2008-01-09 22:19 ` Peter Staubach
2008-01-09 22:33 ` Jakob Oestergaard
2008-01-09 23:41 ` Rik van Riel
2008-01-10 0:03 ` Anton Salikhmetov
2008-01-10 8:51 ` Jakob Oestergaard
2008-01-10 10:53 ` Anton Salikhmetov
2008-01-10 15:45 ` Rik van Riel
2008-01-10 15:56 ` Anton Salikhmetov
2008-01-10 16:07 ` Rik van Riel
2008-01-10 16:40 ` Anton Salikhmetov
2008-01-10 16:52 ` Peter Staubach
2008-01-10 16:46 ` Peter Staubach
2008-01-10 20:48 ` Valdis.Kletnieks
2008-01-10 0:48 ` Anton Salikhmetov
2008-01-10 0:40 ` Anton Salikhmetov
2008-01-09 21:18 ` Peter Staubach
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=20080109155015.4d2d4c1d@cuia.boston.redhat.com \
--to=riel@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=salikhmetov@gmail.com \
--subject='Re: [PATCH][RFC][BUG] updating the ctime and mtime time stamps in msync()' \
/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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).