Message ID | 20210405101440.14937-1-zhengyongjun3@huawei.com |
---|---|
State | New |
Headers | show |
Series | [-next] backlight: backlight: Use DEFINE_MUTEX() for mutex lock | expand |
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 <hulkci@huawei.com> > Signed-off-by: Zheng Yongjun <zhengyongjun3@huawei.com> 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. > --- > 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.
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); return 0;
mutex lock can be initialized automatically with DEFINE_MUTEX() rather than explicitly calling mutex_init(). Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Zheng Yongjun <zhengyongjun3@huawei.com> --- drivers/video/backlight/backlight.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)