Re: [PATCH V3 1/1] livepatch: Add "stack_order" sysfs attribute

[Petr Mladek]

On Thu 2024-10-03 14:26:19, zhang warden wrote:
> 
> Hi, Josh!
> > On Oct 1, 2024, at 07:26, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> >> 
> >> diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch
> >> index a5df9b4910dc..2a60b49aa9a5 100644
> >> --- a/Documentation/ABI/testing/sysfs-kernel-livepatch
> >> +++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
> >> @@ -47,6 +47,14 @@ Description:
> >> disabled when the feature is used. See
> >> Documentation/livepatch/livepatch.rst for more information.
> >> 
> >> +What:           /sys/kernel/livepatch/<patch>/stack_order
> >> +Date:           Sep 2024
> >> +KernelVersion:  6.12.0
> > 
> > These will probably need to be updated (can probably be done by Petr when
> > applying).
> > 
> True, kernel version may need Petr to decide.

It would be for 6.13 if the changes are accepted in time before the
next merge window.

Also please rebase the patch on top of current Linus' master or
v6.11. There are conflicts with the commit adb68ed26a3e922
("livepatch: Add "replace" sysfs attribute").


> >> +Contact:        live-patching@xxxxxxxxxxxxxxx
> >> +Description:
> >> + This attribute holds the stack order of a livepatch module applied
> >> + to the running system.
> > 
> > It's probably a good idea to clarify what "stack order" means.  Also,
> > try to keep the text under 80 columns for consistency.
> > 
> > How about:
> > 
> > This attribute indicates the order the patch was applied
> > compared to other patches.  For example, a stack_order value of
> > '2' indicates the patch was applied after the patch with stack
> > order '1' and before any other currently applied patches.
> > 
> 
> Or how about:
> 
> This attribute indicates the order of the livepatch module 
> applied to the system. The stack_order value N means 
> that this module is the Nth applied to the system. If there
> are serval patches changing the same function, the function
> version of the biggest stack_order is enabling in the system.

The 2nd sentence looks superfluous. The 3rd sentence explains
the important effect.

Well, the part "is enabling in the system" is a bit cryptic.
I would write something like:

This attribute specifies the sequence in which live patch modules
are applied to the system. If multiple live patches modify the same
function, the implementation with the highest stack order is used,
unless a transition is currently in progress.

Best Regards,
Petr