Re: [PATCH v4] char/nvram: Remove redundant nvram_mutex

Tellakula Yeswanth Krishna
2026-04-02T16:03:49Z

On 30/03/26 4:05 pm, Venkat Rao Bagalkote wrote:
> The global nvram_mutex in drivers/char/nvram.c is redundant and unused,
> and this triggers compiler warnings on some configurations.
>
> All platform-specific nvram operations already provide their own internal
> synchronization, meaning the wrapper-level mutex does not provide any
> additional safety.
>
> Remove the nvram_mutex definition along with all remaining lock/unlock
> users across PPC32, x86, and m68k code paths, and rely entirely on the
> per-architecture nvram implementations for locking.
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
> ---
Without Fix
===============
make -j 33 -s && make modules_install && make install
In file included from ./include/linux/seqlock.h:20,
from ./include/linux/mmzone.h:17,
from ./include/linux/gfp.h:7,
from ./include/linux/umh.h:4,
from ./include/linux/kmod.h:9,
from ./include/linux/module.h:18,
from drivers/char/nvram.c:34:
drivers/char/nvram.c:56:21: warning: 'nvram_mutex' defined but not used
[-Wunused-variable]
56 | static DEFINE_MUTEX(nvram_mutex);
|                     ^~~~~~~~~~~
./include/linux/mutex.h:87:22: note: in definition of macro 'DEFINE_MUTEX'
87 |         struct mutex mutexname = __MUTEX_INITIALIZER(mutexname)
|                      ^~~~~~~~~

With this patch issue is fixed

Please add below tag

yeswanth <yeswanth@linux.ibm.com>

Thanks,

Yeswanth Krishna

> v4:
> - Remove all remaining nvram_mutex call sites, completing the mutex removal
>
> v3:
> - Removed global nvram_mutex definition
>
> drivers/char/nvram.c | 16 +++-------------
> 1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
> index 9eff426a9286..e89cc1f1c89e 100644
> --- a/drivers/char/nvram.c
> +++ b/drivers/char/nvram.c
> @@ -53,7 +53,6 @@
> #include <asm/nvram.h>
> #endif
>
> -static DEFINE_MUTEX(nvram_mutex);
> static DEFINE_SPINLOCK(nvram_state_lock);
> static int nvram_open_cnt; /* #times opened */
> static int nvram_open_mode; /* special open modes */
> @@ -310,11 +309,8 @@ static long nvram_misc_ioctl(struct file *file, unsigned int cmd,
> break;
> #ifdef CONFIG_PPC32
> case IOC_NVRAM_SYNC:
> - if (ppc_md.nvram_sync != NULL) {
> - mutex_lock(&nvram_mutex);
> + if (ppc_md.nvram_sync)
> ppc_md.nvram_sync();
> - mutex_unlock(&nvram_mutex);
> - }
> ret = 0;
> break;
> #endif
> @@ -324,11 +320,8 @@ static long nvram_misc_ioctl(struct file *file, unsigned int cmd,
> if (!capable(CAP_SYS_ADMIN))
> return -EACCES;
>
> - if (arch_nvram_ops.initialize != NULL) {
> - mutex_lock(&nvram_mutex);
> + if (arch_nvram_ops.initialize)
> ret = arch_nvram_ops.initialize();
> - mutex_unlock(&nvram_mutex);
> - }
> break;
> case NVRAM_SETCKS:
> /* just set checksum, contents unchanged (maybe useful after
> @@ -336,11 +329,8 @@ static long nvram_misc_ioctl(struct file *file, unsigned int cmd,
> if (!capable(CAP_SYS_ADMIN))
> return -EACCES;
>
> - if (arch_nvram_ops.set_checksum != NULL) {
> - mutex_lock(&nvram_mutex);
> + if (arch_nvram_ops.set_checksum)
> ret = arch_nvram_ops.set_checksum();
> - mutex_unlock(&nvram_mutex);
> - }
> break;
> #endif /* CONFIG_X86 || CONFIG_M68K */
> }