From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vc0-f181.google.com (mail-vc0-f181.google.com [209.85.220.181]) by kanga.kvack.org (Postfix) with ESMTP id C2AD66B0036 for ; Tue, 29 Jul 2014 13:07:46 -0400 (EDT) Received: by mail-vc0-f181.google.com with SMTP id lf12so13752182vcb.40 for ; Tue, 29 Jul 2014 10:07:46 -0700 (PDT) Received: from mail-vc0-x229.google.com (mail-vc0-x229.google.com [2607:f8b0:400c:c03::229]) by mx.google.com with ESMTPS id z4si4955390vei.17.2014.07.29.10.07.46 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 29 Jul 2014 10:07:46 -0700 (PDT) Received: by mail-vc0-f169.google.com with SMTP id le20so5281030vcb.0 for ; Tue, 29 Jul 2014 10:07:46 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20140729142710.656A9E00A3@blue.fi.intel.com> References: <1406633609-17586-1-git-send-email-kirill.shutemov@linux.intel.com> <1406633609-17586-2-git-send-email-kirill.shutemov@linux.intel.com> <53D7A251.7010509@samsung.com> <20140729142710.656A9E00A3@blue.fi.intel.com> Date: Tue, 29 Jul 2014 21:07:45 +0400 Message-ID: Subject: Re: [PATCH 1/2] mm: close race between do_fault_around() and fault_around_bytes_set() From: Andrey Ryabinin Content-Type: text/plain; charset=UTF-8 Sender: owner-linux-mm@kvack.org List-ID: To: "Kirill A. Shutemov" Cc: Andrey Ryabinin , Andrew Morton , Dave Hansen , Sasha Levin , David Rientjes , linux-mm@kvack.org 2014-07-29 18:27 GMT+04:00 Kirill A. Shutemov : > Andrey Ryabinin wrote: >> On 07/29/14 15:33, Kirill A. Shutemov wrote: >> > Things can go wrong if fault_around_bytes will be changed under >> > do_fault_around(): between fault_around_mask() and fault_around_pages(). >> > >> > Let's read fault_around_bytes only once during do_fault_around() and >> > calculate mask based on the reading. >> > >> > Note: fault_around_bytes can only be updated via debug interface. Also >> > I've tried but was not able to trigger a bad behaviour without the >> > patch. So I would not consider this patch as urgent. >> > >> > Signed-off-by: Kirill A. Shutemov >> > --- >> > mm/memory.c | 17 +++++++++++------ >> > 1 file changed, 11 insertions(+), 6 deletions(-) >> > >> > diff --git a/mm/memory.c b/mm/memory.c >> > index 9d66bc66f338..2ce07dc9b52b 100644 >> > --- a/mm/memory.c >> > +++ b/mm/memory.c >> > @@ -2772,12 +2772,12 @@ static unsigned long fault_around_bytes = rounddown_pow_of_two(65536); >> > >> > static inline unsigned long fault_around_pages(void) >> > { >> > - return fault_around_bytes >> PAGE_SHIFT; >> > + return ACCESS_ONCE(fault_around_bytes) >> PAGE_SHIFT; >> > } >> > >> > -static inline unsigned long fault_around_mask(void) >> > +static inline unsigned long fault_around_mask(unsigned long nr_pages) >> > { >> > - return ~(fault_around_bytes - 1) & PAGE_MASK; >> > + return ~(nr_pages * PAGE_SIZE - 1) & PAGE_MASK; >> > } >> > >> > >> > @@ -2844,12 +2844,17 @@ late_initcall(fault_around_debugfs); >> > static void do_fault_around(struct vm_area_struct *vma, unsigned long address, >> > pte_t *pte, pgoff_t pgoff, unsigned int flags) >> > { >> > - unsigned long start_addr; >> > + unsigned long start_addr, nr_pages; >> > pgoff_t max_pgoff; >> > struct vm_fault vmf; >> > int off; >> > >> > - start_addr = max(address & fault_around_mask(), vma->vm_start); >> > + nr_pages = fault_around_pages(); >> > + /* race with fault_around_bytes_set() */ >> > + if (nr_pages <= 1) >> >> unlikely() ? > > Yep. > Btw, do we need this check at all? nr_pages can't be 0, and code below seems able to handle nr_page == 1. -- 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