Live-Patching Archive on lore.kernel.org
 help / color / Atom feed
From: Joe Lawrence <joe.lawrence@redhat.com>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	live-patching@vger.kernel.org,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>
Subject: Re: [PATCH v4 06/10] modpost: Add modinfo flag to livepatch modules
Date: Mon, 12 Aug 2019 11:56:26 -0400
Message-ID: <20190812155626.GA19845@redhat.com> (raw)
In-Reply-To: <CAK7LNAQuS-YcXecfJ21BGzc0CimzWxQcYST5-1xRgnCQGtcL4A@mail.gmail.com>

On Wed, Jul 31, 2019 at 02:58:27PM +0900, Masahiro Yamada wrote:
> Hi Joe,
> 
> 
> On Thu, May 9, 2019 at 11:39 PM Joe Lawrence <joe.lawrence@redhat.com> wrote:
> >
> > From: Miroslav Benes <mbenes@suse.cz>
> >
> > Currently, livepatch infrastructure in the kernel relies on
> > MODULE_INFO(livepatch, "Y") statement in a livepatch module. Then the
> > kernel module loader knows a module is indeed livepatch module and can
> > behave accordingly.
> >
> > klp-convert, on the other hand relies on LIVEPATCH_* statement in the
> > module's Makefile for exactly the same reason.
> >
> > Remove dependency on modinfo and generate MODULE_INFO flag
> > automatically in modpost when LIVEPATCH_* is defined in the module's
> > Makefile. Generate a list of all built livepatch modules based on
> > the .livepatch file and store it in (MODVERDIR)/livepatchmods. Give
> > this list as an argument for modpost which will use it to identify
> > livepatch modules.
> >
> > As MODULE_INFO is no longer needed, remove it.
> 
> 
> I do not understand this patch.
> This makes the implementation so complicated.
> 
> I think MODULE_INFO(livepatch, "Y") is cleaner than
> LIVEPATCH_* in Makefile.
> 
> 
> How about this approach?
> 
> 
> [1] Make modpost generate the list of livepatch modules.
>     (livepatch-modules)
> 
> [2] Generate Symbols.list in scripts/Makefile.modpost
>     (vmlinux + modules excluding livepatch-modules)
> 
> [3] Run klp-convert for modules in livepatch-modules.
> 
> 
> If you do this, you can remove most of the build system hacks
> can't you?
> 
> 
> I attached an example implementation for [1].
> 
> Please check whether this works.
> 

Hi Masahiro, 

I tested and step [1] that you attached did create the livepatch-modules
as expected.  Thanks for that example, it does look cleaner that what
we had in the patchset.

I'm admittedly out of my element with kbuild changes, but here are my
naive attempts at steps [2] and [3]...


[step 2] generate Symbols.list - I tacked this on as a dependency of the
$(modules:.ko=.mod.o), but there is probably a better more logical place
to put it.  Also used grep -Fxv to exclude the livepatch-modules list
from the modules.order list of modules to process.

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 3eca7fccadd4..5409bbc212bb 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -111,7 +111,23 @@ quiet_cmd_cc_o_c = CC      $@
       cmd_cc_o_c = $(CC) $(c_flags) $(KBUILD_CFLAGS_MODULE) $(CFLAGS_MODULE) \
 		   -c -o $@ $<
 
-$(modules:.ko=.mod.o): %.mod.o: %.mod.c FORCE
+quiet_cmd_klp_map = KLP     Symbols.list
+SLIST = $(objtree)/Symbols.list
+
+define cmd_symbols_list
+	$(shell echo "klp-convert-symbol-data.0.1" > $(objtree)/Symbols.list)			\
+	$(shell echo "*vmlinux" >> $(objtree)/Symbols.list)					\
+	$(shell nm -f posix $(objtree)/vmlinux | cut -d\  -f1 >> $(objtree)/Symbols.list)	\
+	$(foreach ko, $(sort $(shell grep -Fxv -f livepatch-modules modules.order)),		\
+		$(shell echo "*$(shell basename -s .ko $(ko))" >> $(objtree)/Symbols.list)	\
+		$(shell nm -f posix $(patsubst %.ko,%.o,$(ko)) | cut -d\  -f1 >> $(objtree)/Symbols.list))
+endef
+
+Symbols.list: __modpost
+	$(if $(CONFIG_LIVEPATCH), $(call cmd,symbols_list))
+
+
+$(modules:.ko=.mod.o): %.mod.o: %.mod.c Symbols.list FORCE
 	$(call if_changed_dep,cc_o_c)
 
 targets += $(modules:.ko=.mod.o)
-- 
2.18.1

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--



[step 3] klp-convert the livepatch-modules - more or less what existed
in the patchset already, however used the grep -Fx trick to process only
modules found in livepatch-modules file:

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 73e80b917f12..f085644c2b97 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -223,6 +223,8 @@ endif
 # (needed for the shell)
 make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst $$,$$$$,$(cmd_$(1)))))
 
