lkml.org 
[lkml]   [2018]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v14 07/11] livepatch: Add atomic replace
On Thu 2018-12-13 16:55:28, Josh Poimboeuf wrote:
> On Thu, Nov 29, 2018 at 10:44:27AM +0100, Petr Mladek wrote:
> > @@ -415,6 +449,124 @@ static struct attribute *klp_patch_attrs[] = {
> > NULL
> > };
> >
> > +/*
> > + * Dynamically allocated objects and functions.
> > + */
>
> I don't think this comment is needed.
>
> > +static void klp_free_object_dynamic(struct klp_object *obj)
> > +{
> > + kfree(obj->name);
> > + kfree(obj);
> > +}
> > @@ -456,6 +620,8 @@ static void klp_free_funcs(struct klp_object *obj)
> > if (func->kobj_alive) {
> > func->kobj_alive = false;
> > kobject_put(&func->kobj);
> > + } else if (func->nop) {
> > + klp_free_func_nop(func);
>
> This removes 'func' from the list, so it needs to do a 'safe' list
> iteration.

Good catch!

Well, note that the 'safe' list iterators were added by the very next
patch anyway. This is why I never triggered a problem with this.


> > +void klp_discard_replaced_patches(struct klp_patch *new_patch)
> > +{
> > + struct klp_patch *old_patch, *tmp_patch;
> > +
> > + list_for_each_entry_safe(old_patch, tmp_patch, &klp_patches, list) {
> > + if (old_patch == new_patch)
> > + return;
> > +
> > + old_patch->enabled = false;
> > + klp_unpatch_objects(old_patch);
> > + klp_free_patch_start(old_patch);
> > + schedule_work(&old_patch->free_work);
> > + }
>
> This doesn't need the "safe" list iteration because it doesn't remove
> the patch from the list.

It does need the 'safe' list. klp_free_patch_start() removes
the patch from the list.


> Side note, it would probably be useful to have a klp_for_each_patch()
> helper.

Will do.

Best Regards,
Petr

\
 
 \ /
  Last update: 2018-12-17 16:28    [W:0.093 / U:21.744 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site