Re: [PATCH] llist: Fix missing memory barrier

From: Mathieu Desnoyers
Date: Fri Feb 06 2015 - 17:16:31 EST


----- Original Message -----
> From: "Greg Kroah-Hartman" <gregkh@xxxxxxxxxxxxxxxxxxx>
> To: "Mathieu Desnoyers" <mathieu.desnoyers@xxxxxxxxxxxx>
> Cc: "Pranith Kumar" <bobby.prani@xxxxxxxxx>, "Huang Ying" <ying.huang@xxxxxxxxx>, "LKML"
> <linux-kernel@xxxxxxxxxxxxxxx>, "Paul McKenney" <paulmck@xxxxxxxxxxxxxxxxxx>, "David Howells" <dhowells@xxxxxxxxxx>
> Sent: Friday, February 6, 2015 10:03:59 AM
> Subject: Re: [PATCH] llist: Fix missing memory barrier
>
> On Fri, Feb 06, 2015 at 02:12:32PM +0000, Mathieu Desnoyers wrote:
> > ----- Original Message -----
> > > From: "Pranith Kumar" <bobby.prani@xxxxxxxxx>
> > > To: "Mathieu Desnoyers" <mathieu.desnoyers@xxxxxxxxxxxx>
> > > Cc: "Huang Ying" <ying.huang@xxxxxxxxx>, "LKML"
> > > <linux-kernel@xxxxxxxxxxxxxxx>, "Paul McKenney"
> > > <paulmck@xxxxxxxxxxxxxxxxxx>, "David Howells" <dhowells@xxxxxxxxxx>
> > > 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
> > > <mathieu.desnoyers@xxxxxxxxxxxx> 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 ?
>
> Yes, why? What code needs these new apis?

The subsystems using llist.h in master: IRQ, smp core code,
vmalloc, scsi, the block layer, some filesystems... and more.
grep for "llist.h" to see the complete list of users.

My question was mainly on how to do the fix process-wise: post-3.18,
it implies using the new lockless_dereference() API. pre-3.18, we
need to use smp_read_barrier_depends().

As Peter Hurley suggested, using the new API for master and 3.18
seems like the right approach. Then the backports to stable branches
can, if needed, use smp_read_barrier_depends() instead. Is that
OK with you ?

Thanks,

Mathieu


>
> confused,
>
> greg k-h
>

--
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/