| Message ID | 20181219141744.32392-1-mbenes@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled | expand |
On Wed, Dec 19, 2018 at 03:17:44PM +0100, Miroslav Benes wrote: > GCC 9 introduces a new option, -flive-patching. It disables certain > optimizations which could make a compilation unsafe for later live > patching of the running kernel. > > The option is used only if CONFIG_LIVEPATCH is enabled and $(CC) > supports it. > > Signed-off-by: Miroslav Benes <mbenes@suse.cz> > --- > Makefile | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Makefile b/Makefile > index a0650bf79606..53f5ab810efe 100644 > --- a/Makefile > +++ b/Makefile > @@ -778,6 +778,10 @@ KBUILD_CFLAGS_KERNEL += $(call cc-option,-ffunction-sections,) > KBUILD_CFLAGS_KERNEL += $(call cc-option,-fdata-sections,) > endif > > +ifdef CONFIG_LIVEPATCH > +KBUILD_CFLAGS += $(call cc-option, -flive-patching=inline-clone) > +endif > + > # arch Makefile may override CC so keep this after arch Makefile is included > NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include) This option only makes sense for source-based patch generation, so isn't it a bit premature to make this change without proper source-based patch tooling? Also the commit message needs an analysis of the performance impacts.
On Wed, 19 Dec 2018, Josh Poimboeuf wrote: > This option only makes sense for source-based patch generation, so isn't > it a bit premature to make this change without proper source-based patch > tooling? The reality is though that before the full-fledged patch tooling exists, people are actually already writing livepatches by hand, so this option is useful for them. > Also the commit message needs an analysis of the performance impacts. Agreed. Especially as it's expected (*) to be completely in the noise particularly for the kernel, it'd be good to have that documented in the changelog. (*) actually measured already for some subset of the IPA optimizations Thanks,
On Wed, Dec 19, 2018 at 05:58:53PM +0100, Jiri Kosina wrote: > On Wed, 19 Dec 2018, Josh Poimboeuf wrote: > > > This option only makes sense for source-based patch generation, so isn't > > it a bit premature to make this change without proper source-based patch > > tooling? > > The reality is though that before the full-fledged patch tooling exists, > people are actually already writing livepatches by hand, so this option is > useful for them. Fair enough. Though, upstream, almost everybody seems to use kpatch-build, for which this patch doesn't help. And people will continue to do so until we have decent source-based tooling. Will the klp-convert patches be revived soon?
On Wed, 19 Dec 2018, Josh Poimboeuf wrote: > > > This option only makes sense for source-based patch generation, so isn't > > > it a bit premature to make this change without proper source-based patch > > > tooling? > > > > The reality is though that before the full-fledged patch tooling exists, > > people are actually already writing livepatches by hand, so this option is > > useful for them. > > Fair enough. > > Though, upstream, almost everybody seems to use kpatch-build, for which > this patch doesn't help. And people will continue to do so until we > have decent source-based tooling. Will the klp-convert patches be > revived soon? Let me add Joao, who's working on that. Joao, I think you had something basically ready for upstream exposure, right? Thanks,
On Wed, 19 Dec 2018, Jiri Kosina wrote: > On Wed, 19 Dec 2018, Josh Poimboeuf wrote: > > > Also the commit message needs an analysis of the performance impacts. > > Agreed. Especially as it's expected (*) to be completely in the noise > particularly for the kernel, it'd be good to have that documented in the > changelog. > > (*) actually measured already for some subset of the IPA optimizations Ok, we can do that. I don't expect the results to be different from the last measurement as Jiri mentions. The sets of disabled optimizations are similar. I'll add it to v2. On Wed, 19 Dec 2018, Jiri Kosina wrote: > On Wed, 19 Dec 2018, Josh Poimboeuf wrote: > > > > > This option only makes sense for source-based patch generation, so isn't > > > > it a bit premature to make this change without proper source-based patch > > > > tooling? > > > > > > The reality is though that before the full-fledged patch tooling exists, > > > people are actually already writing livepatches by hand, so this option is > > > useful for them. > > > > Fair enough. Yes, that was the reason I sent it. It would not make sense to wait for the tooling in this case, because -flive-patching is useful even now, since there is a way to prepare livepatches without any tooling. > > Though, upstream, almost everybody seems to use kpatch-build, for which > > this patch doesn't help. And people will continue to do so until we > > have decent source-based tooling. Will the klp-convert patches be > > revived soon? > > Let me add Joao, who's working on that. > > Joao, I think you had something basically ready for upstream exposure, > right? I think that when Joao posted it a long time ago, the conclusion was that it would be better to wait for the source-based tooling and have the complete solution. I may misremember though. If Josh thinks that it would be acceptable to have klp-convert merged even without the tooling, I'm all for it. We're about to start using it at SUSE and staying close to upstream would definitely be better. Miroslav
On Thu, Dec 20, 2018 at 09:33:05AM +0100, Miroslav Benes wrote: > > > Though, upstream, almost everybody seems to use kpatch-build, for which > > > this patch doesn't help. And people will continue to do so until we > > > have decent source-based tooling. Will the klp-convert patches be > > > revived soon? > > > > Let me add Joao, who's working on that. > > > > Joao, I think you had something basically ready for upstream exposure, > > right? > > I think that when Joao posted it a long time ago, the conclusion was that > it would be better to wait for the source-based tooling and have the > complete solution. I may misremember though. > > If Josh thinks that it would be acceptable to have klp-convert merged even > without the tooling, I'm all for it. > > We're about to start using it at SUSE and staying close to upstream would > definitely be better. A complete toolchain should definitely be the end goal. But as a usable first step, only *some* of the tooling is required, since some of the steps can be done manually, right? So starting out, for something usable, I believe the following would be the bare minimum: * -flive-patching * The analysis tool which analyzes the -fdump-ipa-clones files * klp-convert * Documentation describing the end-to-end patch creation process, including the manual steps and how to use the above tools Did I miss anything? Then over time we can fill in the gaps (manual steps) with automation.
On 12/20/18 12:33 AM, Miroslav Benes wrote: > On Wed, 19 Dec 2018, Jiri Kosina wrote: > >> On Wed, 19 Dec 2018, Josh Poimboeuf wrote: >> >>> Also the commit message needs an analysis of the performance impacts. >> >> Agreed. Especially as it's expected (*) to be completely in the noise >> particularly for the kernel, it'd be good to have that documented in the >> changelog. >> >> (*) actually measured already for some subset of the IPA optimizations > > Ok, we can do that. I don't expect the results to be different from the > last measurement as Jiri mentions. The sets of disabled optimizations are > similar. > > I'll add it to v2. > > On Wed, 19 Dec 2018, Jiri Kosina wrote: > >> On Wed, 19 Dec 2018, Josh Poimboeuf wrote: >> >>>>> This option only makes sense for source-based patch generation, so isn't >>>>> it a bit premature to make this change without proper source-based patch >>>>> tooling? >>>> >>>> The reality is though that before the full-fledged patch tooling exists, >>>> people are actually already writing livepatches by hand, so this option is >>>> useful for them. >>> >>> Fair enough. > > Yes, that was the reason I sent it. It would not make sense to wait for > the tooling in this case, because -flive-patching is useful even now, > since there is a way to prepare livepatches without any tooling. > >>> Though, upstream, almost everybody seems to use kpatch-build, for which >>> this patch doesn't help. And people will continue to do so until we >>> have decent source-based tooling. Will the klp-convert patches be >>> revived soon? >> >> Let me add Joao, who's working on that. >> >> Joao, I think you had something basically ready for upstream exposure, >> right? > > I think that when Joao posted it a long time ago, the conclusion was that > it would be better to wait for the source-based tooling and have the > complete solution. I may misremember though. Your memories match mine, Miroslav. FTR, we recently integrated klp-convert to SLE. There were some fixes in comparison with the version which was submitted upstream, thus a v2 of the patches is necessary. > > If Josh thinks that it would be acceptable to have klp-convert merged even > without the tooling, I'm all for it. > Of course I can work on that and I'll be glad to do so / submit this new version, if this is now something considered useful. > We're about to start using it at SUSE and staying close to upstream would > definitely be better. > > Miroslav >
On Thu, 20 Dec 2018, Josh Poimboeuf wrote: > On Thu, Dec 20, 2018 at 09:33:05AM +0100, Miroslav Benes wrote: > > > > Though, upstream, almost everybody seems to use kpatch-build, for which > > > > this patch doesn't help. And people will continue to do so until we > > > > have decent source-based tooling. Will the klp-convert patches be > > > > revived soon? > > > > > > Let me add Joao, who's working on that. > > > > > > Joao, I think you had something basically ready for upstream exposure, > > > right? > > > > I think that when Joao posted it a long time ago, the conclusion was that > > it would be better to wait for the source-based tooling and have the > > complete solution. I may misremember though. > > > > If Josh thinks that it would be acceptable to have klp-convert merged even > > without the tooling, I'm all for it. > > > > We're about to start using it at SUSE and staying close to upstream would > > definitely be better. > > A complete toolchain should definitely be the end goal. > > But as a usable first step, only *some* of the tooling is required, > since some of the steps can be done manually, right? > > So starting out, for something usable, I believe the following would be > the bare minimum: > > * -flive-patching > > * The analysis tool which analyzes the -fdump-ipa-clones files > > * klp-convert > > * Documentation describing the end-to-end patch creation process, > including the manual steps and how to use the above tools > > Did I miss anything? > > Then over time we can fill in the gaps (manual steps) with automation. Agreed. Miroslav
On Thu, 20 Dec 2018, Joao Moreira wrote: > On 12/20/18 12:33 AM, Miroslav Benes wrote: > > On Wed, 19 Dec 2018, Jiri Kosina wrote: > > > >> On Wed, 19 Dec 2018, Josh Poimboeuf wrote: > >> > >>> Also the commit message needs an analysis of the performance impacts. > >> > >> Agreed. Especially as it's expected (*) to be completely in the noise > >> particularly for the kernel, it'd be good to have that documented in the > >> changelog. > >> > >> (*) actually measured already for some subset of the IPA optimizations > > > > Ok, we can do that. I don't expect the results to be different from the > > last measurement as Jiri mentions. The sets of disabled optimizations are > > similar. > > > > I'll add it to v2. > > > > On Wed, 19 Dec 2018, Jiri Kosina wrote: > > > >> On Wed, 19 Dec 2018, Josh Poimboeuf wrote: > >> > >>>>> This option only makes sense for source-based patch generation, so isn't > >>>>> it a bit premature to make this change without proper source-based patch > >>>>> tooling? > >>>> > >>>> The reality is though that before the full-fledged patch tooling exists, > >>>> people are actually already writing livepatches by hand, so this option > >>>> is > >>>> useful for them. > >>> > >>> Fair enough. > > > > Yes, that was the reason I sent it. It would not make sense to wait for > > the tooling in this case, because -flive-patching is useful even now, > > since there is a way to prepare livepatches without any tooling. > > > >>> Though, upstream, almost everybody seems to use kpatch-build, for which > >>> this patch doesn't help. And people will continue to do so until we > >>> have decent source-based tooling. Will the klp-convert patches be > >>> revived soon? > >> > >> Let me add Joao, who's working on that. > >> > >> Joao, I think you had something basically ready for upstream exposure, > >> right? > > > > I think that when Joao posted it a long time ago, the conclusion was that > > it would be better to wait for the source-based tooling and have the > > complete solution. I may misremember though. > > Your memories match mine, Miroslav. > > FTR, we recently integrated klp-convert to SLE. There were some fixes in > comparison with the version which was submitted upstream, thus a v2 of the > patches is necessary. > > > > > If Josh thinks that it would be acceptable to have klp-convert merged even > > without the tooling, I'm all for it. > > > Of course I can work on that and I'll be glad to do so / submit this new > version, if this is now something considered useful. Yes, please. Some context first. We decided to remove the integration into kbuild at SUSE. klp-convert is called from rpm .spec file directly after a livepatch module is compiled. I think we should preserve kbuild process in upstream though. It makes more sense there. Miroslav
diff --git a/Makefile b/Makefile index a0650bf79606..53f5ab810efe 100644 --- a/Makefile +++ b/Makefile @@ -778,6 +778,10 @@ KBUILD_CFLAGS_KERNEL += $(call cc-option,-ffunction-sections,) KBUILD_CFLAGS_KERNEL += $(call cc-option,-fdata-sections,) endif +ifdef CONFIG_LIVEPATCH +KBUILD_CFLAGS += $(call cc-option, -flive-patching=inline-clone) +endif + # arch Makefile may override CC so keep this after arch Makefile is included NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
GCC 9 introduces a new option, -flive-patching. It disables certain optimizations which could make a compilation unsafe for later live patching of the running kernel. The option is used only if CONFIG_LIVEPATCH is enabled and $(CC) supports it. Signed-off-by: Miroslav Benes <mbenes@suse.cz> --- Makefile | 4 ++++ 1 file changed, 4 insertions(+)