Re: [PATCH V3 1/1] livepatch: Add "stack_order" sysfs attribute
> On Oct 3, 2024, at 23:25, Josh Poimboeuf wrote: > > On Thu, Oct 03, 2024 at 10:59:11PM +0800, zhang warden wrote: >>> 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. >> >> This description looks good to me. What's the suggestion of >> other maintainers ? > > I like it, though "highest stack order" is still a bit arbitrary, since > the highest stack order is actually the lowest number. > > -- > Josh How about: This attribute specifies the sequence in which live patch module are applied to the system. If multiple live patches modify the same function, the implementation with the biggest 'stack_order' number is used, unless a transition is currently in progress. Regards. Wardenjohn.
Re: [PATCH V3 1/1] livepatch: Add "stack_order" sysfs attribute
On Thu, Oct 03, 2024 at 10:59:11PM +0800, zhang warden wrote: > > 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. > > This description looks good to me. What's the suggestion of > other maintainers ? I like it, though "highest stack order" is still a bit arbitrary, since the highest stack order is actually the lowest number. -- Josh
Re: [PATCH V3 1/1] livepatch: Add "stack_order" sysfs attribute
Hi, Petr.
> 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").
>
OK, will fix it.
+Contact:live-patch...@vger.kernel.org
+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.
This description looks good to me. What's the suggestion of
other maintainers ?
Regards.
Wardenjohn.
Re: [PATCH V3 1/1] livepatch: Add "stack_order" sysfs attribute
On Thu 2024-10-03 14:26:19, zhang warden wrote:
>
> Hi, Josh!
> > On Oct 1, 2024, at 07:26, Josh Poimboeuf 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//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-patch...@vger.kernel.org
> >> +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
Re: [PATCH V3 1/1] livepatch: Add "stack_order" sysfs attribute
Hi, Josh! > On Oct 1, 2024, at 07:26, Josh Poimboeuf 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//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. >> +Contact:live-patch...@vger.kernel.org >> +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. Regards. Wardenjohn.
Re: [PATCH V3 1/1] livepatch: Add "stack_order" sysfs attribute
On Sun, Sep 29, 2024 at 10:43:34PM +0800, Wardenjohn wrote: > Add "stack_order" sysfs attribute which holds the order in which a live > patch module was loaded into the system. A user can then determine an > active live patched version of a function. > > cat /sys/kernel/livepatch/livepatch_1/stack_order -> 1 > > means that livepatch_1 is the first live patch applied > > cat /sys/kernel/livepatch/livepatch_module/stack_order -> N > > means that livepatch_module is the Nth live patch applied > > Suggested-by: Petr Mladek > Suggested-by: Miroslav Benes > Suggested-by: Josh Poimboeuf > Signed-off-by: Wardenjohn > --- > .../ABI/testing/sysfs-kernel-livepatch| 8 ++ > kernel/livepatch/core.c | 25 +++ > 2 files changed, 33 insertions(+) > > 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//stack_order > +Date: Sep 2024 > +KernelVersion: 6.12.0 These will probably need to be updated (can probably be done by Petr when applying). > +Contact:live-patch...@vger.kernel.org > +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. -- Josh
