From patchwork Wed Nov 16 15:15:41 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 101510 Delivered-To: patch@linaro.org Received: by 10.140.97.165 with SMTP id m34csp209244qge; Wed, 16 Nov 2016 07:17:01 -0800 (PST) X-Received: by 10.98.24.136 with SMTP id 130mr5247382pfy.73.1479309421734; Wed, 16 Nov 2016 07:17:01 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id m6si32290031pgm.277.2016.11.16.07.17.01; Wed, 16 Nov 2016 07:17:01 -0800 (PST) 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; 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 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934116AbcKPPQ5 (ORCPT + 26 others); Wed, 16 Nov 2016 10:16:57 -0500 Received: from mout.kundenserver.de ([212.227.126.134]:57470 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932123AbcKPPQ4 (ORCPT ); Wed, 16 Nov 2016 10:16:56 -0500 Received: from wuerfel.lan ([78.43.21.235]) by mrelayeu.kundenserver.de (mreue004 [212.227.15.129]) with ESMTPA (Nemesis) id 0LeQ93-1cYt3X1Y9F-00q9SO; Wed, 16 Nov 2016 16:16:42 +0100 From: Arnd Bergmann To: Anton Vorontsov , Colin Cross , Kees Cook , Tony Luck Cc: Arnd Bergmann , Joel Fernandes , Namhyung Kim , Geliang Tang , linux-kernel@vger.kernel.org Subject: [PATCH] pstore: fix warning about conditional locking Date: Wed, 16 Nov 2016 16:15:41 +0100 Message-Id: <20161116151630.3235006-1-arnd@arndb.de> X-Mailer: git-send-email 2.9.0 MIME-Version: 1.0 X-Provags-ID: V03:K0:7y7+8uwpLtR3EI4pG2iR9YdwoG7lVi0Bz18TdDjuyX74zT55wH3 UpG8Mr3VzVlISx13oIJSNB3sjvVsEpo67/vBqnEHPTZZeNNQHerLx3pqAsc8fG9pXhPaugq 3Hm79OhGilpQOoUnMgv3d7BS0Zvg6bJz34F8kIHrGu+7FTimaZcbAjOqLrWEMjVawy5wTEn b0+WDehwzAqfON+s6VhfQ== X-UI-Out-Filterresults: notjunk:1; V01:K0:1i5rG6ylHpo=:52JI7vZWcTQ6rB3iitIXjo lgLnxnrA6rojZJ8u+qdMPj9W+Yqa/lrnsDr4lET/s0A8VWymjgTUuDWHLJp3K4uHc63vCqchC 3EoRfioHu0ejQJiBz2Uo3x7JkkKrSguV+LCurev7YFdvWy34Hs3F+K4SMkd9M1QnOTWQrQmRm GpPZVCkLKPSak5Yelmiy5nuIJ1svF4HM3j5BzOWoc3O0jUOAvyKZSSv5l/+p3PNru7eb4B1fn abYEBz6DKhSNpxG9L2OEJUYF1i1vr86chVppap/GiDWfQ4yDZ8n90i/Oyskxovln0/+zHuBuk hJcv56yq3vOPJAL3seqGncJLmQe4zpzqu97EbQmvTQw92kkILMNFx40P2l4LCcaG7OQ/zAi89 EQajZlo29ovTvFCnL/xkWvfkdLYmhuw6bM9/CyX78k10ygXkYtUTSM8GJeAXDa352zlAj6eDn OuloBLqndF9N3XBzK8yskku2HpPOm5AqVrxit4KnSOEA+HtSPbAe/6siVX1S/9lkCpN5uWAvT mlplzYM+hqatIs1vG7TmfEyfqeUS+Eo/gJIjZLlMQq93mm8IZ8vRFZcg58bBW+/XFQ/x2Gcfc H6Nbet2RXORg65DhOCDjE5YMiR3Q5bQzFc3DHBq0ca11VGszxGohCENCPlIGHzoOQs45Mawpy 9hQ0tqmo5WrvLOgeAvhjzUX919knNYkV6Q4yRDilUMFvgrmLXM47G3mq53YT4O2iEMJ7neaM/ F9TbQ47ng8QeZQay Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org gcc cannot prove that "prz->flags" remains constant over the function, so it warns that we might restore flags that have never been saved: fs/pstore/ram_core.c: In function ‘buffer_size_add’: include/linux/spinlock.h:246:3: error: ‘flags’ may be used uninitialized in this function [-Werror=maybe-uninitialized] This reworks the code to avoid the spinlock entirely, by actually using the atomic_t for atomic updates. The resulting code is more efficient and easier to understand, and it avoids the local variable that the compiler did not track correctly. Fixes: 95937ddce59a ("pstore: Allow prz to control need for locking") Signed-off-by: Arnd Bergmann --- fs/pstore/ram.c | 4 +--- fs/pstore/ram_core.c | 46 ++++++++++++++++++---------------------------- include/linux/pstore_ram.h | 8 -------- 3 files changed, 19 insertions(+), 39 deletions(-) -- 2.9.0 diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 380222432eff..cb66ba710349 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -787,9 +787,7 @@ static int ramoops_probe(struct platform_device *pdev) : 1; err = ramoops_init_przs("ftrace", dev, cxt, &cxt->fprzs, &paddr, cxt->ftrace_size, -1, - &cxt->max_ftrace_cnt, LINUX_VERSION_CODE, - (cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) - ? PRZ_FLAG_NO_LOCK : 0); + &cxt->max_ftrace_cnt, LINUX_VERSION_CODE, 0); if (err) goto fail_init_fprz; diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index 0cc23cb18719..87be7bdfaf48 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -51,21 +51,17 @@ static inline size_t buffer_start(struct persistent_ram_zone *prz) /* increase and wrap the start pointer, returning the old value */ static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a) { - int old; - int new; - unsigned long flags; - - if (!(prz->flags & PRZ_FLAG_NO_LOCK)) - raw_spin_lock_irqsave(&prz->buffer_lock, flags); + int old, new, before; old = atomic_read(&prz->buffer->start); - new = old + a; - while (unlikely(new >= prz->buffer_size)) - new -= prz->buffer_size; - atomic_set(&prz->buffer->start, new); + do { + new = old + a; + while (unlikely(new >= prz->buffer_size)) + new -= prz->buffer_size; - if (!(prz->flags & PRZ_FLAG_NO_LOCK)) - raw_spin_unlock_irqrestore(&prz->buffer_lock, flags); + before = old; + old = atomic_cmpxchg(&prz->buffer->start, before, new); + } while (old != before); return old; } @@ -73,25 +69,20 @@ static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a) /* increase the size counter until it hits the max size */ static void buffer_size_add(struct persistent_ram_zone *prz, size_t a) { - size_t old; - size_t new; - unsigned long flags; - - if (!(prz->flags & PRZ_FLAG_NO_LOCK)) - raw_spin_lock_irqsave(&prz->buffer_lock, flags); + size_t old, new, before; old = atomic_read(&prz->buffer->size); - if (old == prz->buffer_size) - goto exit; + do { + if (old == prz->buffer_size) + break; - new = old + a; - if (new > prz->buffer_size) - new = prz->buffer_size; - atomic_set(&prz->buffer->size, new); + new = old + a; + if (new > prz->buffer_size) + new = prz->buffer_size; -exit: - if (!(prz->flags & PRZ_FLAG_NO_LOCK)) - raw_spin_unlock_irqrestore(&prz->buffer_lock, flags); + before = old; + old = atomic_cmpxchg(&prz->buffer->size, before, new); + } while (old != before); } static void notrace persistent_ram_encode_rs8(struct persistent_ram_zone *prz, @@ -496,7 +487,6 @@ 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); prz->flags = flags; return 0; diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h index 9395f06e8372..e755803ba58a 100644 --- a/include/linux/pstore_ram.h +++ b/include/linux/pstore_ram.h @@ -24,13 +24,6 @@ #include #include -/* - * Choose whether access to the RAM zone requires locking or not. If a zone - * can be written to from different CPUs like with ftrace for example, then - * PRZ_FLAG_NO_LOCK is used. For all other cases, locking is required. - */ -#define PRZ_FLAG_NO_LOCK BIT(0) - struct persistent_ram_buffer; struct rs_control; @@ -48,7 +41,6 @@ struct persistent_ram_zone { struct persistent_ram_buffer *buffer; size_t buffer_size; u32 flags; - raw_spinlock_t buffer_lock; /* ECC correction */ char *par_buffer;