All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Staubach <staubach@redhat.com>
To: Anton Salikhmetov <salikhmetov@gmail.com>
Cc: Rik van Riel <riel@redhat.com>,
	Jakob Oestergaard <jakob@unthought.net>,
	Valdis.Kletnieks@vt.edu, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH][RFC][BUG] updating the ctime and mtime time stamps in msync()
Date: Thu, 10 Jan 2008 11:52:57 -0500	[thread overview]
Message-ID: <47864D69.40209@redhat.com> (raw)
In-Reply-To: <4df4ef0c0801100840uf84fef6g80e456fc5681193@mail.gmail.com>

Anton Salikhmetov wrote:
> 2008/1/10, Rik van Riel <riel@redhat.com>:
>   
>> On Thu, 10 Jan 2008 18:56:07 +0300
>> "Anton Salikhmetov" <salikhmetov@gmail.com> wrote:
>>
>>     
>>> However, I don't see how they will work if there has been
>>> something like a sync(2) done after the mmap'd region is
>>> modified and the msync call.  When the inode is written out
>>> as part of the sync process, I_DIRTY_PAGES will be cleared,
>>> thus causing a miss in this code.
>>>
>>> The I_DIRTY_PAGES check here is good, but I think that there
>>> needs to be some code elsewhere too, to catch the case where
>>> I_DIRTY_PAGES is being cleared, but the time fields still need
>>> to be updated.
>>>       
>> Agreed. The mtime and ctime should probably also be updated
>> when I_DIRTY_PAGES is cleared.
>>
>> The alternative would be to remember that the inode had been
>> dirty in the past, and have the mtime and ctime updated on
>> msync or close - which would be more complex.
>>     
>
> Adding the new flag (AS_MCTIME) has been already suggested by Peter
> Staubach in his first solution for this bug. Now I understand that the
> AS_MCTIME flag is required for fixing the bug.

Well, that was the approach before we had I_DIRTY_PAGES.  I am
still wondering whether we can get this approach to work, with
a little more support and heuristics.  PeterZ's work to better
track dirty pages should be helpful in determining when and why
a patch was dirty.

I keep thinking that by recording the time when a page was found
to be dirty and the file is mmap'd and then updating the mtime
and ctime fields in the inode during msync() and sync_single_inode()
if that time is newer than the mtime and ctime fields, then we
can solve the problem of when and when not to update those two
time fields.

I haven't had a chance to think it all through completely or do
the appropriate analysis yet though.

       ps

WARNING: multiple messages have this Message-ID (diff)
From: Peter Staubach <staubach@redhat.com>
To: Anton Salikhmetov <salikhmetov@gmail.com>
Cc: Rik van Riel <riel@redhat.com>,
	Jakob Oestergaard <jakob@unthought.net>,
	Valdis.Kletnieks@vt.edu, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH][RFC][BUG] updating the ctime and mtime time stamps in msync()
Date: Thu, 10 Jan 2008 11:52:57 -0500	[thread overview]
Message-ID: <47864D69.40209@redhat.com> (raw)
In-Reply-To: <4df4ef0c0801100840uf84fef6g80e456fc5681193@mail.gmail.com>

Anton Salikhmetov wrote:
> 2008/1/10, Rik van Riel <riel@redhat.com>:
>   
>> On Thu, 10 Jan 2008 18:56:07 +0300
>> "Anton Salikhmetov" <salikhmetov@gmail.com> wrote:
>>
>>     
>>> However, I don't see how they will work if there has been
>>> something like a sync(2) done after the mmap'd region is
>>> modified and the msync call.  When the inode is written out
>>> as part of the sync process, I_DIRTY_PAGES will be cleared,
>>> thus causing a miss in this code.
>>>
>>> The I_DIRTY_PAGES check here is good, but I think that there
>>> needs to be some code elsewhere too, to catch the case where
>>> I_DIRTY_PAGES is being cleared, but the time fields still need
>>> to be updated.
>>>       
>> Agreed. The mtime and ctime should probably also be updated
>> when I_DIRTY_PAGES is cleared.
>>
>> The alternative would be to remember that the inode had been
>> dirty in the past, and have the mtime and ctime updated on
>> msync or close - which would be more complex.
>>     
>
> Adding the new flag (AS_MCTIME) has been already suggested by Peter
> Staubach in his first solution for this bug. Now I understand that the
> AS_MCTIME flag is required for fixing the bug.

