Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-09-07 Thread Peter Feiner
On Thu, Sep 04, 2014 at 09:43:11AM -0700, Peter Feiner wrote: > On Mon, Aug 25, 2014 at 09:45:34PM -0700, Hugh Dickins wrote: > > That sets me wondering: have you placed the VM_SOFTDIRTY check in the > > right place in this series of tests? > > > > I think, once pgprot_modify() is correct on all

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-09-07 Thread Peter Feiner
On Thu, Sep 04, 2014 at 09:43:11AM -0700, Peter Feiner wrote: On Mon, Aug 25, 2014 at 09:45:34PM -0700, Hugh Dickins wrote: That sets me wondering: have you placed the VM_SOFTDIRTY check in the right place in this series of tests? I think, once pgprot_modify() is correct on all

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-09-04 Thread Peter Feiner
On Mon, Aug 25, 2014 at 09:45:34PM -0700, Hugh Dickins wrote: > On Sun, 24 Aug 2014, Peter Feiner wrote: > > With this patch, write notifications are enabled when VM_SOFTDIRTY is > > cleared. Furthermore, to avoid unnecessary faults, write > > notifications are disabled when VM_SOFTDIRTY is reset.

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-09-04 Thread Peter Feiner
On Mon, Aug 25, 2014 at 09:45:34PM -0700, Hugh Dickins wrote: On Sun, 24 Aug 2014, Peter Feiner wrote: With this patch, write notifications are enabled when VM_SOFTDIRTY is cleared. Furthermore, to avoid unnecessary faults, write notifications are disabled when VM_SOFTDIRTY is reset.

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-28 Thread Cyrill Gorcunov
On Wed, Aug 27, 2014 at 04:12:43PM -0700, Hugh Dickins wrote: > > > > > > Weak argument to me. > > Yes. However rarely it's modified, we don't want any chance of it > corrupting another flag. > > VM_SOFTDIRTY is special in the sense that it's maintained in a very > different way from the other

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-28 Thread Cyrill Gorcunov
On Wed, Aug 27, 2014 at 04:12:43PM -0700, Hugh Dickins wrote: Weak argument to me. Yes. However rarely it's modified, we don't want any chance of it corrupting another flag. VM_SOFTDIRTY is special in the sense that it's maintained in a very different way from the other VM_flags.

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-27 Thread Hugh Dickins
On Tue, 26 Aug 2014, Cyrill Gorcunov wrote: > On Tue, Aug 26, 2014 at 06:43:55PM +0300, Kirill A. Shutemov wrote: > > On Tue, Aug 26, 2014 at 07:18:13PM +0400, Cyrill Gorcunov wrote: > > > > Basically, it's safe if only soft-dirty is allowed to modify vm_flags > > > > without down_write(). But why

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-27 Thread Hugh Dickins
On Tue, 26 Aug 2014, Cyrill Gorcunov wrote: > On Mon, Aug 25, 2014 at 09:45:34PM -0700, Hugh Dickins wrote: > > > > Hmm. For a long time I thought you were fixing another important bug > > with down_write, since we "always" use down_write to modify vm_flags. > > > > But now I'm realizing that

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-27 Thread Hugh Dickins
On Tue, 26 Aug 2014, Cyrill Gorcunov wrote: On Mon, Aug 25, 2014 at 09:45:34PM -0700, Hugh Dickins wrote: Hmm. For a long time I thought you were fixing another important bug with down_write, since we always use down_write to modify vm_flags. But now I'm realizing that if this is the

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-27 Thread Hugh Dickins
On Tue, 26 Aug 2014, Cyrill Gorcunov wrote: On Tue, Aug 26, 2014 at 06:43:55PM +0300, Kirill A. Shutemov wrote: On Tue, Aug 26, 2014 at 07:18:13PM +0400, Cyrill Gorcunov wrote: Basically, it's safe if only soft-dirty is allowed to modify vm_flags without down_write(). But why is

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Cyrill Gorcunov
On Tue, Aug 26, 2014 at 06:43:55PM +0300, Kirill A. Shutemov wrote: > On Tue, Aug 26, 2014 at 07:18:13PM +0400, Cyrill Gorcunov wrote: > > > Basically, it's safe if only soft-dirty is allowed to modify vm_flags > > > without down_write(). But why is soft-dirty so special? > > > > because how we

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Kirill A. Shutemov
On Tue, Aug 26, 2014 at 07:18:13PM +0400, Cyrill Gorcunov wrote: > > Basically, it's safe if only soft-dirty is allowed to modify vm_flags > > without down_write(). But why is soft-dirty so special? > > because how we use this bit, i mean in normal workload this bit won't > be used intensively i

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Cyrill Gorcunov
On Tue, Aug 26, 2014 at 05:56:12PM +0300, Kirill A. Shutemov wrote: > > > > > > It seems safe in vma-softdirty context. But if somebody else will decide > > > that > > > it's fine to modify vm_flags without down_write (in their context), we > > > will get trouble. Sasha will come with weird bug

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Kirill A. Shutemov
On Tue, Aug 26, 2014 at 06:19:14PM +0400, Cyrill Gorcunov wrote: > On Tue, Aug 26, 2014 at 05:04:19PM +0300, Kirill A. Shutemov wrote: > > > > > > > > But now I'm realizing that if this is the _only_ place which modifies > > > > vm_flags with down_read, then it's "probably" safe. I've a vague >

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Cyrill Gorcunov
On Tue, Aug 26, 2014 at 05:04:19PM +0300, Kirill A. Shutemov wrote: > > > > > > But now I'm realizing that if this is the _only_ place which modifies > > > vm_flags with down_read, then it's "probably" safe. I've a vague > > > feeling that this was discussed before - is that so, Cyrill? > > > >

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Kirill A. Shutemov
On Tue, Aug 26, 2014 at 10:49:52AM +0400, Cyrill Gorcunov wrote: > On Mon, Aug 25, 2014 at 09:45:34PM -0700, Hugh Dickins wrote: > > > +static int clear_refs(struct mm_struct *mm, enum clear_refs_types type, > > > + int write) > > > +{ > ... > > > + > > > + if (write) > > > +

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Cyrill Gorcunov
On Mon, Aug 25, 2014 at 09:45:34PM -0700, Hugh Dickins wrote: > > +static int clear_refs(struct mm_struct *mm, enum clear_refs_types type, > > + int write) > > +{ ... > > + > > + if (write) > > + down_write(>mmap_sem); > > + else > > +

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Cyrill Gorcunov
On Mon, Aug 25, 2014 at 09:45:34PM -0700, Hugh Dickins wrote: +static int clear_refs(struct mm_struct *mm, enum clear_refs_types type, + int write) +{ ... + + if (write) + down_write(mm-mmap_sem); + else + down_read(mm-mmap_sem); + +

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Kirill A. Shutemov
On Tue, Aug 26, 2014 at 10:49:52AM +0400, Cyrill Gorcunov wrote: On Mon, Aug 25, 2014 at 09:45:34PM -0700, Hugh Dickins wrote: +static int clear_refs(struct mm_struct *mm, enum clear_refs_types type, + int write) +{ ... + + if (write) +

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Cyrill Gorcunov
On Tue, Aug 26, 2014 at 05:04:19PM +0300, Kirill A. Shutemov wrote: But now I'm realizing that if this is the _only_ place which modifies vm_flags with down_read, then it's probably safe. I've a vague feeling that this was discussed before - is that so, Cyrill? Well, as far as I

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Kirill A. Shutemov
On Tue, Aug 26, 2014 at 06:19:14PM +0400, Cyrill Gorcunov wrote: On Tue, Aug 26, 2014 at 05:04:19PM +0300, Kirill A. Shutemov wrote: But now I'm realizing that if this is the _only_ place which modifies vm_flags with down_read, then it's probably safe. I've a vague feeling that

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Cyrill Gorcunov
On Tue, Aug 26, 2014 at 05:56:12PM +0300, Kirill A. Shutemov wrote: It seems safe in vma-softdirty context. But if somebody else will decide that it's fine to modify vm_flags without down_write (in their context), we will get trouble. Sasha will come with weird bug report one day

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Kirill A. Shutemov
On Tue, Aug 26, 2014 at 07:18:13PM +0400, Cyrill Gorcunov wrote: Basically, it's safe if only soft-dirty is allowed to modify vm_flags without down_write(). But why is soft-dirty so special? because how we use this bit, i mean in normal workload this bit won't be used intensively i think

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-26 Thread Cyrill Gorcunov
On Tue, Aug 26, 2014 at 06:43:55PM +0300, Kirill A. Shutemov wrote: On Tue, Aug 26, 2014 at 07:18:13PM +0400, Cyrill Gorcunov wrote: Basically, it's safe if only soft-dirty is allowed to modify vm_flags without down_write(). But why is soft-dirty so special? because how we use this

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-25 Thread Hugh Dickins
On Sun, 24 Aug 2014, Peter Feiner wrote: > For VMAs that don't want write notifications, PTEs created for read > faults have their write bit set. If the read fault happens after > VM_SOFTDIRTY is cleared, then the PTE's softdirty bit will remain > clear after subsequent writes. Good catch.

Re: [PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-25 Thread Hugh Dickins
On Sun, 24 Aug 2014, Peter Feiner wrote: For VMAs that don't want write notifications, PTEs created for read faults have their write bit set. If the read fault happens after VM_SOFTDIRTY is cleared, then the PTE's softdirty bit will remain clear after subsequent writes. Good catch. Worrying

[PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-24 Thread Peter Feiner
For VMAs that don't want write notifications, PTEs created for read faults have their write bit set. If the read fault happens after VM_SOFTDIRTY is cleared, then the PTE's softdirty bit will remain clear after subsequent writes. Here's a simple code snippet to demonstrate the bug: char* m =

[PATCH v5] mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared

2014-08-24 Thread Peter Feiner
For VMAs that don't want write notifications, PTEs created for read faults have their write bit set. If the read fault happens after VM_SOFTDIRTY is cleared, then the PTE's softdirty bit will remain clear after subsequent writes. Here's a simple code snippet to demonstrate the bug: char* m =