All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: David Vernet <void@manifault.com>
Cc: linux-doc@vger.kernel.org, live-patching@vger.kernel.org,
	linux-kernel@vger.kernel.org, jpoimboe@redhat.com,
	jikos@kernel.org, mbenes@suse.cz, joe.lawrence@redhat.com,
	corbet@lwn.net, yhs@fb.com, songliubraving@fb.com
Subject: Re: [PATCH] Documentation: livepatch: Add kernel-doc link to klp_enable_patch
Date: Fri, 10 Dec 2021 10:25:05 +0100	[thread overview]
Message-ID: <YbMc8YGIoyRU5nwJ@alley> (raw)
In-Reply-To: <20211209165303.3205464-1-void@manifault.com>

On Thu 2021-12-09 08:53:04, David Vernet wrote:
> The `klp_enable_patch()` function is the main entrypoint to the livepatch
> subsystem, and is invoked by a KLP module from the module_init callback
> when it is ready to be enabled.  The livepatch documentation specifies that
> `klp_enable_patch()` should be invoked from the `module_init()` callback,
> but does not actually link the user to the function's kerneldoc comment.
> 
> This simple change therefore adds a kernel-doc directive to link the
> `klp_enable_patch()` function's kerneldoc comment in the livepatch
> documentation page. With this, kernel/livepatch/core.c no longer comes up
> as a file containing an unused doc with
> `scripts/find-unused-docs.sh kernel/livepatch`
> 
> --- a/Documentation/livepatch/livepatch.rst
> +++ b/Documentation/livepatch/livepatch.rst
> @@ -312,8 +312,15 @@ the patch cannot get enabled.
>  -------------
>  
>  The livepatch gets enabled by calling klp_enable_patch() from
> -the module_init() callback. The system will start using the new
> -implementation of the patched functions at this stage.
> +the module_init() callback:
> +
> +.. kernel-doc:: kernel/livepatch/core.c
> +   :functions: klp_enable_patch
> +
> +----
> +
> +The system will start using the new implementation of the patched functions at
> +this stage.
>  
>  First, the addresses of the patched functions are found according to their
>  names. The special relocations, mentioned in the section "New functions",

Honestly, I do not like this. It might be acceptable when it converts
klp_enable_patch() into a link pointing to another page describing the API.

But this patch causes the entire documentation of klp_enable_patch()
inserted into livepatch.html. It does not fit there and breaks
the text flow.


Heh, I had hard times to build the documentation (sphinx crashed, ...).
So, I paste the html output for others here:

<cut&paste>
5.2. Enabling¶
The livepatch gets enabled by calling klp_enable_patch() from the module_init() callback:

int klp_enable_patch(struct klp_patch *patch)¶
enable the livepatch

Parameters

struct klp_patch *patch
patch to be enabled

Description

Initializes the data structure associated with the patch, creates the sysfs interface, performs the needed symbol lookups and code relocations, registers the patched functions with ftrace.

This function is supposed to be called from the livepatch module_init() callback.

Return

0 on success, otherwise error

The system will start using the new implementation of the patched functions at this stage.

First, the addresses of the patched functions are found according to their names. The special relocations, mentioned in the section “New functions”, are applied. The relevant entries are created under /sys/kernel/livepatch/<name>. The patch is rejected when any above operation fails.

Second, livepatch enters into a transition state where tasks are converging to the patched state. If an original function is patched for the first time, a function specific struct klp_ops is created and an universal ftrace handler is registered1. This stage is indicated by a value of ‘1’ in /sys/kernel/livepatch/<name>/transition. For more information about this process, see the “Consistency model” section.

Finally, once all tasks have been patched, the ‘transition’ value changes to ‘0’.

1
Note that functions might be patched multiple times. The ftrace handler is registered only once for a given function. Further patches just add an entry to the list (see field func_stack) of the struct klp_ops. The right implementation is selected by the ftrace handler, see the “Consistency model” section.

That said, it is highly recommended to use cumulative livepatches
because they help keeping the consistency of all changes. In this
case, functions might be patched two times only during the transition
period.
</cut&paste>


Best Regards,
Petr

  reply	other threads:[~2021-12-10  9:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09 16:53 [PATCH] Documentation: livepatch: Add kernel-doc link to klp_enable_patch David Vernet
2021-12-10  9:25 ` Petr Mladek [this message]
2021-12-10 18:24   ` David Vernet
2021-12-13 16:06     ` Petr Mladek
2021-12-14  8:54       ` Miroslav Benes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YbMc8YGIoyRU5nwJ@alley \
    --to=pmladek@suse.com \
    --cc=corbet@lwn.net \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=songliubraving@fb.com \
    --cc=void@manifault.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.