Hi, Josh.
> On Oct 8, 2024, at 01:39, Josh Poimboeuf wrote:
>
> On Mon, Oct 07, 2024 at 10:11:39PM +0800, Wardenjohn wrote:
>> Add test case of stack_order sysfs interface of livepatch.
>>
>> Signed-off-by: Wardenjohn
>> ---
>> .../testing/selftests/livepatch/test-sysfs.sh | 24 ++
> 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
>&
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 attribut
Hi, Miroslav.
> On Oct 2, 2024, at 19:44, Miroslav Benes wrote:
>
> Hello,
>
> could you also include the selftests as discussed before, please?
>
> Miroslav
Should I include selftests in one patch?
Regards.
Wardenjohn.
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/Do
Hi, Miroslav!
> On Sep 27, 2024, at 22:11, Miroslav Benes wrote:
>
>
> How do you prepare your patches?
>
> "---" delimiter is missing here.
I seem to found out what cause this problem.
I seemed to use 'git format-patch' with '-p' option which
will make my patch have no diff state.
Sorry f
Hi, Miroslav!
>
> Perhaps something like
>
> "
> 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 ->
> On Sep 25, 2024, at 21:08, Marcos Paulo de Souza wrote:
>
> On Wed, 2024-09-25 at 14:40 +0800, Wardenjohn wrote:
>> As previous discussion, maintainers think that patch-level sysfs
>> interface is the
>> only acceptable way to maintain the information of the order that
>> klp_patch is
>> app
Hi! Petr!
> On Sep 24, 2024, at 19:27, Petr Mladek wrote:
>
> This does not work well. It uses the order on the stack when
> the livepatch is being loaded. It is not updated when any livepatch gets
> removed. It might create wrong values.
>
> I have even tried to reproduce this:
>
> # modprob
Hi, Miroslav & Petr
> On Sep 6, 2024, at 17:44, zhang warden wrote:
>
>>
>>>>
>>>>> + struct list_head func_stack;
>>>>> + struct ftrace_ops fops;
>>>>> +};
>>>>> +
>>>>>
>
Hi, Petr
> On Sep 10, 2024, at 16:01, Petr Mladek wrote:
>
> On Sun 2024-09-08 10:51:14, zhang warden wrote:
>>
>> Hi, Petr
>>>
>>> The 1st patch adds the pointer to struct klp_ops into struct
>>> klp_func. We might check the state a sim
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_at
Hi, Petr
> On Sep 7, 2024, at 00:39, Petr Mladek wrote:
>
> On Fri 2024-09-06 17:39:46, zhang warden wrote:
>> Hi, John & Miroslav
>>
>>>>
>>>> Would it be possible to just use klp_transition_patch and implement the
>>>> logic ju
Hi Miroslav
>
> node member. You removed the global list, hence this member is not needed
> anymore.
OK, I got it.
>
>>>
+ struct list_head func_stack;
+ struct ftrace_ops fops;
+};
+
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
>>
Hi, John & Miroslav
>>
>> Would it be possible to just use klp_transition_patch and implement the
>> logic just in using_show()?
>
> Yes, containing the logic to the sysfs file sounds a lot better.
Maybe I can try to use the state of klp_transition_patch to update the
function's state instead
Hi, Miroslav
> On Sep 5, 2024, at 18:10, Miroslav Benes wrote:
>
> Hi,
>
> the subject is missing "livepatch: " prefix and I would prefer something
> like
>
> "livepatch: Move struct klp_ops to struct klp_func"
>
> On Wed, 28 Aug 2024, Wardenjohn wrote:
>
>> Before introduce feature "using
Hi Miroslav,
>
>
> I am not a fan. Josh wrote most of my objections already so I will not
> repeat them. I understand that the attribute might be useful but the
> amount of code it adds to sensitive functions like
> klp_complete_transition() is no fun.
>
OK, the point I make changes to klp_
Hi, Josh.
> Most of this information is already available in sysfs, with the
> exception of patch stacking order.
>
Well, this is the problem my patch want to fix. But my patch is more simpler,
it just shows the stack top of the target function, which is the only thing
users care.
>
> We wan
> On Sep 4, 2024, at 15:14, Josh Poimboeuf wrote:
>
> On Wed, Sep 04, 2024 at 02:34:44PM +0800, zhang warden wrote:
>> In the scenario where multiple people work together to maintain the
>> same system, or a long time running system, the patch sets will
>> inevit
> On Sep 4, 2024, at 12:48, Josh Poimboeuf wrote:
>
> On Wed, Aug 28, 2024 at 10:23:50AM +0800, Wardenjohn wrote:
>> One system may contains more than one livepatch module. We can see
>> which patch is enabled. If some patches applied to one system
>> modifing the same function, livepatch will
> On Aug 28, 2024, at 10:23, Wardenjohn wrote:
>
> One system may contains more than one livepatch module. We can see
> which patch is enabled. If some patches applied to one system
> modifing the same function, livepatch will use the function enabled
> on top of the function stack. However, w
> On Aug 26, 2024, at 04:17, Jiri Kosina wrote:
>
> I believe that Christoph's "Why?" in fact meant "please include the
> rationale for the changes being made (such as the above) in the patch
> changelog" :)
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
Hi Kosina.
OK, the relation on
> On Aug 25, 2024, at 12:48, Christoph Hellwig wrote:
>
> On Thu, Aug 22, 2024 at 11:01:58AM +0800, Wardenjohn wrote:
>> 1. Move klp_ops into klp_func structure.
>> Rewrite the logic of klp_find_ops and
>> other logic to get klp_ops of a function.
>>
>> 2. Move definition of struct klp_ops in
> On Aug 14, 2024, at 00:05, Petr Mladek wrote:
>
> Alternative solution would be to store the pointer of struct klp_ops
> *ops into struct klp_func. Then using_show() could just check if
> the related struct klp_func in on top of the stack.
>
> It would allow to remove the global list klp_op
> The search would need more code. But it would be simple and
> straightforward. We do this many times all over the code.
>
> IMHO, it would actually remove some complexity and be a win-win solution.
>
> Best Regards,
> Petr
Hi Petr!
Thank you for taking the time to review my patch.
I will u
> On Aug 5, 2024, at 14:46, zhangyongde.zyd wrote:
>
> From: Wardenjohn
>
> One system may contains more than one livepatch module. We can see
> which patch is enabled. If some patches applied to one system
> modifing the same function, livepatch will use the function enabled
> on top of the
> IIUC, constprop means const propagation. For example, function
> "foo(int a, int b)" that is called as "foo(a, 10)" will be come
> "foo(int a)" with a hard-coded b = 10 inside.
>
> .part.0 is part of the function, as the other part is inlined in
> the caller.
>
> With binary-diff based
> In my GCC built, we have suffixes like ".constprop.0", ".part.0", ".isra.0",
> and ".isra.0.cold".
A fresher's eye, I met sometime when try to build a livepatch module and found
some mistake caused by ".constprop.0" ".part.0" which is generated by GCC.
These section with such suffixes is s
> On Aug 5, 2024, at 14:46, zhangyongde.zyd wrote:
>
> From: Wardenjohn
>
>
> static void klp_init_func_early(struct klp_object *obj,
> struct klp_func *func)
> {
> + func->using = false;
> kobject_init(&func->kobj, &klp_ktype_func);
> list_add_tail(&func->node, &obj->func_list);
> }
I rev
Hi Petr!
> The value is useless when the transition is in progress.
> You simply do not know which variant is used in this case.
>
Yes, I agree that if the patch is in transition, we can not know which version
of this function is running by one task.
As my previous explanation, each patch hav
> is this always correct though? See the logic in klp_ftrace_handler(). If
> there is a transition running, it is a little bit more complicated.
>
> Miroslav
Hi! Miroslav.
In reality, we often encounter such situation that serval patch in one system,
some patch make changes to one same funct
> On Jun 7, 2024, at 17:07, Miroslav Benes wrote:
>
> It would be better than this patch but given what was mentioned in the
> thread I wonder if it is possible to use ftrace even for this. See
> /sys/kernel/tracing/trace_stat/function*. It already gathers the number of
> hits.
>
Hi, Miro
> We don't have very urgent use for this. As we discussed, various tracing
> tools are sufficient in most cases. I brought this up in the context of the
> "called" entry: if we are really adding a new entry, let's do "counter"
> instead of "called".
>
> Thanks,
> Song
Hi, Song
I hope to find a
> On Jun 6, 2024, at 23:01, Joe Lawrence wrote:
>
> Hi Wardenjohn,
>
> To follow up, Red Hat kpatch QE pointed me toward this test:
>
> https://gitlab.com/redhat/centos-stream/tests/kernel/kernel-tests/-/tree/main/general/kpatch/kpatch-trace?ref_type=heads
>
> which reports a few interestin
Hi Joe,
>
> Perhaps "responsibility" is a better description. This would introduce
> an attribute that someone's userspace utility is relying on. It should
> at least have a kselftest to ensure a random patch in 2027 doesn't break
> it.
I sent this patch to see the what the community thinks abo
> My intention to introduce this attitude to sysfs is that user who what to see
> if this function is called can just need to show this function attribute in
> the livepatch sysfs interface.
>
Sorry bros,
There is a typo in my word : attitude -> attribute
Autocomplete make it wrong….lol..
> On May 31, 2024, at 22:06, Miroslav Benes wrote:
>
>> And for the unlikely branch, isn’t the complier will compile this branch
>> into a cold branch that will do no harm to the function performance?
>
> The test (cmp insn or something like that) still needs to be there. Since
> there is o
> On Jun 1, 2024, at 03:16, Joe Lawrence wrote:
>
> Adding these attributes to livepatch sysfs would be expedient and
> probably easier for us to use, but imposes a recurring burden on us to
> maintain and test (where is the documentation and kselftest for this new
> interface?). Or, we could
> On May 31, 2024, at 15:21, Miroslav Benes wrote:
>
> Hi,
>
> On Fri, 31 May 2024, zhang warden wrote:
>
> you have not replied to my questions/feedback yet.
>
> Also, I do not think that unlikely changes anything here. It is a simple
> branch after all.
>
Hi Bros,
How about my patch? I do think it is a viable feature to show the state of the
patched function. If we add an unlikely branch test before we set the 'called'
state, once this function is called, there maybe no negative effect to the
performance.
Please give me some advice.
Regards,
> On May 23, 2024, at 22:22, Dan Carpenter wrote:
>
> Always run your patches through checkpatch.
>
> So this patch is so that testers can see if a function has been called?
> Can you not get the same information from gcov or ftrace?
>
> There are style issues with the patch, but it's not so
> On May 21, 2024, at 16:04, Petr Mladek wrote:
>
> Another motivation to use ftrace for testing is that it does not
> affect the performance in production.
>
> We should keep klp_ftrace_handler() as fast as possible so that we
> could livepatch also performance sensitive functions.
>
How a
OK, I will try to optimize my description after the patch is reviewed. I am
sure there are something still need to be fix for that patch.
> On May 20, 2024, at 16:00, Markus Elfring wrote:
>
> Please add a version identifier to the message subject.
>
>
> …
>> If the patched function have bug,
> On May 20, 2024, at 14:46, Miroslav Benes wrote:
>
> Hi,
>
> On Mon, 20 May 2024, Wardenjohn wrote:
>
>> Livepatch module usually used to modify kernel functions.
>> If the patched function have bug, it may cause serious result
>> such as kernel crash.
>>
>> This is a kobject attribute of
OK, I will optimize my patch’s changelog in my next patch.
> On May 20, 2024, at 02:05, Markus Elfring wrote:
>
> I suggest to take preferred line lengths better into account
> also for such a change description.
> On May 7, 2024, at 10:41, Josh Poimboeuf wrote:
>
> On Tue, May 07, 2024 at 10:21:40AM +0800, zhang warden wrote:
>>
>>
>>>
>>> transition state. With this marcos renamed, comments are not
>>> necessary at this point.
>>>
>&g
>
> transition state. With this marcos renamed, comments are not
> necessary at this point.
>
Sorry for my careless, the comment still remains in the code. However, comment
in the code do no harms here. Maybe it can be kept.
> On May 6, 2024, at 05:00, Josh Poimboeuf wrote:
>
> On Mon, Apr 29, 2024 at 03:26:28PM +0800, zhangwar...@gmail.com wrote:
>> From: Wardenjohn
>>
>> livepatch.h use KLP_UNDEFINED\KLP_UNPATCHED\KLP_PATCHED for klp transition
>> state.
>> When livepatch is ready but idle, using KLP_UNDEFINE
Hi Joe and Petr :
> On Apr 5, 2024, at 01:50, Joe Lawrence wrote:
>
> On 4/4/24 11:17, Petr Mladek wrote:
>> On Tue 2024-04-02 09:52:31, Joe Lawrence wrote:
>>> On Tue, Apr 02, 2024 at 11:09:54AM +0800, zhangwar...@gmail.com wrote:
From: Wardenjohn
In livepatch, using KLP_UNDEF
49 matches
Mail list logo
