linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Jesper Dangaard Brouer <netdev@brouer.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Hugh Dickins <hughd@google.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@suse.cz>,
	David Rientjes <rientjes@google.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Christoph Lameter <cl@linux.com>
Subject: Re: [PATCHv3 4/5] mm: make compound_head() robust
Date: Mon, 24 Aug 2015 12:29:42 +0300	[thread overview]
Message-ID: <20150824092942.GA1994@node.dhcp.inet.fi> (raw)
In-Reply-To: <20150824015945.58b25f3a@brouer.com>

On Mon, Aug 24, 2015 at 01:59:45AM +0200, Jesper Dangaard Brouer wrote:
> On Wed, 19 Aug 2015 12:21:45 +0300
> "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote:
> 
> > Hugh has pointed that compound_head() call can be unsafe in some
> > context. There's one example:
> > 
> [...]
> 
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 0735bc0a351a..a4c4b7d07473 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> 
> [...]
> > -/*
> > - * If we access compound page synchronously such as access to
> > - * allocated page, there is no need to handle tail flag race, so we can
> > - * check tail flag directly without any synchronization primitive.
> > - */
> > -static inline struct page *compound_head_fast(struct page *page)
> > -{
> > -	if (unlikely(PageTail(page)))
> > -		return page->first_page;
> > -	return page;
> > -}
> > -
> [...]
> 
> > @@ -548,13 +508,7 @@ static inline struct page *virt_to_head_page(const void *x)
> >  {
> >  	struct page *page = virt_to_page(x);
> >  
> > -	/*
> > -	 * We don't need to worry about synchronization of tail flag
> > -	 * when we call virt_to_head_page() since it is only called for
> > -	 * already allocated page and this page won't be freed until
> > -	 * this virt_to_head_page() is finished. So use _fast variant.
> > -	 */
> > -	return compound_head_fast(page);
> > +	return compound_head(page);
> >  }
> 
> I hope this does not slow down the SLAB/slub allocator?
> (which calls virt_to_head_page() frequently)

It should be slightly faster.

Before:

00002e90 <test_virt_to_head_page>:
    2e90:	8b 15 00 00 00 00    	mov    0x0,%edx
    2e96:	05 00 00 00 40       	add    $0x40000000,%eax
    2e9b:	c1 e8 0c             	shr    $0xc,%eax
    2e9e:	c1 e0 05             	shl    $0x5,%eax
    2ea1:	01 d0                	add    %edx,%eax
    2ea3:	8b 10                	mov    (%eax),%edx
    2ea5:	f6 c6 80             	test   $0x80,%dh
    2ea8:	75 06                	jne    2eb0 <test_virt_to_head_page+0x20>
    2eaa:	c3                   	ret    
    2eab:	90                   	nop
    2eac:	8d 74 26 00          	lea    0x0(%esi,%eiz,1),%esi
    2eb0:	8b 40 1c             	mov    0x1c(%eax),%eax
    2eb3:	c3                   	ret    

After:

00003070 <test_virt_to_head_page>:
    3070:	8b 15 00 00 00 00    	mov    0x0,%edx
    3076:	05 00 00 00 40       	add    $0x40000000,%eax
    307b:	c1 e8 0c             	shr    $0xc,%eax
    307e:	c1 e0 05             	shl    $0x5,%eax
    3081:	01 d0                	add    %edx,%eax
    3083:	8b 50 14             	mov    0x14(%eax),%edx
    3086:	8d 4a ff             	lea    -0x1(%edx),%ecx
    3089:	f6 c2 01             	test   $0x1,%dl
    308c:	0f 45 c1             	cmovne %ecx,%eax
    308f:	c3                   	ret    

-- 
 Kirill A. Shutemov

--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2015-08-24  9:29 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-19  9:21 [PATCHv3 0/5] Fix compound_head() race Kirill A. Shutemov
2015-08-19  9:21 ` [PATCHv3 1/5] mm: drop page->slab_page Kirill A. Shutemov
2015-08-24 14:59   ` Vlastimil Babka
2015-08-24 15:02   ` Vlastimil Babka
2015-08-25 17:24     ` Kirill A. Shutemov
2015-08-19  9:21 ` [PATCHv3 2/5] zsmalloc: use page->private instead of page->first_page Kirill A. Shutemov
2015-08-24 15:04   ` Vlastimil Babka
2015-08-19  9:21 ` [PATCHv3 3/5] mm: pack compound_dtor and compound_order into one word in struct page Kirill A. Shutemov
2015-08-20 23:26   ` Andrew Morton
2015-08-21  7:13     ` Michal Hocko
2015-08-21 10:40       ` Kirill A. Shutemov
2015-08-21 10:51         ` Michal Hocko
2015-08-19  9:21 ` [PATCHv3 4/5] mm: make compound_head() robust Kirill A. Shutemov
2015-08-20 23:36   ` Andrew Morton
2015-08-21 12:10     ` Kirill A. Shutemov
2015-08-21 16:11       ` Christoph Lameter
2015-08-21 19:31         ` Kirill A. Shutemov
2015-08-21 19:34           ` Andrew Morton
2015-08-21 21:15             ` Christoph Lameter
2015-08-24 15:49             ` Vlastimil Babka
2015-08-25 11:44       ` Vlastimil Babka
2015-08-25 18:33         ` Kirill A. Shutemov
2015-08-25 20:11           ` Paul E. McKenney
2015-08-25 20:46             ` Vlastimil Babka
2015-08-25 21:19               ` Paul E. McKenney
2015-08-26 15:04                 ` Kirill A. Shutemov
2015-08-26 15:39                   ` Vlastimil Babka
2015-08-26 16:38                   ` Paul E. McKenney
2015-08-26 18:18                 ` Hugh Dickins
2015-08-26 21:29                   ` Paul E. McKenney
2015-08-26 22:28                     ` Hugh Dickins
2015-08-26 23:34                       ` Paul E. McKenney
2015-08-27 15:09                     ` Michal Hocko
2015-08-27 16:03                       ` Michal Hocko
2015-08-27 17:28                         ` Hugh Dickins
2015-08-27 18:06                           ` Michal Hocko
2015-08-27 16:36                       ` Paul E. McKenney
2015-08-27 18:14                         ` Michal Hocko
2015-08-27 19:01                           ` Paul E. McKenney
2015-08-23 23:59   ` Jesper Dangaard Brouer
2015-08-24  9:29     ` Kirill A. Shutemov [this message]
2015-08-24 10:17   ` Kirill A. Shutemov
2015-08-19  9:21 ` [PATCHv3 5/5] mm: use 'unsigned int' for page order Kirill A. Shutemov
2015-08-20  8:32   ` Michal Hocko
2015-08-20 12:31 ` [PATCHv3 0/5] Fix compound_head() race Kirill A. Shutemov
2015-08-20 23:38   ` Andrew Morton
2015-08-22 20:13     ` Hugh Dickins
2015-08-24  9:36       ` Kirill A. Shutemov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150824092942.GA1994@node.dhcp.inet.fi \
    --to=kirill@shutemov.name \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=dave.hansen@intel.com \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=netdev@brouer.com \
    --cc=rientjes@google.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).