Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756719AbZLXJkU (ORCPT ); Thu, 24 Dec 2009 04:40:20 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754570AbZLXJkT (ORCPT ); Thu, 24 Dec 2009 04:40:19 -0500 Received: from bombadil.infradead.org ([18.85.46.34]:44403 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750971AbZLXJkS (ORCPT ); Thu, 24 Dec 2009 04:40:18 -0500 Subject: Re: [PATCH] futex: Fix ZERO_PAGE cause infinite loop From: Peter Zijlstra To: KOSAKI Motohiro Cc: Hugh Dickins , KAMEZAWA Hiroyuki , Nick Piggin , Ingo Molnar , LKML , Thomas Gleixner , Darren Hart In-Reply-To: <20091224182630.3DCB.A69D9226@jp.fujitsu.com> References: <20091224182630.3DCB.A69D9226@jp.fujitsu.com> Content-Type: text/plain; charset="UTF-8" Date: Thu, 24 Dec 2009 10:39:25 +0100 Message-ID: <1261647565.4937.177.camel@laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3386 Lines: 106 Added tglx and dvhart to the CC. On Thu, 2009-12-24 at 18:29 +0900, KOSAKI Motohiro wrote: > This patch need both futex and zero-page developer's ack. > ========================================================== > > commit a13ea5b7 (mm: reinstate ZERO_PAGE) made the unfortunate regression. > following test program never finish and waste 100% cpu time. > > At the making commit 38d47c1b7 (rely on get_user_pages() for shared > futexes). There isn't zero page in linux kernel. then, futex developers > thought gup retry is safe. but we reinstated zero page later... > > This patch fixes it. > > futex-zero.c > --------------------------------------------------------------------- > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > > int main(int argc, char **argv) > { > long page_size; > int ret; > void *buf; > > page_size = sysconf(_SC_PAGESIZE); > > buf = mmap(NULL, page_size, PROT_READ|PROT_WRITE, > MAP_PRIVATE|MAP_ANONYMOUS, 0, 0); > if (buf == (void *)-1) { > perror("mmap error.\n"); > exit(1); > } > > fprintf(stderr, "futex wait\n"); > ret = syscall( SYS_futex, buf, FUTEX_WAIT, 1, NULL, NULL, NULL); > if (ret != 0 && errno != EWOULDBLOCK) { > perror("futex error.\n"); > exit(1); > } > fprintf(stderr, "futex_wait: ret = %d, errno = %d\n", ret, errno); > > return 0; > } > --------------------------------------------------------------------- > > Signed-off-by: KOSAKI Motohiro > Cc: Hugh Dickins > Cc: KAMEZAWA Hiroyuki > Cc: Nick Piggin > Cc: Peter Zijlstra > Cc: Ingo Molnar > --- > include/linux/mm.h | 16 ++++++++++++++++ > kernel/futex.c | 3 +++ > mm/memory.c | 14 -------------- > 3 files changed, 19 insertions(+), 14 deletions(-) > diff --git a/kernel/futex.c b/kernel/futex.c > index 8e3c3ff..79b89cc 100644 > --- a/kernel/futex.c > +++ b/kernel/futex.c > @@ -254,6 +254,8 @@ again: > > page = compound_head(page); > lock_page(page); > + if (is_zero_pfn(page_to_pfn(page))) > + goto anon_key; > if (!page->mapping) { > unlock_page(page); > put_page(page); > @@ -268,6 +270,7 @@ again: > * the object not the particular process. > */ > if (PageAnon(page)) { > + anon_key: > key->both.offset |= FUT_OFF_MMSHARED; /* ref taken on mm */ > key->private.mm = mm; > key->private.address = address; Doesn't it make more sense to simply fail the futex op? What I mean is, all (?) futex ops assume userspace actually wrote something to the address in question to begin with, therefore it can never be the zero page. So it being the zero page means someone goofed up, and we should bail, no? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/