Messages in this thread Patch in this message | | | From | Arnd Bergmann <> | Subject | [PATCH] pstore: fix warning about conditional locking | Date | Wed, 16 Nov 2016 16:15:41 +0100 |
| |
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 <arnd@arndb.de> --- fs/pstore/ram.c | 4 +--- fs/pstore/ram_core.c | 46 ++++++++++++++++++---------------------------- include/linux/pstore_ram.h | 8 -------- 3 files changed, 19 insertions(+), 39 deletions(-)
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 <linux/list.h> #include <linux/types.h> -/* - * 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; -- 2.9.0
| |