* [PATCH] char: misc: Make the code for allocating minor in misc_register more concise
@ 2025-09-09 8:58 zhangjiao2
2025-09-09 9:07 ` Greg KH
0 siblings, 1 reply; 5+ messages in thread
From: zhangjiao2 @ 2025-09-09 8:58 UTC (permalink / raw)
To: arnd; +Cc: gregkh, linux-kernel, zhang jiao
From: zhang jiao <zhangjiao2@cmss.chinamobile.com>
There is no need to check the registered misc dev in misc_list.
If misc_minor_alloc failed, it meens the minor is already alloced
and the misc dev is linked in misc_list.
Signed-off-by: zhang jiao <zhangjiao2@cmss.chinamobile.com>
---
drivers/char/misc.c | 32 +++++++-------------------------
1 file changed, 7 insertions(+), 25 deletions(-)
diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index a0aae0fc7926..fc2f5e8b2f95 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -211,6 +211,7 @@ int misc_register(struct miscdevice *misc)
dev_t dev;
int err = 0;
bool is_dynamic = (misc->minor == MISC_DYNAMIC_MINOR);
+ int minor = 0;
if (misc->minor > MISC_DYNAMIC_MINOR) {
pr_err("Invalid fixed minor %d for miscdevice '%s'\n",
@@ -221,32 +222,13 @@ int misc_register(struct miscdevice *misc)
INIT_LIST_HEAD(&misc->list);
mutex_lock(&misc_mtx);
-
- if (is_dynamic) {
- int i = misc_minor_alloc(misc->minor);
-
- if (i < 0) {
- err = -EBUSY;
- goto out;
- }
- misc->minor = i;
- } else {
- struct miscdevice *c;
- int i;
-
- list_for_each_entry(c, &misc_list, list) {
- if (c->minor == misc->minor) {
- err = -EBUSY;
- goto out;
- }
- }
-
- i = misc_minor_alloc(misc->minor);
- if (i < 0) {
- err = -EBUSY;
- goto out;
- }
+ minor = misc_minor_alloc(misc->minor);
+ if (minor < 0) {
+ err = -EBUSY;
+ goto out;
}
+ if (is_dynamic)
+ misc->minor = minor;
dev = MKDEV(MISC_MAJOR, misc->minor);
--
2.33.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] char: misc: Make the code for allocating minor in misc_register more concise
2025-09-09 8:58 [PATCH] char: misc: Make the code for allocating minor in misc_register more concise zhangjiao2
@ 2025-09-09 9:07 ` Greg KH
2025-09-10 1:44 ` zhangjiao2
0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2025-09-09 9:07 UTC (permalink / raw)
To: zhangjiao2; +Cc: arnd, linux-kernel
On Tue, Sep 09, 2025 at 04:58:35PM +0800, zhangjiao2 wrote:
> From: zhang jiao <zhangjiao2@cmss.chinamobile.com>
>
> There is no need to check the registered misc dev in misc_list.
> If misc_minor_alloc failed, it meens the minor is already alloced
> and the misc dev is linked in misc_list.
>
> Signed-off-by: zhang jiao <zhangjiao2@cmss.chinamobile.com>
> ---
> drivers/char/misc.c | 32 +++++++-------------------------
> 1 file changed, 7 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/char/misc.c b/drivers/char/misc.c
> index a0aae0fc7926..fc2f5e8b2f95 100644
> --- a/drivers/char/misc.c
> +++ b/drivers/char/misc.c
> @@ -211,6 +211,7 @@ int misc_register(struct miscdevice *misc)
> dev_t dev;
> int err = 0;
> bool is_dynamic = (misc->minor == MISC_DYNAMIC_MINOR);
> + int minor = 0;
>
> if (misc->minor > MISC_DYNAMIC_MINOR) {
> pr_err("Invalid fixed minor %d for miscdevice '%s'\n",
> @@ -221,32 +222,13 @@ int misc_register(struct miscdevice *misc)
> INIT_LIST_HEAD(&misc->list);
>
> mutex_lock(&misc_mtx);
> -
> - if (is_dynamic) {
> - int i = misc_minor_alloc(misc->minor);
> -
> - if (i < 0) {
> - err = -EBUSY;
> - goto out;
> - }
> - misc->minor = i;
> - } else {
> - struct miscdevice *c;
> - int i;
> -
> - list_for_each_entry(c, &misc_list, list) {
> - if (c->minor == misc->minor) {
> - err = -EBUSY;
> - goto out;
> - }
> - }
> -
> - i = misc_minor_alloc(misc->minor);
> - if (i < 0) {
> - err = -EBUSY;
> - goto out;
> - }
> + minor = misc_minor_alloc(misc->minor);
> + if (minor < 0) {
> + err = -EBUSY;
> + goto out;
> }
> + if (is_dynamic)
> + misc->minor = minor;
>
> dev = MKDEV(MISC_MAJOR, misc->minor);
>
Does this pass the new test suite for the misc code allocation logic
that we now have in the tree? Or do we need to write a new test-case
for this codepath?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] char: misc: Make the code for allocating minor in misc_register more concise
2025-09-09 9:07 ` Greg KH
@ 2025-09-10 1:44 ` zhangjiao2
2025-10-07 12:45 ` Greg KH
2025-11-27 9:55 ` Greg KH
0 siblings, 2 replies; 5+ messages in thread
From: zhangjiao2 @ 2025-09-10 1:44 UTC (permalink / raw)
To: gregkh; +Cc: arnd, linux-kernel, zhangjiao2
>> On Tue, Sep 09, 2025 at 04:58:35PM +0800, zhangjiao2 wrote:
>> From: zhang jiao <zhangjiao2@cmss.chinamobile.com>
>>
>> There is no need to check the registered misc dev in misc_list.
>> If misc_minor_alloc failed, it meens the minor is already alloced
>> and the misc dev is linked in misc_list.
>>
>> Signed-off-by: zhang jiao <zhangjiao2@cmss.chinamobile.com>
>> ---
>> drivers/char/misc.c | 32 +++++++-------------------------
>> 1 file changed, 7 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/char/misc.c b/drivers/char/misc.c
>> index a0aae0fc7926..fc2f5e8b2f95 100644
>> --- a/drivers/char/misc.c
>> +++ b/drivers/char/misc.c
>> @@ -211,6 +211,7 @@ int misc_register(struct miscdevice *misc)
>> dev_t dev;
>> int err = 0;
>> bool is_dynamic = (misc->minor == MISC_DYNAMIC_MINOR);
>> + int minor = 0;
>>
>> if (misc->minor > MISC_DYNAMIC_MINOR) {
>> pr_err("Invalid fixed minor %d for miscdevice '%s'\n",
>> @@ -221,32 +222,13 @@ int misc_register(struct miscdevice *misc)
>> INIT_LIST_HEAD(&misc->list);
>>
>> mutex_lock(&misc_mtx);
>> -
>> - if (is_dynamic) {
>> - int i = misc_minor_alloc(misc->minor);
>> -
>> - if (i < 0) {
>> - err = -EBUSY;
>> - goto out;
>> - }
>> - misc->minor = i;
>> - } else {
>> - struct miscdevice *c;
>> - int i;
>> -
>> - list_for_each_entry(c, &misc_list, list) {
>> - if (c->minor == misc->minor) {
>> - err = -EBUSY;
>> - goto out;
>> - }
>> - }
>> -
>> - i = misc_minor_alloc(misc->minor);
>> - if (i < 0) {
>> - err = -EBUSY;
>> - goto out;
>> - }
>> + minor = misc_minor_alloc(misc->minor);
>> + if (minor < 0) {
>> + err = -EBUSY;
>> + goto out;
>> }
>> + if (is_dynamic)
>> + misc->minor = minor;
>>
>> dev = MKDEV(MISC_MAJOR, misc->minor);
>>
> Does this pass the new test suite for the misc code allocation logic
> that we now have in the tree? Or do we need to write a new test-case
> for this codepath?
I didn't take this new test suite, can you do it for me? I don't think there's
a need to write a new test-case for this codepath.
thanks,
zhang jiao
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] char: misc: Make the code for allocating minor in misc_register more concise
2025-09-10 1:44 ` zhangjiao2
@ 2025-10-07 12:45 ` Greg KH
2025-11-27 9:55 ` Greg KH
1 sibling, 0 replies; 5+ messages in thread
From: Greg KH @ 2025-10-07 12:45 UTC (permalink / raw)
To: zhangjiao2; +Cc: arnd, linux-kernel
On Wed, Sep 10, 2025 at 09:44:20AM +0800, zhangjiao2 wrote:
> >> On Tue, Sep 09, 2025 at 04:58:35PM +0800, zhangjiao2 wrote:
> >> From: zhang jiao <zhangjiao2@cmss.chinamobile.com>
> >>
> >> There is no need to check the registered misc dev in misc_list.
> >> If misc_minor_alloc failed, it meens the minor is already alloced
> >> and the misc dev is linked in misc_list.
> >>
> >> Signed-off-by: zhang jiao <zhangjiao2@cmss.chinamobile.com>
> >> ---
> >> drivers/char/misc.c | 32 +++++++-------------------------
> >> 1 file changed, 7 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/drivers/char/misc.c b/drivers/char/misc.c
> >> index a0aae0fc7926..fc2f5e8b2f95 100644
> >> --- a/drivers/char/misc.c
> >> +++ b/drivers/char/misc.c
> >> @@ -211,6 +211,7 @@ int misc_register(struct miscdevice *misc)
> >> dev_t dev;
> >> int err = 0;
> >> bool is_dynamic = (misc->minor == MISC_DYNAMIC_MINOR);
> >> + int minor = 0;
> >>
> >> if (misc->minor > MISC_DYNAMIC_MINOR) {
> >> pr_err("Invalid fixed minor %d for miscdevice '%s'\n",
> >> @@ -221,32 +222,13 @@ int misc_register(struct miscdevice *misc)
> >> INIT_LIST_HEAD(&misc->list);
> >>
> >> mutex_lock(&misc_mtx);
> >> -
> >> - if (is_dynamic) {
> >> - int i = misc_minor_alloc(misc->minor);
> >> -
> >> - if (i < 0) {
> >> - err = -EBUSY;
> >> - goto out;
> >> - }
> >> - misc->minor = i;
> >> - } else {
> >> - struct miscdevice *c;
> >> - int i;
> >> -
> >> - list_for_each_entry(c, &misc_list, list) {
> >> - if (c->minor == misc->minor) {
> >> - err = -EBUSY;
> >> - goto out;
> >> - }
> >> - }
> >> -
> >> - i = misc_minor_alloc(misc->minor);
> >> - if (i < 0) {
> >> - err = -EBUSY;
> >> - goto out;
> >> - }
> >> + minor = misc_minor_alloc(misc->minor);
> >> + if (minor < 0) {
> >> + err = -EBUSY;
> >> + goto out;
> >> }
> >> + if (is_dynamic)
> >> + misc->minor = minor;
> >>
> >> dev = MKDEV(MISC_MAJOR, misc->minor);
> >>
>
> > Does this pass the new test suite for the misc code allocation logic
> > that we now have in the tree? Or do we need to write a new test-case
> > for this codepath?
>
> I didn't take this new test suite, can you do it for me?
Please test your changes first, don't make maintainers do it for you :)
> I don't think there's
> a need to write a new test-case for this codepath.
Why not? It seems to be a code path that is worth changing, so why not
test to verify you got it correct?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] char: misc: Make the code for allocating minor in misc_register more concise
2025-09-10 1:44 ` zhangjiao2
2025-10-07 12:45 ` Greg KH
@ 2025-11-27 9:55 ` Greg KH
1 sibling, 0 replies; 5+ messages in thread
From: Greg KH @ 2025-11-27 9:55 UTC (permalink / raw)
To: zhangjiao2; +Cc: arnd, linux-kernel
On Wed, Sep 10, 2025 at 09:44:20AM +0800, zhangjiao2 wrote:
> >> On Tue, Sep 09, 2025 at 04:58:35PM +0800, zhangjiao2 wrote:
> >> From: zhang jiao <zhangjiao2@cmss.chinamobile.com>
> >>
> >> There is no need to check the registered misc dev in misc_list.
> >> If misc_minor_alloc failed, it meens the minor is already alloced
> >> and the misc dev is linked in misc_list.
> >>
> >> Signed-off-by: zhang jiao <zhangjiao2@cmss.chinamobile.com>
> >> ---
> >> drivers/char/misc.c | 32 +++++++-------------------------
> >> 1 file changed, 7 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/drivers/char/misc.c b/drivers/char/misc.c
> >> index a0aae0fc7926..fc2f5e8b2f95 100644
> >> --- a/drivers/char/misc.c
> >> +++ b/drivers/char/misc.c
> >> @@ -211,6 +211,7 @@ int misc_register(struct miscdevice *misc)
> >> dev_t dev;
> >> int err = 0;
> >> bool is_dynamic = (misc->minor == MISC_DYNAMIC_MINOR);
> >> + int minor = 0;
> >>
> >> if (misc->minor > MISC_DYNAMIC_MINOR) {
> >> pr_err("Invalid fixed minor %d for miscdevice '%s'\n",
> >> @@ -221,32 +222,13 @@ int misc_register(struct miscdevice *misc)
> >> INIT_LIST_HEAD(&misc->list);
> >>
> >> mutex_lock(&misc_mtx);
> >> -
> >> - if (is_dynamic) {
> >> - int i = misc_minor_alloc(misc->minor);
> >> -
> >> - if (i < 0) {
> >> - err = -EBUSY;
> >> - goto out;
> >> - }
> >> - misc->minor = i;
> >> - } else {
> >> - struct miscdevice *c;
> >> - int i;
> >> -
> >> - list_for_each_entry(c, &misc_list, list) {
> >> - if (c->minor == misc->minor) {
> >> - err = -EBUSY;
> >> - goto out;
> >> - }
> >> - }
> >> -
> >> - i = misc_minor_alloc(misc->minor);
> >> - if (i < 0) {
> >> - err = -EBUSY;
> >> - goto out;
> >> - }
> >> + minor = misc_minor_alloc(misc->minor);
> >> + if (minor < 0) {
> >> + err = -EBUSY;
> >> + goto out;
> >> }
> >> + if (is_dynamic)
> >> + misc->minor = minor;
> >>
> >> dev = MKDEV(MISC_MAJOR, misc->minor);
> >>
>
> > Does this pass the new test suite for the misc code allocation logic
> > that we now have in the tree? Or do we need to write a new test-case
> > for this codepath?
>
> I didn't take this new test suite, can you do it for me? I don't think there's
> a need to write a new test-case for this codepath.
Please run it to verify it works properly with the new code paths you
have added here.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-11-27 9:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-09 8:58 [PATCH] char: misc: Make the code for allocating minor in misc_register more concise zhangjiao2
2025-09-09 9:07 ` Greg KH
2025-09-10 1:44 ` zhangjiao2
2025-10-07 12:45 ` Greg KH
2025-11-27 9:55 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.