Received: by 2002:ac0:a582:0:0:0:0:0 with SMTP id m2-v6csp3341987imm; Mon, 8 Oct 2018 02:16:54 -0700 (PDT) X-Google-Smtp-Source: ACcGV63FOjqXTgX5/KI+nFBJnSnUjyFx9k7eqqq2HVtMacK6YyUdRQXyQR9MJTE3jo/XQ+h73si+ X-Received: by 2002:a62:d841:: with SMTP id e62-v6mr24141618pfg.60.1538990214237; Mon, 08 Oct 2018 02:16:54 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1538990214; cv=none; d=google.com; s=arc-20160816; b=CB9c/gcIw6ZqxY08kBHTJReO6yaeionOr93RUEtWXDILKDTO9RZ4mlSbeyisJrWGxX JGSPIbjYnxU5TJII4AGGgWwO96HzF/lVw3KCleKindk4XHJh89E/NyUmSauNJkYRZD0Q Ywo7F1FjYIiBC0xAn3m+rPpQTCagqxpEQj3a2Pwa82+MeARvaKX8lrDWPgEupXof6fpI Nx72zVETXTwr/fsJT+aEv7rXrOytd9mVtvSJd30O/zjn4S/2BiNIJ+b9gOOosLn7YF0Q aq7ordmS4ma6VKyTYVVbWueLDhzkEfkjYQQP98XtGWOHZANurckz3e4NU3N6cwgWYnXp /raA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:cc:to:subject :message-id:date:from:references:in-reply-to:mime-version :dkim-signature; bh=Mbli7CMmIfNi0JObzelbupTxAGlZUk3IRM+vMOHrQHk=; b=bCR0LanDcIqXyJcZrpVgqC66smdXT4uz7qHy4IJhgJKi5HxzGVK+nSDHmChCF9K61J rB4n+MleeBDFt/RgqzLJlWqaxztgLqyVDFbag27eAQoVJWWZb01Tj3JWYU0ElJGvzxNS MtIm2ZDKlnQNZntgzU42hKIqWIIEneHOH425GrE/lfwdEMoL7hU8FAAf2wkL/qOY3ivt VJpPrpGEN7ZQnVsTTVv9Y4nlvMnyAXITlRkvM2nF/TwGzo4gbHDYsM11qjoMRlESu3aN ZAouytVMmnEQffMx47thNISBTuVXxdXleWVJxJ0jblEMIMNC/pJxaBlKIqZy7PqHwQvE KRxw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=asJ90udZ; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id t13-v6si17463488ply.279.2018.10.08.02.16.38; Mon, 08 Oct 2018 02:16:54 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=asJ90udZ; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727484AbeJHQ1D (ORCPT + 99 others); Mon, 8 Oct 2018 12:27:03 -0400 Received: from mail-io1-f67.google.com ([209.85.166.67]:44522 "EHLO mail-io1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727065AbeJHQ1D (ORCPT ); Mon, 8 Oct 2018 12:27:03 -0400 Received: by mail-io1-f67.google.com with SMTP id x26-v6so15289861iog.11 for ; Mon, 08 Oct 2018 02:16:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=Mbli7CMmIfNi0JObzelbupTxAGlZUk3IRM+vMOHrQHk=; b=asJ90udZ2VHW+r/A3ESWzsXPn1cEukla8aTu9cQ7k6MZp/ofoyTmka9OU6ZcotegMe /zWfCbghZ50V7y/V6x6htEAksvSqwgN/5TtacoNKOfv+2gZ6gZImDVRQJeE/Abf+OUIO 5b1LnsOKTDscf2JFlbL2Dom6qQkIF+qzgAMr74CZ5JkE2+OllhuWaiUoUpL2l+gOMwji Pz58cLdBlz0cUE5TAC6znI68/GD3tClXkUboKmpzLfBjLYmg8uiglofpChB9pMtqMYZm Evq2NvTIsvCFGU+UU17/mbWCMKjZHA3sz5+8xo+yAMokg7wtYilfmgfzB/iv5/Pse37c ur8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=Mbli7CMmIfNi0JObzelbupTxAGlZUk3IRM+vMOHrQHk=; b=lqe9IAAD4SuPPgOjvgYU/avJijUJaOFkIzBj9FYITCyNQSxpFLHH1c8jAKfP76ZRqo R9D++p9IXhcRNrEnWQERAV+LgEkIOyRM2zW9+BjaptdsnUbmYEL/yyLZNKbWhI9M6jG4 WvoCjx6JFJvVsU2KrrV0BBxCqZVfBrD8XM3HAfNXXusGm8tUPkLd3J5debslO8hCxQDI pN7Jpy0425ubUA4KBquDM2luTxVhzdi7bfeIyktTznapsfLQvGC93pRW8M0PhIwe+/Ma Sw7Cr1Y2jC5Xivrs5nU7Z0ik2+JASbfjCfnjyV/ltT+sZV/J9xSDy3XG4IhXHvKuW2bJ M2tw== X-Gm-Message-State: ABuFfogPJEJOJetqyRdrreEtv4Ecp1tom80lS3fk5g7z0rzlB/Xfj0Os EoLnuxT+crTkZaGjkCFR0fTTxztcTNrjTQ/Kj1bN2A== X-Received: by 2002:a6b:6209:: with SMTP id f9-v6mr5392254iog.11.1538990178370; Mon, 08 Oct 2018 02:16:18 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a02:1003:0:0:0:0:0 with HTTP; Mon, 8 Oct 2018 02:15:57 -0700 (PDT) In-Reply-To: <20181005163320.zkacovxvlih6blpp@linutronix.de> References: <20180918152931.17322-1-williams@redhat.com> <20181005163018.icbknlzymwjhdehi@linutronix.de> <20181005163320.zkacovxvlih6blpp@linutronix.de> From: Dmitry Vyukov Date: Mon, 8 Oct 2018 11:15:57 +0200 Message-ID: Subject: Re: [PATCH] kasan: convert kasan/quarantine_lock to raw_spinlock To: Sebastian Andrzej Siewior Cc: Clark Williams , Alexander Potapenko , kasan-dev , Linux-MM , LKML , linux-rt-users@vger.kernel.org, Peter Zijlstra , Thomas Gleixner Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 5, 2018 at 6:33 PM, Sebastian Andrzej Siewior wrote: > On 2018-10-05 18:30:18 [+0200], To Clark Williams wrote: >> This is the minimum to get this working on RT splat free. There is one >> memory deallocation with irqs off which should work on RT in its current >> way. >> Once this and the on_each_cpu() invocation, I was wondering if=E2=80=A6 > > the patch at the bottom wouldn't work just fine for everyone. > It would have the beaty of annotating the locking scope a little and > avoiding the on_each_cpu() invocation. No local_irq_save() but actually > the proper locking primitives. > I haven't fully decoded the srcu part in the code. > > Wouldn't that work for you? Hi Sebastian, This seems to beak quarantine_remove_cache( ) in the sense that some object from the cache may still be in quarantine when quarantine_remove_cache() returns. When quarantine_remove_cache() returns all objects from the cache must be purged from quarantine. That srcu and irq trickery is there for a reason. This code is also on hot path of kmallock/kfree, an additional lock/unlock per operation is expensive. Adding 2 locked RMW per kmalloc is not something that should be done only out of refactoring reasons. The original message from Clark mentions that the problem can be fixed by just changing type of spinlock. This looks like a better and simpler way to resolve the problem to me. > Signed-off-by: Sebastian Andrzej Siewior > --- > mm/kasan/quarantine.c | 45 +++++++++++++++++++++++++------------------ > 1 file changed, 26 insertions(+), 19 deletions(-) > > diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c > index 3a8ddf8baf7dc..8ed595960e3c1 100644 > --- a/mm/kasan/quarantine.c > +++ b/mm/kasan/quarantine.c > @@ -39,12 +39,13 @@ > * objects inside of it. > */ > struct qlist_head { > + spinlock_t lock; > struct qlist_node *head; > struct qlist_node *tail; > size_t bytes; > }; > > -#define QLIST_INIT { NULL, NULL, 0 } > +#define QLIST_INIT {.head =3D NULL, .tail =3D NULL, .bytes =3D 0 } > > static bool qlist_empty(struct qlist_head *q) > { > @@ -95,7 +96,9 @@ static void qlist_move_all(struct qlist_head *from, str= uct qlist_head *to) > * The object quarantine consists of per-cpu queues and a global queue, > * guarded by quarantine_lock. > */ > -static DEFINE_PER_CPU(struct qlist_head, cpu_quarantine); > +static DEFINE_PER_CPU(struct qlist_head, cpu_quarantine) =3D { > + .lock =3D __SPIN_LOCK_UNLOCKED(cpu_quarantine.lock), > +}; > > /* Round-robin FIFO array of batches. */ > static struct qlist_head global_quarantine[QUARANTINE_BATCHES]; > @@ -183,12 +186,13 @@ void quarantine_put(struct kasan_free_meta *info, s= truct kmem_cache *cache) > * beginning which ensures that it either sees the objects in per= -cpu > * lists or in the global quarantine. > */ > - local_irq_save(flags); > + q =3D raw_cpu_ptr(&cpu_quarantine); > + spin_lock_irqsave(&q->lock, flags); > > - q =3D this_cpu_ptr(&cpu_quarantine); > qlist_put(q, &info->quarantine_link, cache->size); > if (unlikely(q->bytes > QUARANTINE_PERCPU_SIZE)) { > qlist_move_all(q, &temp); > + spin_unlock(&q->lock); > > spin_lock(&quarantine_lock); > WRITE_ONCE(quarantine_size, quarantine_size + temp.bytes)= ; > @@ -203,10 +207,10 @@ void quarantine_put(struct kasan_free_meta *info, s= truct kmem_cache *cache) > if (new_tail !=3D quarantine_head) > quarantine_tail =3D new_tail; > } > - spin_unlock(&quarantine_lock); > + spin_unlock_irqrestore(&quarantine_lock, flags); > + } else { > + spin_unlock_irqrestore(&q->lock, flags); > } > - > - local_irq_restore(flags); > } > > void quarantine_reduce(void) > @@ -284,21 +288,11 @@ static void qlist_move_cache(struct qlist_head *fro= m, > } > } > > -static void per_cpu_remove_cache(void *arg) > -{ > - struct kmem_cache *cache =3D arg; > - struct qlist_head to_free =3D QLIST_INIT; > - struct qlist_head *q; > - > - q =3D this_cpu_ptr(&cpu_quarantine); > - qlist_move_cache(q, &to_free, cache); > - qlist_free_all(&to_free, cache); > -} > - > /* Free all quarantined objects belonging to cache. */ > void quarantine_remove_cache(struct kmem_cache *cache) > { > unsigned long flags, i; > + unsigned int cpu; > struct qlist_head to_free =3D QLIST_INIT; > > /* > @@ -308,7 +302,20 @@ void quarantine_remove_cache(struct kmem_cache *cach= e) > * achieves the first goal, while synchronize_srcu() achieves the > * second. > */ > - on_each_cpu(per_cpu_remove_cache, cache, 1); > + /* get_online_cpus() invoked by caller */ > + for_each_online_cpu(cpu) { > + struct qlist_head *q; > + unsigned long flags; > + struct qlist_head to_free =3D QLIST_INIT; > + > + q =3D per_cpu_ptr(&cpu_quarantine, cpu); > + spin_lock_irqsave(&q->lock, flags); > + qlist_move_cache(q, &to_free, cache); > + spin_unlock_irqrestore(&q->lock, flags); > + > + qlist_free_all(&to_free, cache); > + > + } > > spin_lock_irqsave(&quarantine_lock, flags); > for (i =3D 0; i < QUARANTINE_BATCHES; i++) { > -- > 2.19.0 >