Re: [PATCH 1/3] s390/vmlogrdr: Remove function pointer cast

Heiko Carstens
2024-04-18T15:15:16Z

Hi Nathan,

> > > > - /*
> > > > - * The release function could be called after the
> > > > - * module has been unloaded. It's _only_ task is to
> > > > - * free the struct. Therefore, we specify kfree()
> > > > - * directly here. (Probably a little bit obfuscating
> > > > - * but legitime ...).
> > > > - */
> > >
> > > Why is the comment not relevant after this change? Or better: why is it not
> > > valid before this change, which is why the code was introduced a very long
> > > time ago? Any reference?
> > >
> > > I've seen the warning since quite some time, but didn't change the code
> > > before sure that this doesn't introduce the bug described in the comment.
> >
> > From only 20 years ago:
> >
> > https://lore.kernel.org/all/20040316170812.GA14971@kroah.com/
> >
> > The particular code (zfcp) was changed, so it doesn't have this code
> > (or never did?) anymore, but for the rest this may or may not still
> > be valid.
>
> I guess relevant may not have been the correct word. Maybe obvious? I
> can keep the comment but I do not really see what it adds, although
> reading the above thread, I suppose it was added as justification for
> calling kfree() as ->release() for a 'struct device'? Kind of seems like
> that ship has sailed since I see this all over the place as a
> ->release() function. I do not see how this patch could have a function
> change beyond that but I may be misreading or misinterpreting your full
> comment.

That doesn't answer my question what prevents the release function
from being called after the module has been unloaded.

At least back then when the code was added it was a real bug.