Linux-Next Archive on lore.kernel.org
 help / color / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>,
	akpm@linux-foundation.org,
	Hans-Peter Nilsson <hans-peter.nilsson@axis.com>,
	starvik@axis.com, jespern@axis.com, hughd@google.com,
	kirill.shutemov@linux.intel.com, linux-next@vger.kernel.org,
	linux-kernel@vger.kernel.org, minchan@kernel.org,
	linux-cris-kernel@axis.com
Subject: Re: crisv32 runtime failure in -next due to 'page-flags: define behavior SL*B-related flags on compound pages'
Date: Wed, 23 Sep 2015 21:45:16 -0700
Message-ID: <20150924044516.GF4029@linux.vnet.ibm.com> (raw)
In-Reply-To: <5602BF14.8040803@roeck-us.net>

On Wed, Sep 23, 2015 at 08:02:44AM -0700, Guenter Roeck wrote:
> On 09/23/2015 03:53 AM, Kirill A. Shutemov wrote:
> >On Tue, Sep 22, 2015 at 08:40:14AM -0700, Paul E. McKenney wrote:
> >>On Tue, Sep 22, 2015 at 06:31:04PM +0300, Kirill A. Shutemov wrote:
> >>>On Tue, Sep 22, 2015 at 08:18:35AM -0700, Paul E. McKenney wrote:
> >>>>On Tue, Sep 22, 2015 at 03:57:06PM +0200, Hans-Peter Nilsson wrote:
> >>>>>I guess you hit the right spot, but I'd think people would be
> >>>>>more comfortable with aligning to sizeof (void *).
> >>>>
> >>>>I would indeed prefer sizeof(void *).
> >>>
> >>>Do you prefer to have the attribute set for whole structure or for ->next?
> >>>I think attribute on ->next is more appropriate from documentation POV.
> >
> >I retract this claim: we have requirement about pointee alignment, not
> >pointer alignment.
> >
> >>From edbab9e89f5e4ad42e63d93ab05519e6a5f4d552 Mon Sep 17 00:00:00 2001
> >From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >Date: Wed, 23 Sep 2015 13:39:28 +0300
> >Subject: [PATCH] rcu: force alignment on struct callback_head/rcu_head
> >
> >This patch makes struct callback_head aligned to size of pointer. On
> >most architectures it happens naturally due ABI requirements, but some
> >architectures (like CRIS) have weird ABI and we need to ask it
> >explicitly.
> >
> >The alignment is required to guarantee that bits 0 and 1 of @next will
> >be clear under normal conditions -- as long as we use call_rcu(),
> >call_rcu_bh(), call_rcu_sched(), or call_srcu() to queue callback.
> >
> >This guarantee is important for few reasons:
> >  - future call_rcu_lazy() will make use of lower bits in the pointer;
> >  - the structure shares storage spacer in struct page with @compound_head,
> >    which encode PageTail() in bit 0. The guarantee is needed to avoid
> >    false-positive PageTail().
> >
> >False postive PageTail() caused crash on crisv32[1]. It happend due
> >misaligned task_struct->rcu, which was byte-aligned.
> >
> >[1] http://lkml.kernel.org/r/55FAEA67.9000102@roeck-us.net
> >
> >Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> >Reported-by: Guenter Roeck <linux@roeck-us.net>
> 
> Tested-by: Guenter Roeck <linux@roeck-us.net>
> 
> Hope the patch won't get lost since it was attached to an e-mail.
> Can it be added to the branch introducing the problem ?

Andrew Morton picked it up.  No idea where the problem was introduced.

						Thanx, Paul

> Thanks,
> Guenter
> 
> >---
> >  include/linux/types.h | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> >diff --git a/include/linux/types.h b/include/linux/types.h
> >index c314989d9158..70d8500bddf1 100644
> >--- a/include/linux/types.h
> >+++ b/include/linux/types.h
> >@@ -205,11 +205,25 @@ struct ustat {
> >   * struct callback_head - callback structure for use with RCU and task_work
> >   * @next: next update requests in a list
> >   * @func: actual update function to call after the grace period.
> >+ *
> >+ * The struct is aligned to size of pointer. On most architectures it happens
> >+ * naturally due ABI requirements, but some architectures (like CRIS) have
> >+ * weird ABI and we need to ask it explicitly.
> >+ *
> >+ * The alignment is required to guarantee that bits 0 and 1 of @next will be
> >+ * clear under normal conditions -- as long as we use call_rcu(),
> >+ * call_rcu_bh(), call_rcu_sched(), or call_srcu() to queue callback.
> >+ *
> >+ * This guarantee is important for few reasons:
> >+ *  - future call_rcu_lazy() will make use of lower bits in the pointer;
> >+ *  - the structure shares storage spacer in struct page with @compound_head,
> >+ *    which encode PageTail() in bit 0. The guarantee is needed to avoid
> >+ *    false-positive PageTail().
> >   */
> >  struct callback_head {
> >  	struct callback_head *next;
> >  	void (*func)(struct callback_head *head);
> >-};
> >+} __attribute__((aligned(sizeof(void *))));
> >  #define rcu_head callback_head
> >
> >  typedef void (*rcu_callback_t)(struct rcu_head *head);
> >
> 

  reply index

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-17 16:29 Guenter Roeck
2015-09-18 14:25 ` Kirill A. Shutemov
2015-09-18 14:53   ` Jesper Nilsson
2015-09-18 15:13     ` Guenter Roeck
2015-09-21 15:34       ` Kirill A. Shutemov
2015-09-22  1:17         ` Guenter Roeck
2015-09-22 12:03           ` Kirill A. Shutemov
2015-09-22 12:19             ` Mikael Starvik
2015-09-22 12:50               ` Hans-Peter Nilsson
2015-09-22 13:27                 ` Kirill A. Shutemov
2015-09-22 13:57                   ` Hans-Peter Nilsson
2015-09-22 15:18                     ` Paul E. McKenney
2015-09-22 15:31                       ` Kirill A. Shutemov
2015-09-22 15:40                         ` Paul E. McKenney
2015-09-23 10:53                           ` Kirill A. Shutemov
2015-09-23 15:02                             ` Guenter Roeck
2015-09-24  4:45                               ` Paul E. McKenney [this message]
2015-09-22 16:16                         ` Hans-Peter Nilsson
2015-09-22 16:39                           ` Paul E. McKenney

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=20150924044516.GF4029@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=hans-peter.nilsson@axis.com \
    --cc=hughd@google.com \
    --cc=jespern@axis.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kirill@shutemov.name \
    --cc=linux-cris-kernel@axis.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=minchan@kernel.org \
    --cc=starvik@axis.com \
    /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

Linux-Next Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-next/0 linux-next/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-next linux-next/ https://lore.kernel.org/linux-next \
		linux-next@vger.kernel.org
	public-inbox-index linux-next

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-next


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git