On Fri 2021-12-17 13:50:42, Josh Poimboeuf wrote:
> On Fri, Dec 17, 2021 at 02:51:42PM +0100, Petr Mladek wrote:
> > On Wed 2021-12-15 07:20:04, David Vernet wrote:
> > Josh's proposal adds pre-early-init() to allow calling
> > klp_free_patch_*() already when klp_init_*_early() fails.
> > The purpose is to make sure that klp_init_*_early()
> > will actually never fail.
> >
> > This might make things somehow complicated. Any future change
> > in klp_init_*_early() might require change in pre-early-init()
> > to catch eventual problems earlier.
>
> I'm not sure why that would be a problem. If we can separate out the
> things which fail from the things which don't, it simplifies things.
I think that there is no right answer. It depends on personal
preferences.
> > My proposal is to use simple trick. klp_free_patch_*() iterate
> > using the dynamic list (list_for_each_entry) while klp_init_*_early()
> > iterate using the arrays.
> >
> > Now, we just need to make sure that klp_init_*_early() will only add
> > fully initialized structures into the dynamic list. As a result,
> > klp_free_patch() will see only the fully initialized structures
> > and could release them. It will not see that not-yet-initialized
> > structures but it is fine. They are not initialized and they do not
> > need to be freed.
> >
> > In fact, I think that klp_init_*_early() functions already do
> > the right thing. IMHO, if you move the module_get() then you
> > could safely do:
> >
> > int klp_enable_patch(struct klp_patch *patch)
> > {
> > [...]
> > if (!try_module_get(patch->mod)) {
> > mutex_unlock(&klp_mutex);
> > return -ENODEV;
> > }
> >
> > ret = klp_init_patch_early(patch);
> > if (ret)
> > goto err;
> >
> >
> > Note that it would need to get tested.
> >
> > Anyway, does it make sense now?
>
> It would work, but it's too clever/non-obvious. If we rely on tricks
> then we may eventually forget about them and accidentally break
> assumptions later.
It is not that super magic from my POV. But again, it is a personal
taste.
I do not want to bikeshed about it. Feel free to use the pre-early
thing.
Best Regards,
Petr
