Re: [PATCH 2/5] kexec: do not special-case the -s option

["Michal Suchánek" <msuchanek@suse.de>]

On Fri, 2 Mar 2018 13:36:16 +0100
Simon Horman <horms@verge.net.au> wrote:

> On Mon, Feb 26, 2018 at 01:00:35PM +0100, Michal Suchanek wrote:
> > It is parsed separately to save a few CPU cycles when setting up
> > other options but it just complicates the code. So fold it back and
> > set up all flags both for KEXEC_LOAD and KEXEC_FILE_LOAD
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  kexec/kexec.c | 25 ++++---------------------
> >  1 file changed, 4 insertions(+), 21 deletions(-)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index ab8cff7fe083..9ea102e1565a 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
> >  	};
> >  	static const char short_options[] = KEXEC_ALL_OPT_STR;
> >  
> > -	/*
> > -	 * First check if --use-kexec-file-syscall is set. That
> > changes lot of
> > -	 * things
> > -	 */
> > -	while ((opt = getopt_long(argc, argv, short_options,
> > -				  options, 0)) != -1) {
> > -		switch(opt) {
> > -		case OPT_KEXEC_FILE_SYSCALL:
> > -			do_kexec_file_syscall = 1;
> > -			break;
> > -		}
> > -	}
> > -
> >  	/* Reset getopt for the next pass. */
> >  	opterr = 1;
> >  	optind = 1;
> > @@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
> >  			do_shutdown = 0;
> >  			do_sync = 0;
> >  			do_unload = 1;
> > -			if (do_kexec_file_syscall)
> > -				kexec_file_flags |=
> > KEXEC_FILE_UNLOAD;
> > +			kexec_file_flags |= KEXEC_FILE_UNLOAD;
> >  			break;
> >  		case OPT_EXEC:
> >  			do_load = 0;
> > @@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
> >  			do_exec = 0;
> >  			do_shutdown = 0;
> >  			do_sync = 0;
> > -			if (do_kexec_file_syscall)
> > -				kexec_file_flags |=
> > KEXEC_FILE_ON_CRASH;
> > -			else
> > -				kexec_flags = KEXEC_ON_CRASH;
> > -			break;
> > +			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> > +			kexec_flags = KEXEC_ON_CRASH;  
> 
> This appears to change the logic of options parsing.
> 

Care to share how exactly is the logic changed? I do not see it. To me
it looks like we have separate flags for KEXEC_LOAD and KEXEC_FILE_LOAD
so setting up both does not change anything. Whatever syscall is used
it has the flags set up.

In the existing logic we decide which syscall to do beforehand and set
up only flags for that syscall but that does not save much CPU cycles
and makes the code more complex and fragile. 

Thanks

Michal

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec