Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752578AbcJJXvT (ORCPT ); Mon, 10 Oct 2016 19:51:19 -0400 Received: from mail-lf0-f51.google.com ([209.85.215.51]:33206 "EHLO mail-lf0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752443AbcJJXvS (ORCPT ); Mon, 10 Oct 2016 19:51:18 -0400 MIME-Version: 1.0 In-Reply-To: <1475904515-24970-2-git-send-email-joelaf@google.com> References: <1475904515-24970-1-git-send-email-joelaf@google.com> <1475904515-24970-2-git-send-email-joelaf@google.com> From: Kees Cook Date: Mon, 10 Oct 2016 16:44:55 -0700 X-Google-Sender-Auth: bfRmgqJVKgMkwnJ7zebfuWuHEGM Message-ID: Subject: Re: [PATCH 1/7] pstore: Make spinlock per zone instead of global To: Joel Fernandes Cc: LKML , Steven Rostedt , Anton Vorontsov , Colin Cross , Tony Luck Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3301 Lines: 96 On Fri, Oct 7, 2016 at 10:28 PM, Joel Fernandes wrote: > Currently pstore has a global spinlock for all zones. Since the zones are > independent and modify different areas of memory, there's no need to have > a global lock, make the lock per-zone to protect the respective zone. This seems fine to me, though I'd like the commit message to at least hint at what's next, and why this change is needed, rather than just the general organizational justification. -Kees > > Signed-off-by: Joel Fernandes > --- > fs/pstore/ram_core.c | 11 +++++------ > include/linux/pstore_ram.h | 1 + > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c > index 3975dee..cb92055 100644 > --- a/fs/pstore/ram_core.c > +++ b/fs/pstore/ram_core.c > @@ -48,8 +48,6 @@ static inline size_t buffer_start(struct persistent_ram_zone *prz) > return atomic_read(&prz->buffer->start); > } > > -static DEFINE_RAW_SPINLOCK(buffer_lock); > - > /* increase and wrap the start pointer, returning the old value */ > static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a) > { > @@ -57,7 +55,7 @@ static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a) > int new; > unsigned long flags; > > - raw_spin_lock_irqsave(&buffer_lock, flags); > + raw_spin_lock_irqsave(&prz->buffer_lock, flags); > > old = atomic_read(&prz->buffer->start); > new = old + a; > @@ -65,7 +63,7 @@ static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a) > new -= prz->buffer_size; > atomic_set(&prz->buffer->start, new); > > - raw_spin_unlock_irqrestore(&buffer_lock, flags); > + raw_spin_unlock_irqrestore(&prz->buffer_lock, flags); > > return old; > } > @@ -77,7 +75,7 @@ static void buffer_size_add(struct persistent_ram_zone *prz, size_t a) > size_t new; > unsigned long flags; > > - raw_spin_lock_irqsave(&buffer_lock, flags); > + raw_spin_lock_irqsave(&prz->buffer_lock, flags); > > old = atomic_read(&prz->buffer->size); > if (old == prz->buffer_size) > @@ -89,7 +87,7 @@ static void buffer_size_add(struct persistent_ram_zone *prz, size_t a) > atomic_set(&prz->buffer->size, new); > > exit: > - raw_spin_unlock_irqrestore(&buffer_lock, flags); > + raw_spin_unlock_irqrestore(&prz->buffer_lock, flags); > } > > static void notrace persistent_ram_encode_rs8(struct persistent_ram_zone *prz, > @@ -493,6 +491,7 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig, > > prz->buffer->sig = sig; > persistent_ram_zap(prz); > + prz->buffer_lock = __RAW_SPIN_LOCK_UNLOCKED(buffer_lock); > > return 0; > } > diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h > index c668c86..244d242 100644 > --- a/include/linux/pstore_ram.h > +++ b/include/linux/pstore_ram.h > @@ -40,6 +40,7 @@ struct persistent_ram_zone { > void *vaddr; > struct persistent_ram_buffer *buffer; > size_t buffer_size; > + raw_spinlock_t buffer_lock; > > /* ECC correction */ > char *par_buffer; > -- > 2.7.4 > -- Kees Cook Nexus Security