Re: [PATCH 00/21] kGraft
On Wed, Jun 25, 2014 at 05:54:50PM +0200, Jiri Kosina wrote:
> > - wait_event_freezable(khubd_wait,
> > + wait_event_freezable(khubd_wait,
> > ({ kgr_task_safe(current);
> >
> > The changes are somewhat ugly with all the kgraft crap leaking into plces
> > like jbd and freezer and usb. That says to me its not well isolated and
> > there must be a better way of hiding that kgr stuff so we don't have to
> > kepe putting more code into all the kthreads, and inevitably missing them
> > or have people getting it wrong.
>
> Tejun commented on this very issue during the first RFC submission a
> couple weeks ago. Part of his proposal was actually to take this as a good
> pretext to review the existing kernel threads, as the idea is that a big
> portion of those could easily be converted to workqueues.
In the past few years there was a significant proliferation of kernel
threads just for cases where something needs to be done from a process
context now and then.
This is underlined by the fact that on an average installation there are
more kernel threads than real userspace processes.
Converting these bits to workqueues would then also take care of their
interaction with freezer, kthread_stop. The main loop code wouldn't have
to be replicated over and over with slight variations. kgr_taks_safe
would then plug into that rather elegantly.
In the absence of this rework, kGraft hijacks the freezer and
kthread_stop to pinpoint the 'end' of the main loop of most kernel
threads and annotates the rest that doesn't handle freezing or stopping
with kgr_task_safe directly.
> It of course has its own implications, such as not being able to
> prioritize that easily, but there is probably a lot of low-hanging fruits
> where driver authors and their grandmas have been creating kthreads where
> workqueue would suffice.
Indeed, on the other hand, there are enough workqueues today and on
realtime systems the need for prioritizing them exists already.
> But it's a very long term goal, and it's probably impractical to make
> kGraft dependent on it.
Should the kernel thread changes turn to be a big issue, we could do the
initial submission without them and depend on the kthread->workqueue
rework. Once we add support for multiple patches in progress, the fact
that we don't support kernel threads would not be as painful.
> > You add instructions to various hotpaths (despite the CONFIG
> > comment).
>
> Kernel entry is hidden behind _TIF_WORK_SYSCALL_ENTRY and
> _TIF_ALLWORK_MASK, so it doesn't add any overhead if kGraft is not
> enabled. What other hot paths do you refer to?
As Jiří says, I don't see any hot path where kGraft would add
instructions: Kernel exit and entry are handled outside the hot path
in the _TIF_WORK_SYSCALL_ENTRY and _TIF_ALLWORK_MASK handlers. [patch 15/21]
It adds instructions into the main loops of kernel threads, but those
can hardly be called hot paths as they mostly sleep for long times.
--
Vojtech Pavlik
Director SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/21] kGraft
On Wed, 25 Jun 2014, One Thousand Gnomes wrote:
> > this is a repost of the second round of RFC on kGraft, the linux
> > kernel online patching developed at SUSE. This repost only widened the
> > target audience for broader review, no code change happened.
>
> - wait_event_freezable(khubd_wait,
> + wait_event_freezable(khubd_wait,
> ({ kgr_task_safe(current);
>
> The changes are somewhat ugly with all the kgraft crap leaking into plces
> like jbd and freezer and usb. That says to me its not well isolated and
> there must be a better way of hiding that kgr stuff so we don't have to
> kepe putting more code into all the kthreads, and inevitably missing them
> or have people getting it wrong.
Tejun commented on this very issue during the first RFC submission a
couple weeks ago. Part of his proposal was actually to take this as a good
pretext to review the existing kernel threads, as the idea is that a big
portion of those could easily be converted to workqueues.
It of course has its own implications, such as not being able to
prioritize that easily, but there is probably a lot of low-hanging fruits
where driver authors and their grandmas have been creating kthreads where
workqueue would suffice.
But it's a very long term goal, and it's probably impractical to make
kGraft dependent on it.
> You add instructions to various hotpaths (despite the CONFIG comment).
Kernel entry is hidden behind _TIF_WORK_SYSCALL_ENTRY and
_TIF_ALLWORK_MASK, so it doesn't add any overhead if kGraft is not
enabled. What other hot paths do you refer to?
> Don't get me wrong - I'd like a good working ksplice/graft !
Thanks Alan,
--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/21] kGraft
On Wed, 25 Jun 2014 13:05:29 +0200
Jiri Slaby wrote:
> Hi,
>
> this is a repost of the second round of RFC on kGraft, the linux
> kernel online patching developed at SUSE. This repost only widened the
> target audience for broader review, no code change happened.
- wait_event_freezable(khubd_wait,
+ wait_event_freezable(khubd_wait,
({ kgr_task_safe(current);
The changes are somewhat ugly with all the kgraft crap leaking into plces
like jbd and freezer and usb. That says to me its not well isolated and
there must be a better way of hiding that kgr stuff so we don't have to
kepe putting more code into all the kthreads, and inevitably missing them
or have people getting it wrong.
You also wake up all the kthreads when some of them might not expect to
be woken and may not check for the case of a bogus wake.
You add instructions to various hotpaths (despite the CONFIG comment).
Have you done timing analysis of their impact when turned on ?
Can you explain a bit more about why the path you've chosen was used as
opposed to using the power management freeze/resume path. Would that not
be a lot less intrusive ?
Don't get me wrong - I'd like a good working ksplice/graft !
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 00/21] kGraft
Hi, this is a repost of the second round of RFC on kGraft, the linux kernel online patching developed at SUSE. This repost only widened the target audience for broader review, no code change happened. Please speak up now (or be silent till the next merge window). That is, if there are no objections, we plan pushing the tree into -next and asking Linus in the next merge window for comments. The patches are posted as a reply to this email and can be also obtained as a whole tree from: https://git.kernel.org/cgit/linux/kernel/git/jirislaby/kgraft.git/log/?h=kgraft Jiri Kosina (6): kgr: initial code kgr: x86: refuse to build without fentry support kgr: add procfs interface for per-process 'kgr_in_progress' kgr: make a per-process 'in progress' flag a single bit kgr: expose global 'in_progress' state through procfs kgr: x86: optimize handling of CPU-bound tasks Jiri Slaby (14): ftrace: Add function to find fentry of function ftrace: Make ftrace_is_dead available globally kgr: add testing kgraft patch kgr: update Kconfig documentation kgr: add Documentation kgr: trigger the first check earlier kgr: sched.h, introduce kgr_task_safe helper kgr: mark task_safe in some kthreads kgr: kthreads support kgr: handle irqs kgr: add MAINTAINERS entry kgr: add support for missing functions kgr: exercise non-present function kgr: fix race of stub and patching Libor Pechacek (1): kgr: rephrase the "kGraft failed" message Documentation/kgraft.txt | 44 MAINTAINERS| 9 + arch/x86/Kconfig | 2 + arch/x86/include/asm/kgraft.h | 61 ++ arch/x86/include/asm/thread_info.h | 6 +- arch/x86/kernel/entry_64.S | 9 + drivers/base/devtmpfs.c| 1 + drivers/scsi/scsi_error.c | 2 + drivers/usb/core/hub.c | 4 +- fs/jbd2/journal.c | 2 + fs/notify/mark.c | 5 +- fs/proc/base.c | 11 + include/linux/freezer.h| 2 + include/linux/ftrace.h | 4 + include/linux/kgraft.h | 90 include/linux/sched.h | 9 + kernel/Kconfig.kgraft | 10 + kernel/Makefile| 1 + kernel/hung_task.c | 5 +- kernel/kgraft.c| 430 + kernel/kthread.c | 3 + kernel/rcu/tree.c | 6 +- kernel/rcu/tree_plugin.h | 10 +- kernel/smpboot.c | 2 + kernel/trace/ftrace.c | 30 +++ kernel/trace/trace.h | 2 - kernel/workqueue.c | 3 + mm/huge_memory.c | 1 + net/bluetooth/rfcomm/core.c| 2 + samples/Kconfig| 8 + samples/Makefile | 3 +- samples/kgraft/Makefile| 1 + samples/kgraft/kgraft_patcher.c| 99 + 33 files changed, 864 insertions(+), 13 deletions(-) create mode 100644 Documentation/kgraft.txt create mode 100644 arch/x86/include/asm/kgraft.h create mode 100644 include/linux/kgraft.h create mode 100644 kernel/Kconfig.kgraft create mode 100644 kernel/kgraft.c create mode 100644 samples/kgraft/Makefile create mode 100644 samples/kgraft/kgraft_patcher.c -- 2.0.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 00/21] kGraft
Hi, this is the second round of RFC on kGraft, the linux kernel online patching developed at SUSE. The patches are posted as a reply to this email and can be also obtained as a whole tree at: https://git.kernel.org/cgit/linux/kernel/git/jirislaby/kgraft.git/log/?h=kgraft Jiri Kosina (4): kgr: initial code kgr: x86: refuse to build without fentry support kgr: add procfs interface for per-process 'kgr_in_progress' kgr: make a per-process 'in progress' flag a single bit Jiri Slaby (12): ftrace: Add function to find fentry of function ftrace: Make ftrace_is_dead available globally kgr: add testing kgraft patch kgr: update Kconfig documentation kgr: add Documentation kgr: trigger the first check earlier kgr: sched.h, introduce kgr_task_safe helper kgr: mark task_safe in some kthreads kgr: kthreads support kgr: handle irqs kgr: add tools kgr: add MAINTAINERS entry Jiri Kosina (6): kgr: initial code kgr: x86: refuse to build without fentry support kgr: add procfs interface for per-process 'kgr_in_progress' kgr: make a per-process 'in progress' flag a single bit kgr: expose global 'in_progress' state through procfs kgr: x86: optimize handling of CPU-bound tasks Jiri Slaby (14): ftrace: Add function to find fentry of function ftrace: Make ftrace_is_dead available globally kgr: add testing kgraft patch kgr: update Kconfig documentation kgr: add Documentation kgr: trigger the first check earlier kgr: sched.h, introduce kgr_task_safe helper kgr: mark task_safe in some kthreads kgr: kthreads support kgr: handle irqs kgr: add MAINTAINERS entry kgr: add support for missing functions kgr: exercise non-present function kgr: fix race of stub and patching Libor Pechacek (1): kgr: rephrase the "kGraft failed" message Documentation/kgraft.txt | 44 MAINTAINERS| 9 + arch/x86/Kconfig | 2 + arch/x86/include/asm/kgraft.h | 61 ++ arch/x86/include/asm/thread_info.h | 6 +- arch/x86/kernel/entry_64.S | 9 + drivers/base/devtmpfs.c| 1 + drivers/scsi/scsi_error.c | 2 + drivers/usb/core/hub.c | 4 +- fs/jbd2/journal.c | 2 + fs/notify/mark.c | 5 +- fs/proc/base.c | 11 + include/linux/freezer.h| 2 + include/linux/ftrace.h | 4 + include/linux/kgraft.h | 90 include/linux/sched.h | 9 + kernel/Kconfig.kgraft | 10 + kernel/Makefile| 1 + kernel/hung_task.c | 5 +- kernel/kgraft.c| 430 + kernel/kthread.c | 3 + kernel/rcu/tree.c | 6 +- kernel/rcu/tree_plugin.h | 10 +- kernel/smpboot.c | 2 + kernel/trace/ftrace.c | 30 +++ kernel/trace/trace.h | 2 - kernel/workqueue.c | 3 + mm/huge_memory.c | 1 + net/bluetooth/rfcomm/core.c| 2 + samples/Kconfig| 8 + samples/Makefile | 3 +- samples/kgraft/Makefile| 1 + samples/kgraft/kgraft_patcher.c| 99 + 33 files changed, 864 insertions(+), 13 deletions(-) create mode 100644 Documentation/kgraft.txt create mode 100644 arch/x86/include/asm/kgraft.h create mode 100644 include/linux/kgraft.h create mode 100644 kernel/Kconfig.kgraft create mode 100644 kernel/kgraft.c create mode 100644 samples/kgraft/Makefile create mode 100644 samples/kgraft/kgraft_patcher.c -- 2.0.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
