On Wed 2021-12-15 09:47:00, David Vernet wrote:
> The livepatch subsystem has several exported functions and objects with
> kerneldoc comments. Though the livepatch documentation contains
> handwritten descriptions of all of these exported functions, they are
> currently not pulled into the docs build using the kernel-doc directive.
>
> Note that all of the handwritten API descriptions were left alone with
> the exception of Documentation/livepatch/system-state.rst, which was
> updated to allow the cross-referencing to work correctly. The file now
> follows the cross-referencing formatting guidance specified in
> Documentation/doc-guide/kernel-doc.rst. Furthermore, some comments
> around klp_shadow_free_all() were updated to say <obj, id> rather than
> <*, id> to match the rest of the file, and to prevent the docs build
> from emitting an "Inline emphasis start-string without end string"
> error.
>
> --- a/kernel/livepatch/shadow.c
> +++ b/kernel/livepatch/shadow.c
> @@ -272,12 +272,12 @@ void klp_shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor)
> EXPORT_SYMBOL_GPL(klp_shadow_free);
>
> /**
> - * klp_shadow_free_all() - detach and free all <*, id> shadow variables
> + * klp_shadow_free_all() - detach and free all <obj, id> shadow variables
This change is not good. The function releases all existing shadow
variables with the given @id for any @obj. And it is not longer clear.
I guess that the primary motivation was to remove "Inline emphasis
start-string without end string" mentioned in the commit message.
A solution would be replace '*' with something else, for example, < , id>.
Another solution would be to describe it another way, for example:
* klp_shadow_free_all() - detach and free all <obj, id> shadow variables
* with the given @id.
> * @id: data identifier
> * @dtor: custom callback that can be used to unregister the variable
> * and/or free data that the shadow variable points to (optional)
> *
> - * This function releases the memory for all <*, id> shadow variable
> + * This function releases the memory for all <obj, id> shadow variable
Same here.
> * instances, callers should stop referencing them accordingly.
> */
> void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor)
> @@ -288,7 +288,7 @@ void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor)
>
> spin_lock_irqsave(&klp_shadow_lock, flags);
>
> - /* Delete all <*, id> from hash */
> + /* Delete all <obj, id> from hash */
and here
> hash_for_each(klp_shadow_hash, i, shadow, node) {
> if (klp_shadow_match(shadow, shadow->obj, id))
> klp_shadow_free_struct(shadow, dtor);
BTW: There is likely the same problem in Documentation/livepatch/shadow-vars.rst.
I see <*, id> there as well.
Otherwise, the patch looks fine to me.
Best Regards,
Petr