Well, that was the approach before we had I_DIRTY_PAGES.  I am
still wondering whether we can get this approach to work, with
a little more support and heuristics.  PeterZ's work to better
track dirty pages should be helpful in determining when and why
a patch was dirty.

I keep thinking that by recording the time when a page was found
to be dirty and the file is mmap'd and then updating the mtime
and ctime fields in the inode during msync() and sync_single_inode()
if that time is newer than the mtime and ctime fields, then we
can solve the problem of when and when not to update those two
time fields.

I haven't had a chance to think it all through completely or do
the appropriate analysis yet though.

       ps

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2008-01-10 16:53 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-07 17:54 [PATCH][RFC][BUG] updating the ctime and mtime time stamps in msync() Anton Salikhmetov
2008-01-07 17:54 ` Anton Salikhmetov, Anton Salikhmetov
2008-01-09 11:32 ` Anton Salikhmetov
2008-01-09 11:32   ` Anton Salikhmetov
2008-01-09 11:47   ` Jakob Oestergaard
2008-01-09 11:47     ` Jakob Oestergaard
2008-01-09 12:22   ` Jakob Oestergaard
2008-01-09 12:22     ` Jakob Oestergaard
2008-01-09 14:41   ` Jesper Juhl
2008-01-09 14:41     ` Jesper Juhl
2008-01-09 15:31     ` Anton Salikhmetov
2008-01-09 15:31       ` Anton Salikhmetov
2008-01-09 21:28   ` Peter Staubach
2008-01-09 21:28     ` Peter Staubach
2008-01-09 20:50 ` Rik van Riel
2008-01-09 20:50   ` Rik van Riel
2008-01-09 21:01   ` Klaus S. Madsen
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:06       ` Rik van Riel
2008-01-09 22:19       ` Peter Staubach
2008-01-09 22:19         ` Peter Staubach
2008-01-09 22:33       ` Jakob Oestergaard
2008-01-09 22:33         ` Jakob Oestergaard
2008-01-09 23:41         ` Rik van Riel
2008-01-09 23:41           ` Rik van Riel
2008-01-10  0:03           ` Anton Salikhmetov
2008-01-10  0:03             ` Anton Salikhmetov
2008-01-10  8:51             ` Jakob Oestergaard
2008-01-10  8:51               ` Jakob Oestergaard
2008-01-10 10:53               ` Anton Salikhmetov
2008-01-10 10:53                 ` Anton Salikhmetov
2008-01-10 15:45                 ` Rik van Riel
2008-01-10 15:45                   ` Rik van Riel
2008-01-10 15:56                   ` Anton Salikhmetov
2008-01-10 15:56                     ` Anton Salikhmetov
2008-01-10 16:07                     ` Rik van Riel
2008-01-10 16:07                       ` Rik van Riel
2008-01-10 16:40                       ` Anton Salikhmetov
2008-01-10 16:40                         ` Anton Salikhmetov
2008-01-10 16:52                         ` Peter Staubach [this message]
2008-01-10 16:52                           ` Peter Staubach
2008-01-10 16:46                       ` 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:48         ` Anton Salikhmetov
2008-01-10  0:40   ` Anton Salikhmetov
2008-01-10  0:40     ` Anton Salikhmetov
2008-01-09 21:18 ` Peter Staubach
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=47864D69.40209@redhat.com \
    --to=staubach@redhat.com \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=jakob@unthought.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=riel@redhat.com \
    --cc=salikhmetov@gmail.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.