Re: [PATCH] char: misc: Make the code for allocating minor in misc_register more concise

[Greg KH <gregkh@linuxfoundation.org>]

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