From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753269AbbKLGC4 (ORCPT ); Thu, 12 Nov 2015 01:02:56 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51775 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750908AbbKLGCy (ORCPT ); Thu, 12 Nov 2015 01:02:54 -0500 Date: Thu, 12 Nov 2015 01:02:50 -0500 From: Jessica Yu To: Petr Mladek Cc: Rusty Russell , Josh Poimboeuf , Seth Jennings , Jiri Kosina , Vojtech Pavlik , Miroslav Benes , linux-api@vger.kernel.org, live-patching@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org Subject: Re: samples: livepatch: init reloc list and mark as klp module Message-ID: <20151112060249.GA5841@packer-debian-8-amd64.digitalocean.com> References: <1447130755-17383-1-git-send-email-jeyu@redhat.com> <1447130755-17383-5-git-send-email-jeyu@redhat.com> <20151111154249.GQ2599@pathway.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20151111154249.GQ2599@pathway.suse.cz> X-OS: Linux eisen.io 3.16.0-4-amd64 x86_64 User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org +++ Petr Mladek [11/11/15 16:42 +0100]: >On Mon 2015-11-09 23:45:54, Jessica Yu wrote: >> Intialize the list of relocation sections in the sample >> klp_object (even if the list will be empty in this case). >> Also mark module as a livepatch module so that the module >> loader can appropriately initialize it. >> >> Signed-off-by: Jessica Yu >> --- >> samples/livepatch/livepatch-sample.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/samples/livepatch/livepatch-sample.c b/samples/livepatch/livepatch-sample.c >> index fb8c861..2ef9345 100644 >> --- a/samples/livepatch/livepatch-sample.c >> +++ b/samples/livepatch/livepatch-sample.c >> @@ -89,3 +90,4 @@ static void livepatch_exit(void) >> module_init(livepatch_init); >> module_exit(livepatch_exit); >> MODULE_LICENSE("GPL"); >> +MODULE_INFO(livepatch, "Y"); > >This looks a bit error prone. I wonder if we could detect this >information another way. For example, by a check for the >livepatch-related elf sections. If it is missing, >we do not need to preserve struct load_info even >when it is a livepatch. Yeah, I agree that it is unnecessary for a livepatch module without reloc secs to keep a copy of the load_info struct. My justification for using MODULE_INFO is that I was trying to be consistent with the way how other module "characteristics" are checked in the module loader. For example, if the module came from the staging tree, the module loader simply checks get_modinfo(info, "staging")). If the module is a livepatch module, we check get_modinfo(info, "livepatch")). I also thought that it might be useful additional information for the user to be able to issue the modinfo command on a module to see if it's a livepatch module or not (but maybe this information won't be so useful after all, that's quite subjective). But if we want to do a more thorough check, we could, like you said, check for the livepatch-related elf sections before copying load_info. Thanks, Jessica From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jessica Yu Subject: Re: samples: livepatch: init reloc list and mark as klp module Date: Thu, 12 Nov 2015 01:02:50 -0500 Message-ID: <20151112060249.GA5841@packer-debian-8-amd64.digitalocean.com> References: <1447130755-17383-1-git-send-email-jeyu@redhat.com> <1447130755-17383-5-git-send-email-jeyu@redhat.com> <20151111154249.GQ2599@pathway.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Return-path: Content-Disposition: inline In-Reply-To: <20151111154249.GQ2599-KsEp0d+Q8qECVLCxKZUutA@public.gmane.org> Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Petr Mladek Cc: Rusty Russell , Josh Poimboeuf , Seth Jennings , Jiri Kosina , Vojtech Pavlik , Miroslav Benes , linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, live-patching-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-api@vger.kernel.org +++ Petr Mladek [11/11/15 16:42 +0100]: >On Mon 2015-11-09 23:45:54, Jessica Yu wrote: >> Intialize the list of relocation sections in the sample >> klp_object (even if the list will be empty in this case). >> Also mark module as a livepatch module so that the module >> loader can appropriately initialize it. >> >> Signed-off-by: Jessica Yu >> --- >> samples/livepatch/livepatch-sample.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/samples/livepatch/livepatch-sample.c b/samples/livepatch/livepatch-sample.c >> index fb8c861..2ef9345 100644 >> --- a/samples/livepatch/livepatch-sample.c >> +++ b/samples/livepatch/livepatch-sample.c >> @@ -89,3 +90,4 @@ static void livepatch_exit(void) >> module_init(livepatch_init); >> module_exit(livepatch_exit); >> MODULE_LICENSE("GPL"); >> +MODULE_INFO(livepatch, "Y"); > >This looks a bit error prone. I wonder if we could detect this >information another way. For example, by a check for the >livepatch-related elf sections. If it is missing, >we do not need to preserve struct load_info even >when it is a livepatch. Yeah, I agree that it is unnecessary for a livepatch module without reloc secs to keep a copy of the load_info struct. My justification for using MODULE_INFO is that I was trying to be consistent with the way how other module "characteristics" are checked in the module loader. For example, if the module came from the staging tree, the module loader simply checks get_modinfo(info, "staging")). If the module is a livepatch module, we check get_modinfo(info, "livepatch")). I also thought that it might be useful additional information for the user to be able to issue the modinfo command on a module to see if it's a livepatch module or not (but maybe this information won't be so useful after all, that's quite subjective). But if we want to do a more thorough check, we could, like you said, check for the livepatch-related elf sections before copying load_info. Thanks, Jessica