+save-cmd = printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd
+
 # Find any prerequisites that is newer than target or that does not exist.
 # PHONY targets skipped in both cases.
 any-prereq = $(filter-out $(PHONY),$?)$(filter-out $(PHONY) $(wildcard $^),$^)
@@ -230,7 +232,7 @@ any-prereq = $(filter-out $(PHONY),$?)$(filter-out $(PHONY) $(wildcard $^),$^)
 # Execute command if command has changed or prerequisite(s) are updated.
 if_changed = $(if $(any-prereq)$(cmd-check),                                 \
 	$(cmd);                                                              \
-	printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
+	$(save-cmd), @:)
 
 # Execute the command and also postprocess generated .d dependencies file.
 if_changed_dep = $(if $(any-prereq)$(cmd-check),$(cmd_and_fixdep),@:)
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 5409bbc212bb..bc3b7b9dd8fa 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -142,8 +142,22 @@ quiet_cmd_ld_ko_o = LD [M]  $@
                  -o $@ $(real-prereqs) ;                                \
 	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 
+SLIST = $(objtree)/Symbols.list
+KLP_CONVERT = scripts/livepatch/klp-convert
+quiet_cmd_klp_convert = KLP     $@
+      cmd_klp_convert = mv $@ $(@:.ko=.klp.o);				\
+			$(KLP_CONVERT) $(SLIST) $(@:.ko=.klp.o) $@
+
+define rule_ld_ko_o
+	$(Q)$(call echo-cmd,ld_ko_o) $(cmd_ld_ko_o) ;				\
+	$(call save-cmd,ld_ko_o) ;						\
+	$(if $(CONFIG_LIVEPATCH),						\
+		$(if $(shell grep -Fx "$@" livepatch-modules),			\
+			$(call echo-cmd,klp_convert) $(cmd_klp_convert)))
+endef
+
 $(modules): %.ko :%.o %.mod.o FORCE
-	+$(call if_changed,ld_ko_o)
+	+$(call if_changed_rule,ld_ko_o)
 
 targets += $(modules)
 
-- 
2.18.1

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--


Thanks,

-- Joe

  reply index

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190509143859.9050-1-joe.lawrence@redhat.com>
     [not found] ` <alpine.LSU.2.21.1906131451560.22698@pobox.suse.cz>
     [not found]   ` <b1a627a4-3702-9689-6c03-0c2123c06a2d@redhat.com>
     [not found]     ` <c9021573-11c6-b576-0aa6-97754c98a06e@redhat.com>
     [not found]       ` <20190614083435.uq3mk6mprbatysol@pathway.suse.cz>
2019-06-25 11:36         ` [PATCH v4 00/10] klp-convert livepatch build tooling Miroslav Benes
2019-06-25 13:24           ` Joe Lawrence
2019-06-25 19:08           ` Joe Lawrence
2019-06-26 10:27             ` Miroslav Benes
     [not found] ` <20190509143859.9050-4-joe.lawrence@redhat.com>
2019-07-31  2:50   ` [PATCH v4 03/10] livepatch: Add klp-convert tool Masahiro Yamada
2019-07-31  3:36     ` Masahiro Yamada
2019-08-09 18:42       ` Joe Lawrence
2019-08-13  1:15         ` Masahiro Yamada
     [not found] ` <20190509143859.9050-7-joe.lawrence@redhat.com>
2019-07-31  5:58   ` [PATCH v4 06/10] modpost: Add modinfo flag to livepatch modules Masahiro Yamada
2019-08-12 15:56     ` Joe Lawrence [this message]
2019-08-15 15:05       ` Masahiro Yamada
2019-08-16  8:19         ` Miroslav Benes
2019-08-16 12:43           ` Joe Lawrence
2019-08-16 19:01             ` Joe Lawrence
2019-08-19  3:50               ` Masahiro Yamada
2019-08-19 15:55                 ` Joe Lawrence
2019-08-20  7:54                   ` Miroslav Benes
2019-08-19  3:49             ` Masahiro Yamada
2019-08-19  7:31               ` Miroslav Benes
2019-08-19 16:02                 ` Joe Lawrence
2019-08-22  3:35                   ` Masahiro Yamada
2019-08-13 10:26     ` Miroslav Benes
     [not found] ` <20190509143859.9050-8-joe.lawrence@redhat.com>
2019-08-16 11:35   ` [PATCH v4 07/10] livepatch: Add sample livepatch module Masahiro Yamada
2019-08-16 12:47     ` Joe Lawrence

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190812155626.GA19845@redhat.com \
    --to=joe.lawrence@redhat.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Live-Patching Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/live-patching/0 live-patching/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 live-patching live-patching/ https://lore.kernel.org/live-patching \
		live-patching@vger.kernel.org
	public-inbox-index live-patching

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.live-patching


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git