Re: [PATCH -next] backlight: backlight: Use DEFINE_MUTEX() for mutex lock
From: Daniel Thompson <hidden>
Date: 2021-04-06 08:51:46
Also in:
dri-devel, kernel-janitors
On Mon, Apr 05, 2021 at 06:14:40PM +0800, Zheng Yongjun wrote:
mutex lock can be initialized automatically with DEFINE_MUTEX() rather than explicitly calling mutex_init(). Reported-by: Hulk Robot <redacted> Signed-off-by: Zheng Yongjun <redacted>
This patch looks like a resend of this one (but with a different revision number): https://lkml.org/lkml/2020/12/24/229 A resend should always maintain the version number and be clearly marked as a resend. In this case, there is also a pending review comment that you have ignored. Given I also clarified when you asked (off-list) for additional details I'm very surprised to see this patch circulated again without modification. I have repeated the feedback below.
quoted hunk ↗ jump to hunk
--- drivers/video/backlight/backlight.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c index 537fe1b376ad..d7a09c422547 100644 --- a/drivers/video/backlight/backlight.c +++ b/drivers/video/backlight/backlight.c@@ -64,7 +64,7 @@ */ static struct list_head backlight_dev_list; -static struct mutex backlight_dev_list_mutex; +static DEFINE_MUTEX(backlight_dev_list_mutex); static struct blocking_notifier_head backlight_notifier; static const char *const backlight_types[] = {@@ -757,7 +757,6 @@ static int __init backlight_class_init(void) backlight_class->dev_groups = bl_device_groups; backlight_class->pm = &backlight_class_dev_pm_ops; INIT_LIST_HEAD(&backlight_dev_list); - mutex_init(&backlight_dev_list_mutex); BLOCKING_INIT_NOTIFIER_HEAD(&backlight_notifier);
On Mon, 4 Jan 2021 at 14:19, Daniel Thompson wrote: : the purpose of backlight_dev_list_mutex is (as the name suggests) to : protect backlight_dev_list. It makes no sense to initialize these : variables from different places within the code. It just makes it : harder to reason about the lifetimes of the variables. : : To be clear, switching over to static initializers is a good change, : but please change all three in one patch. Daniel.