Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757890AbbBFWSG (ORCPT ); Fri, 6 Feb 2015 17:18:06 -0500 Received: from mail.efficios.com ([78.47.125.74]:37867 "EHLO mail.efficios.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755134AbbBFWSE (ORCPT ); Fri, 6 Feb 2015 17:18:04 -0500 Date: Fri, 6 Feb 2015 22:18:09 +0000 (UTC) From: Mathieu Desnoyers To: Peter Hurley Cc: Pranith Kumar , Huang Ying , LKML , Paul McKenney , David Howells , Greg Kroah-Hartman Message-ID: <1490510464.88166.1423261089883.JavaMail.zimbra@efficios.com> In-Reply-To: <54D4DB11.6000306@hurleysoftware.com> References: <1423192017-16735-1-git-send-email-mathieu.desnoyers@efficios.com> <625236311.87350.1423231952716.JavaMail.zimbra@efficios.com> <54D4DB11.6000306@hurleysoftware.com> Subject: Re: [PATCH] llist: Fix missing memory barrier MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [173.246.22.116] X-Mailer: Zimbra 8.0.7_GA_6021 (ZimbraWebClient - FF35 (Linux)/8.0.7_GA_6021) Thread-Topic: llist: Fix missing memory barrier Thread-Index: LYAaQxlxAg8oF3zr86XQOYi02mCyBw== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4562 Lines: 136 ----- Original Message ----- > From: "Peter Hurley" > To: "Mathieu Desnoyers" , "Pranith Kumar" > Cc: "Huang Ying" , "LKML" , "Paul McKenney" > , "David Howells" , "Greg Kroah-Hartman" > > Sent: Friday, February 6, 2015 10:17:37 AM > Subject: Re: [PATCH] llist: Fix missing memory barrier > > On 02/06/2015 09:12 AM, Mathieu Desnoyers wrote: > > ----- Original Message ----- > >> From: "Pranith Kumar" > >> To: "Mathieu Desnoyers" > >> Cc: "Huang Ying" , "LKML" > >> , "Paul McKenney" > >> , "David Howells" > >> Sent: Thursday, February 5, 2015 10:44:07 PM > >> Subject: Re: [PATCH] llist: Fix missing memory barrier > >> > >> Hi Mathieu, > >> > >> On Thu, Feb 5, 2015 at 10:06 PM, Mathieu Desnoyers > >> wrote: > >>> A smp_read_barrier_depends() appears to be missing in llist_del_first(). > >>> It should only matter for Alpha in practice. Adding it after the check > >>> of entry against NULL allows skipping the barrier in a common case. > >> > >> We recently decided on using lockless_dereference() instead of > >> hard-coding smp_read_barrier_depends()[1]. The advantage is that > >> lockless_dereference() clearly shows what loads are being ordered. > >> Could you resend the patch using that API? > > > > Since llist.h has been introduced prior to 3.18, I'm wondering if > > it would be worthwhile to submit 2 patches for the purpose of > > backporting to stable branches: > > > > 1) Fix introducing smp_read_barrier_depends() (for master and > > stable branches) > > 2) Move master from smp_read_barrier_depends() to > > lockless_dereference(), > > > > Thoughts ? > > Other way around. > > The first patch should use lockless_dereference() for mainline. > > Then once that's been picked up and has a SHA, then a backport > patch for stable using smp_read_barrier_depends() instead > before lockless_dereference() was introduced. That sounds like a good approach. I'll wait for a final word from Greg and proceed this way unless there are objections. Thanks! Mathieu > > Regards, > Peter Hurley > > > Thanks! > > > > Mathieu > > > > > >> > >> Thanks! > >> > >> [1] http://lkml.iu.edu/hypermail/linux/kernel/1410.3/04561.html > >> > >>> > >>> Signed-off-by: Mathieu Desnoyers > >>> CC: Huang Ying > >>> CC: Paul McKenney > >>> CC: David Howells > >>> --- > >>> lib/llist.c | 7 +++++++ > >>> 1 file changed, 7 insertions(+) > >>> > >>> diff --git a/lib/llist.c b/lib/llist.c > >>> index f76196d..72861f3 100644 > >>> --- a/lib/llist.c > >>> +++ b/lib/llist.c > >>> @@ -26,6 +26,7 @@ > >>> #include > >>> #include > >>> #include > >>> +#include > >>> > >>> > >>> /** > >>> @@ -72,6 +73,12 @@ struct llist_node *llist_del_first(struct llist_head > >>> *head) > >>> if (entry == NULL) > >>> return NULL; > >>> old_entry = entry; > >>> + /* > >>> + * Load entry before entry->next. Matches the implicit > >>> + * memory barrier before the cmpxchg in > >>> llist_add_batch(), > >>> + * which ensures entry->next is stored before entry. > >>> + */ > >>> + smp_read_barrier_depends(); > >>> next = entry->next; > >>> entry = cmpxchg(&head->first, old_entry, next); > >>> if (entry == old_entry) > >>> -- > >>> 2.1.4 > >>> > >>> -- > >>> 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/ > >> > >> > >> > >> -- > >> Pranith > >> > > > > -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com -- 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/