Re: [PATCH] chardev: Simplify usage of try_module_get()

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 1457 bytes --]

On Mon, Oct 16, 2023 at 11:43:11PM +0100, Al Viro wrote:
> On Fri, Oct 13, 2023 at 03:24:42PM +0200, Uwe Kleine-König wrote:
> > try_module_get(NULL) is true, so there is no need to check owner being
> > NULL.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  fs/char_dev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/char_dev.c b/fs/char_dev.c
> > index 950b6919fb87..6ba032442b39 100644
> > --- a/fs/char_dev.c
> > +++ b/fs/char_dev.c
> > @@ -350,7 +350,7 @@ static struct kobject *cdev_get(struct cdev *p)
> >  	struct module *owner = p->owner;
> >  	struct kobject *kobj;
> >  
> > -	if (owner && !try_module_get(owner))
> > +	if (!try_module_get(owner))
> >  		return NULL;
> >  	kobj = kobject_get_unless_zero(&p->kobj);
> >  	if (!kobj)
> 
> I wouldn't mind that, if that logics in try_module_get() was inlined.
> It isn't...

I don't understand what you intend to say here. What is "that"? Are you
talking about

	owner && !try_module_get(owner)

vs

	!try_module_get(owner)

or the following line with kobject_get_unless_zero? Do you doubt the
validity of my patch, or is it about something try_module_get()
could/should do more than it currently does?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]