Hi, Petr
>
> The 1st patch adds the pointer to struct klp_ops into struct
> klp_func. We might check the state a similar way as klp_ftrace_handler().
>
> I had something like this in mind when I suggested to move the pointer:
>
> static ssize_t using_show(struct kobject *kobj,
> struct kobj_attribute *attr, char *buf)
> {
> struct klp_func *func, *using_func;
> struct klp_ops *ops;
> int using;
>
> func = container_of(kobj, struct klp_func, kobj);
>
> rcu_read_lock();
>
> if (func->transition) {
> using = -1;
> goto out;
> }
>
> # FIXME: This requires releasing struct klp_ops via call_rcu()
> ops = func->ops;
> if (!ops) {
> using = 0;
> goto out;
> }
>
> using_func = list_first_or_null_rcu(&ops->func_stack,
> struct klp_func, stack_node);
> if (func == using_func)
> using = 1;
> else
> using = 0;
>
> out:
> rcu_read_unlock();
>
> return sysfs_emit(buf, "%d\n", func->using);
> }
Bravo, I also have something like this in mind when Miroslav suggested to implement the logic in using_show.
>
> It is racy and tricky. We probably should add some memory barriers.
> And maybe even the ordering of reads should be different.
>
> We could not take klp_mutex because it might cause a deadlock when
> the sysfs file gets removed. kobject_put(&func->kobj) is called
> by __klp_free_funcs() under klp_mutex.
>
> It would be easier if we could take klp_mutex. But it would require
> decrementing the kobject refcout without of klp_mutex. It might
> be complicated.
>
> I am afraid that this approach is not worth the effort and
> is is not the way to go.
>
But I don't think I've thought as deeply as you do and may not have considered the possible risks. Therefore, I do need your help to make my patch perfect. ^_^
Thank you.
Wardenjohn.
