All of lore.kernel.org
 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>

WARNING: multiple messages have this Message-ID (diff)
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

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

Thread overview: 96+ 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 ` Kirill A. Shutemov
2015-08-19  9:21 ` [PATCHv3 1/5] mm: drop page->slab_page Kirill A. Shutemov
2015-08-19  9:21   ` Kirill A. Shutemov
2015-08-24 14:59   ` Vlastimil Babka
2015-08-24 14:59     ` Vlastimil Babka
2015-08-24 15:02   ` Vlastimil Babka
2015-08-24 15:02     ` Vlastimil Babka
2015-08-25 17:24     ` Kirill A. Shutemov
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-19  9:21   ` Kirill A. Shutemov
2015-08-24 15:04   ` Vlastimil Babka
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-19  9:21   ` Kirill A. Shutemov
2015-08-20 23:26   ` Andrew Morton
2015-08-20 23:26     ` Andrew Morton
2015-08-21  7:13     ` Michal Hocko
2015-08-21  7:13       ` Michal Hocko
2015-08-21 10:40       ` Kirill A. Shutemov
2015-08-21 10:40         ` Kirill A. Shutemov
2015-08-21 10:51         ` Michal Hocko
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-19  9:21   ` Kirill A. Shutemov
2015-08-20 23:36   ` Andrew Morton
2015-08-20 23:36     ` Andrew Morton
2015-08-21 12:10     ` Kirill A. Shutemov
2015-08-21 12:10       ` Kirill A. Shutemov
2015-08-21 16:11       ` Christoph Lameter
2015-08-21 16:11         ` Christoph Lameter
2015-08-21 19:31         ` Kirill A. Shutemov
2015-08-21 19:31           ` Kirill A. Shutemov
2015-08-21 19:34           ` Andrew Morton
2015-08-21 19:34             ` Andrew Morton
2015-08-21 21:15             ` Christoph Lameter
2015-08-21 21:15               ` Christoph Lameter
2015-08-24 15:49             ` Vlastimil Babka
2015-08-24 15:49               ` Vlastimil Babka
2015-08-25 11:44       ` Vlastimil Babka
2015-08-25 11:44         ` Vlastimil Babka
2015-08-25 18:33         ` Kirill A. Shutemov
2015-08-25 18:33           ` Kirill A. Shutemov
2015-08-25 20:11           ` Paul E. McKenney
2015-08-25 20:11             ` Paul E. McKenney
2015-08-25 20:46             ` Vlastimil Babka
2015-08-25 20:46               ` Vlastimil Babka
2015-08-25 21:19               ` Paul E. McKenney
2015-08-25 21:19                 ` Paul E. McKenney
2015-08-26 15:04                 ` Kirill A. Shutemov
2015-08-26 15:04                   ` Kirill A. Shutemov
2015-08-26 15:39                   ` Vlastimil Babka
2015-08-26 15:39                     ` Vlastimil Babka
2015-08-26 16:38                   ` Paul E. McKenney
2015-08-26 16:38                     ` Paul E. McKenney
2015-08-26 18:18                 ` Hugh Dickins
2015-08-26 18:18                   ` Hugh Dickins
2015-08-26 21:29                   ` Paul E. McKenney
2015-08-26 21:29                     ` Paul E. McKenney
2015-08-26 22:28                     ` Hugh Dickins
2015-08-26 22:28                       ` Hugh Dickins
2015-08-26 23:34                       ` Paul E. McKenney
2015-08-26 23:34                         ` Paul E. McKenney
2015-08-27 15:09                     ` Michal Hocko
2015-08-27 15:09                       ` Michal Hocko
2015-08-27 16:03                       ` Michal Hocko
2015-08-27 16:03                         ` Michal Hocko
2015-08-27 17:28                         ` Hugh Dickins
2015-08-27 17:28                           ` Hugh Dickins
2015-08-27 18:06                           ` Michal Hocko
2015-08-27 18:06                             ` Michal Hocko
2015-08-27 16:36                       ` Paul E. McKenney
2015-08-27 16:36                         ` Paul E. McKenney
2015-08-27 18:14                         ` Michal Hocko
2015-08-27 18:14                           ` Michal Hocko
2015-08-27 19:01                           ` Paul E. McKenney
2015-08-27 19:01                             ` Paul E. McKenney
2015-08-23 23:59   ` Jesper Dangaard Brouer
2015-08-23 23:59     ` Jesper Dangaard Brouer
2015-08-24  9:29     ` Kirill A. Shutemov [this message]
2015-08-24  9:29       ` Kirill A. Shutemov
2015-08-24 10:17   ` Kirill A. Shutemov
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-19  9:21   ` Kirill A. Shutemov
2015-08-20  8:32   ` Michal Hocko
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 12:31   ` Kirill A. Shutemov
2015-08-20 23:38   ` Andrew Morton
2015-08-20 23:38     ` Andrew Morton
2015-08-22 20:13     ` Hugh Dickins
2015-08-22 20:13       ` Hugh Dickins
2015-08-24  9:36       ` Kirill A. Shutemov
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.