From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f70.google.com (mail-lf0-f70.google.com [209.85.215.70]) by kanga.kvack.org (Postfix) with ESMTP id 785306B0005 for ; Tue, 17 May 2016 07:47:25 -0400 (EDT) Received: by mail-lf0-f70.google.com with SMTP id u64so7900327lff.2 for ; Tue, 17 May 2016 04:47:25 -0700 (PDT) Received: from mail-lb0-x242.google.com (mail-lb0-x242.google.com. [2a00:1450:4010:c04::242]) by mx.google.com with ESMTPS id h5si2183085lbb.110.2016.05.17.04.47.23 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 17 May 2016 04:47:24 -0700 (PDT) Received: by mail-lb0-x242.google.com with SMTP id qf3so786697lbb.0 for ; Tue, 17 May 2016 04:47:23 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20160517113634.GD9540@node.shutemov.name> References: <20160506150112.9b27324b4b2b141146b0ff25@linux-foundation.org> <20160516133543.GA9540@node.shutemov.name> <20160517113634.GD9540@node.shutemov.name> Date: Tue, 17 May 2016 17:17:23 +0530 Message-ID: Subject: Re: [Bug 117731] New: Doing mprotect for PROT_NONE and then for PROT_READ|PROT_WRITE reduces CPU write B/W on buffer From: Ashish Srivastava Content-Type: multipart/alternative; boundary=001a11c3423aefa0420533084d4a Sender: owner-linux-mm@kvack.org List-ID: To: "Kirill A. Shutemov" Cc: Andrew Morton , bugzilla-daemon@bugzilla.kernel.org, Peter Feiner , linux-mm@kvack.org --001a11c3423aefa0420533084d4a Content-Type: text/plain; charset=UTF-8 > Test-case for that would be helpful, as normal malloc()'ed anon memory > cannot be subject for the bug. Unless I miss something obvious. I've modified the test-case attached to the bug and now it doesn't use malloc()'ed memory but file backed mmap shared memory. On Tue, May 17, 2016 at 5:06 PM, Kirill A. Shutemov wrote: > On Tue, May 17, 2016 at 04:56:02PM +0530, Ashish Srivastava wrote: > > Yes, the original repro was using a custom allocator but I was seeing the > > issue with malloc'd memory as well on my (ARMv7) platform. > > Test-case for that would be helpful, as normal malloc()'ed anon memory > cannot be subject for the bug. Unless I miss something obvious. > > > I agree that the repro code won't reliably work so have modified the > repro > > code attached to the bug to use file backed memory. > > > > That really is the root cause of the problem. I can make the following > > change in the kernel that can make the slow writes problem go away. > > This makes vma_set_page_prot return the value of vma_wants_writenotify to > > the caller after setting vma->vmpage_prot. > > > > In vma_set_page_prot: > > -void vma_set_page_prot(struct vm_area_struct *vma) > > +bool vma_set_page_prot(struct vm_area_struct *vma) > > { > > unsigned long vm_flags = vma->vm_flags; > > > > vma->vm_page_prot = vm_pgprot_modify(vma->vm_page_prot, vm_flags); > > if (vma_wants_writenotify(vma)) { > > vm_flags &= ~VM_SHARED; > > vma->vm_page_prot = vm_pgprot_modify(vma->vm_page_prot, > > vm_flags); > > + return 1; > > } > > + return 0; > > } > > > > In mprotect_fixup: > > > > * held in write mode. > > */ > > vma->vm_flags = newflags; > > - dirty_accountable = vma_wants_writenotify(vma); > > - vma_set_page_prot(vma); > > + dirty_accountable = vma_set_page_prot(vma); > > > > change_protection(vma, start, end, vma->vm_page_prot, > > dirty_accountable, 0) > > > > That looks good to me. Please prepare proper patch. > > -- > Kirill A. Shutemov > --001a11c3423aefa0420533084d4a Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
> Test-case for that would be helpful, as normal m= alloc()'ed anon memory
> cannot be subject for the bug. Unless I = miss something obvious.

I've modified the test-case attach= ed to the bug and now it doesn't use malloc()'ed memory but file ba= cked mmap shared memory.

On Tue, May 17, 2016 at 5:06 PM, Kirill A. Shutemov <k= irill@shutemov.name> wrote:
On Tue, May 17, 2016 at 04:56:02PM +0530, Ashish Srivasta= va wrote:
> Yes, the original repro was using a custom allocator but I was seeing = the
> issue with malloc'd memory as well on my (ARMv7) platform.

Test-case for that would be helpful, as normal malloc()'ed anon = memory
cannot be subject for the bug. Unless I miss something obvious.

> I agree that the repro code won't reliably work so have modified t= he repro
> code attached to the bug to use file backed memory.
>
> That really is the root cause of the problem. I can make the following=
> change in the kernel that can make the slow writes problem go away. > This makes vma_set_page_prot return the value of vma_wants_writenotify= to
> the caller after setting vma->vmpage_prot.
>
> In vma_set_page_prot:
> -void vma_set_page_prot(struct vm_area_struct *vma)
> +bool vma_set_page_prot(struct vm_area_struct *vma)
> {
>=C2=A0 =C2=A0 =C2=A0unsigned long vm_flags =3D vma->vm_flags;
>
>=C2=A0 =C2=A0 =C2=A0vma->vm_page_prot =3D vm_pgprot_modify(vma->v= m_page_prot, vm_flags);
>=C2=A0 =C2=A0 =C2=A0if (vma_wants_writenotify(vma)) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0vm_flags &=3D ~VM_SHARED;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0vma->vm_page_prot =3D vm_pgprot_mo= dify(vma->vm_page_prot,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 vm_flags);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return 1;
>=C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 return 0;
> }
>
> In mprotect_fixup:
>
>=C2=A0 =C2=A0 =C2=A0 * held in write mode.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0*/
>=C2=A0 =C2=A0 =C2=A0 vma->vm_flags =3D newflags;
> -=C2=A0 =C2=A0 dirty_accountable =3D vma_wants_writenotify(vma);
> -=C2=A0 =C2=A0 vma_set_page_prot(vma);
> +=C2=A0 =C2=A0 dirty_accountable =3D vma_set_page_prot(vma);
>
>=C2=A0 =C2=A0 =C2=A0 change_protection(vma, start, end, vma->vm_page= _prot,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dirty_accountab= le, 0)
>

That looks good to me. Please prepare proper patch.

--
=C2=A0Kirill A. Shutemov

--001a11c3423aefa0420533084d4a-- -- 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: email@kvack.org