Messages in this thread |  | | | Date | Mon, 10 Sep 2012 17:44:38 +0300 | | From | "Kirill A. Shutemov" <> | | Subject | Re: [PATCH v2 10/10] thp: implement refcounting for huge zero page |
| |
On Mon, Sep 10, 2012 at 04:02:39PM +0200, Eric Dumazet wrote: > On Mon, 2012-09-10 at 16:13 +0300, Kirill A. Shutemov wrote: > > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > > > H. Peter Anvin doesn't like huge zero page which sticks in memory forever > > after the first allocation. Here's implementation of lockless refcounting > > for huge zero page. > > > ... > > > +static unsigned long get_huge_zero_page(void) > > +{ > > + struct page *zero_page; > > +retry: > > + if (likely(atomic_inc_not_zero(&huge_zero_refcount))) > > + return ACCESS_ONCE(huge_zero_pfn); > > + > > + zero_page = alloc_pages(GFP_TRANSHUGE | __GFP_ZERO, HPAGE_PMD_ORDER); > > + if (!zero_page) > > + return 0; > > + if (cmpxchg(&huge_zero_pfn, 0, page_to_pfn(zero_page))) { > > + __free_page(zero_page); > > + goto retry; > > + } > > This might break if preemption can happen here ? > > The second thread might loop forever because huge_zero_refcount is 0, > and huge_zero_pfn not zero.
I fail to see why the second thread might loop forever. Long time yes, but forever?
Yes, disabling preemption before alloc_pages() and enabling after atomic_set() looks reasonable. Thanks.
> > If preemption already disabled, a comment would be nice. > > > > + > > + /* We take additional reference here. It will be put back by shinker */ > > typo : shrinker
Thx.
> > + atomic_set(&huge_zero_refcount, 2); > > + return ACCESS_ONCE(huge_zero_pfn); > > +} > > + > > >
-- Kirill A. Shutemov [unhandled content-type:application/pgp-signature] |  |