Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
>>> On 24.11.15 at 18:51, wrote: > --- /dev/null > +++ b/xen/include/linux/kconfig.h > @@ -0,0 +1,54 @@ > +#ifndef __LINUX_KCONFIG_H > +#define __LINUX_KCONFIG_H Neither placement in the source tree nor guard variable should say "Linux". > --- /dev/null > +++ b/xen/scripts/Makefile.host > @@ -0,0 +1,128 @@ We already have ways to build host programs. Do we really need a second mechanism, the more an abstract one? I'd prefer a more minimalistic approach (confined to the xen/scripts/kconfig/ subtree, which imo actually should be xen/tools/kconfig/). In any even I'd expect, for everything outside of the actual kconfig/ subtree, a few words in the commit message towards the rationale for including those pieces. That'll help future cleanup by clarifying what certain pieces are actually needed for. > --- /dev/null > +++ b/xen/scripts/kconfig/.gitignore > @@ -0,0 +1,22 @@ Does the top level one (perhaps suitably adjusted) not fit the needs? > --- /dev/null > +++ b/xen/scripts/kconfig/Makefile.linux > @@ -0,0 +1,317 @@ This doesn't seem to be referenced anywhere. > --- /dev/null > +++ b/xen/scripts/kconfig/POTFILES.in > @@ -0,0 +1,12 @@ Do we really need this? > +void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const > char *), void *data, int prevtoken) > +{ > + if (!e) { > + fn(data, NULL, "y"); > + return; > + } > + > + if (expr_compare_type(prevtoken, e->type) > 0) > + fn(data, NULL, "("); > + switch (e->type) { > + case E_SYMBOL: > + if (e->left.sym->name) > + fn(data, e->left.sym, e->left.sym->name); > + else > + fn(data, NULL, ""); > + break; > + case E_NOT: > + fn(data, NULL, "!"); > + expr_print(e->left.expr, fn, data, E_NOT); > + break; > + case E_EQUAL: > + if (e->left.sym->name) > + fn(data, e->left.sym, e->left.sym->name); > + else > + fn(data, NULL, ""); > + fn(data, NULL, "="); > + fn(data, e->right.sym, e->right.sym->name); > + break; > + case E_LEQ: > + case E_LTH: > + if (e->left.sym->name) > + fn(data, e->left.sym, e->left.sym->name); > + else > + fn(data, NULL, ""); > + fn(data, NULL, e->type == E_LEQ ? "<=" : "<"); > + fn(data, e->right.sym, e->right.sym->name); > + break; > + case E_GEQ: > + case E_GTH: > + if (e->left.sym->name) > + fn(data, e->left.sym, e->left.sym->name); > + else > + fn(data, NULL, ""); > + fn(data, NULL, e->type == E_LEQ ? ">=" : ">"); Please eliminate the copy-and-paste mistake here right away (see Linux commit f6aad2615c). > --- /dev/null > +++ b/xen/scripts/kconfig/gconf.c > @@ -0,0 +1,1521 @@ I think I had asked before, and with us not wanting any user visible prompts for now I wonder even more: Do we really need [gmnq]conf? All I think we really need is conf. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
Doug Goldstein writes ("Re: [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2"): > The whole point here was to bring in the kconfig bits from Linux 4.2 > untouched for traceability for where the code came from. By doing it > this way it allows Xen to rebase kconfig support to a newer version much > easier since its all self-contained. I think this is a very good point. I would go further than this, and wholesale import the relevant files into a subdirectory in a single commit that contains nothing else, and mention the specific git commit id and version number in the Linux tree in the commit message. Something like Import scripts/kconfig from Linux 4.2.0 commit id abcdefgh into our own xen/scripts/kconfig. So far, these are unused - they will be wired into our own build system shortly. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
On 11/30/15 7:59 AM, Jan Beulich wrote: On 24.11.15 at 18:51, wrote: >> --- /dev/null >> +++ b/xen/include/linux/kconfig.h >> @@ -0,0 +1,54 @@ >> +#ifndef __LINUX_KCONFIG_H >> +#define __LINUX_KCONFIG_H > > Neither placement in the source tree nor guard variable should say > "Linux". This file gets removed later in the series. I meant to squash that into this patch. > >> --- /dev/null >> +++ b/xen/scripts/Makefile.host >> @@ -0,0 +1,128 @@ > > We already have ways to build host programs. Do we really need a > second mechanism, the more an abstract one? I'd prefer a more > minimalistic approach (confined to the xen/scripts/kconfig/ subtree, > which imo actually should be xen/tools/kconfig/). In any even I'd > expect, for everything outside of the actual kconfig/ subtree, a > few words in the commit message towards the rationale for including > those pieces. That'll help future cleanup by clarifying what certain > pieces are actually needed for. I can move it into xen/scripts/kconfig. The Xen make rules to build host programs leave things to be desired and its actually necessary to use these to build the different config variants. Earlier in the review process (or really the RFC) I suggested bringing those parts of Kbuild into Xen, which would have the effect of providing the ability to build all the necessary pieces but I was told not to. So I'll just move this file into xen/scripts/kconfig. I can also move everything to xen/tools/kconfig. > >> --- /dev/null >> +++ b/xen/scripts/kconfig/.gitignore >> @@ -0,0 +1,22 @@ > > Does the top level one (perhaps suitably adjusted) not fit the needs? The whole point here was to bring in the kconfig bits from Linux 4.2 untouched for traceability for where the code came from. By doing it this way it allows Xen to rebase kconfig support to a newer version much easier since its all self-contained. > >> --- /dev/null >> +++ b/xen/scripts/kconfig/Makefile.linux >> @@ -0,0 +1,317 @@ > > This doesn't seem to be referenced anywhere. Its the central file for building kconfig. Its refereinced in xen/scripts/kconfig/Makefile on line 63. This is the only file I've renamed from its original name in the Linux source tree. > >> --- /dev/null >> +++ b/xen/scripts/kconfig/POTFILES.in >> @@ -0,0 +1,12 @@ > > Do we really need this? If we don't expect to provide localization of kconfig, then no. Granted I didn't wire up the other localization bits. > >> +void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const >> char *), void *data, int prevtoken) >> +{ >> +if (!e) { >> +fn(data, NULL, "y"); >> +return; >> +} >> + >> +if (expr_compare_type(prevtoken, e->type) > 0) >> +fn(data, NULL, "("); >> +switch (e->type) { >> +case E_SYMBOL: >> +if (e->left.sym->name) >> +fn(data, e->left.sym, e->left.sym->name); >> +else >> +fn(data, NULL, ""); >> +break; >> +case E_NOT: >> +fn(data, NULL, "!"); >> +expr_print(e->left.expr, fn, data, E_NOT); >> +break; >> +case E_EQUAL: >> +if (e->left.sym->name) >> +fn(data, e->left.sym, e->left.sym->name); >> +else >> +fn(data, NULL, ""); >> +fn(data, NULL, "="); >> +fn(data, e->right.sym, e->right.sym->name); >> +break; >> +case E_LEQ: >> +case E_LTH: >> +if (e->left.sym->name) >> +fn(data, e->left.sym, e->left.sym->name); >> +else >> +fn(data, NULL, ""); >> +fn(data, NULL, e->type == E_LEQ ? "<=" : "<"); >> +fn(data, e->right.sym, e->right.sym->name); >> +break; >> +case E_GEQ: >> +case E_GTH: >> +if (e->left.sym->name) >> +fn(data, e->left.sym, e->left.sym->name); >> +else >> +fn(data, NULL, ""); >> +fn(data, NULL, e->type == E_LEQ ? ">=" : ">"); > > Please eliminate the copy-and-paste mistake here right away (see > Linux commit f6aad2615c). In an effort to keep the history clean at 4.2, I'll pull this in as an additional patch to the series. > >> --- /dev/null >> +++ b/xen/scripts/kconfig/gconf.c >> @@ -0,0 +1,1521 @@ > > I think I had asked before, and with us not wanting any user visible > prompts for now I wonder even more: Do we really need [gmnq]conf? > All I think we really need is conf. > > Jan > Well as you (and others) have commented about the fact that KEXEC and the ARM UARTs should be user configurable which is why I included those. So I'll continue to leave those in ok? -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
>>> On 30.11.15 at 16:34, wrote: > On 11/30/15 7:59 AM, Jan Beulich wrote: > On 24.11.15 at 18:51, wrote: >>> --- /dev/null >>> +++ b/xen/scripts/kconfig/Makefile.linux >>> @@ -0,0 +1,317 @@ >> >> This doesn't seem to be referenced anywhere. > > Its the central file for building kconfig. Its refereinced in > xen/scripts/kconfig/Makefile on line 63. This is the only file I've > renamed from its original name in the Linux source tree. In a later patch I think? >>> --- /dev/null >>> +++ b/xen/scripts/kconfig/gconf.c >>> @@ -0,0 +1,1521 @@ >> >> I think I had asked before, and with us not wanting any user visible >> prompts for now I wonder even more: Do we really need [gmnq]conf? >> All I think we really need is conf. > > Well as you (and others) have commented about the fact that KEXEC and > the ARM UARTs should be user configurable which is why I included those. > So I'll continue to leave those in ok? Afaic I'd prefer a minimal import as a first step. Adding further frontends - if really needed - could be done later. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
On 11/30/15 9:55 AM, Jan Beulich wrote: On 30.11.15 at 16:34, wrote: >> On 11/30/15 7:59 AM, Jan Beulich wrote: >> On 24.11.15 at 18:51, wrote: --- /dev/null +++ b/xen/scripts/kconfig/Makefile.linux @@ -0,0 +1,317 @@ >>> >>> This doesn't seem to be referenced anywhere. >> >> Its the central file for building kconfig. Its refereinced in >> xen/scripts/kconfig/Makefile on line 63. This is the only file I've >> renamed from its original name in the Linux source tree. > > In a later patch I think? Yes. This whole patch is just bringing in files from Linux 4.2.0 unmodified. I plan on modifying the commit message like Ian Jackson suggested. > --- /dev/null +++ b/xen/scripts/kconfig/gconf.c @@ -0,0 +1,1521 @@ >>> >>> I think I had asked before, and with us not wanting any user visible >>> prompts for now I wonder even more: Do we really need [gmnq]conf? >>> All I think we really need is conf. >> >> Well as you (and others) have commented about the fact that KEXEC and >> the ARM UARTs should be user configurable which is why I included those. >> So I'll continue to leave those in ok? > > Afaic I'd prefer a minimal import as a first step. Adding further > frontends - if really needed - could be done later. > > Jan > So you want me to break this patch up to bring in the frontends individually? Since there is a request to have KEXEC and the UARTs configurable by the user I would bring in all these frontends. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
>>> On 30.11.15 at 16:42, wrote: > Doug Goldstein writes ("Re: [PATCHv6] 01/28] build: import Kbuild/Kconfig > from Linux 4.2"): >> The whole point here was to bring in the kconfig bits from Linux 4.2 >> untouched for traceability for where the code came from. By doing it >> this way it allows Xen to rebase kconfig support to a newer version much >> easier since its all self-contained. > > I think this is a very good point. > > I would go further than this, and wholesale import the relevant files > into a subdirectory in a single commit that contains nothing else, and > mention the specific git commit id and version number in the Linux > tree in the commit message. But what's the point of importing things we're not going to use? Imo such an import should be limited to those files that (later on) would actually get wired up. Hence the request to do a minimalistic import for this first round. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
Doug Goldstein writes ("Re: [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2"): > On 11/30/15 9:55 AM, Jan Beulich wrote: >> On 30.11.15 at 16:34, wrote: > > Afaic I'd prefer a minimal import as a first step. Adding further > > frontends - if really needed - could be done later. > > So you want me to break this patch up to bring in the frontends > individually? Since there is a request to have KEXEC and the UARTs > configurable by the user I would bring in all these frontends. I may be disagreeing with Jan here, but I think the import should have the simplest glob pattern or filename rules (consistent with not importing vastly too mkuch), rather than necessarily the smallest set of files. We want to be able to do future imports with a minimum of fuss. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
Jan Beulich writes ("Re: [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2"): > On 30.11.15 at 16:42, wrote: > > I would go further than this, and wholesale import the relevant files > > into a subdirectory in a single commit that contains nothing else, and > > mention the specific git commit id and version number in the Linux > > tree in the commit message. > > But what's the point of importing things we're not going to use? I thought I had explained that. A future re-import or merge will be much simpler if this import is a simple wholesale copy of a directory or two or a simple glob pattern. The runes used to do the import could be in the commit message for easy reproduction. > Imo such an import should be limited to those files that (later on) > would actually get wired up. Hence the request to do a minimalistic > import for this first round. This would involve a series of piece-by-piece imports. Any future merge or re-import would involve retracing and re-executing those steps. It appears that we do disagree. Perhaps the basic thing is: I don't think we want to fork this code. We want to be a downstream of Linux for it. If necessary we may edit, to an extent, the parts we import, but hopefully we'll keep that to a minimum. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
On 11/30/15 11:04 AM, Ian Jackson wrote: > Doug Goldstein writes ("Re: [PATCHv6] 01/28] build: import Kbuild/Kconfig > from Linux 4.2"): >> On 11/30/15 9:55 AM, Jan Beulich wrote: >>> On 30.11.15 at 16:34, wrote: >>> Afaic I'd prefer a minimal import as a first step. Adding further >>> frontends - if really needed - could be done later. >> >> So you want me to break this patch up to bring in the frontends >> individually? Since there is a request to have KEXEC and the UARTs >> configurable by the user I would bring in all these frontends. > > I may be disagreeing with Jan here, but I think the import should have > the simplest glob pattern or filename rules (consistent with not > importing vastly too mkuch), rather than necessarily the smallest set > of files. > > We want to be able to do future imports with a minimum of fuss. > > Ian. > Ok. So do the minimum of kconfig in the first patch. Then import kconfig frontends in another patch? Later if you want to update any parts of kconfig they would need to be updated together (e.g. frontends and the kconfig build bits). I can do that. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
On Mon, 2015-11-30 at 11:00 -0600, Doug Goldstein wrote: > Since there is a request to have KEXEC and the UARTs > configurable by the user Who asked for this? I have quite a strong preference for not adding _any_ new[*] user configurable options in this first pass, since I think those need to be considered quite carefully whereas this first series should be largely about the mechanics of introducing Kconfig files. Ian. [*] i.e. anything which is not already controllable by the current .config driven thing. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
On Mon, 2015-11-30 at 17:16 +, Ian Jackson wrote: > It appears that we do disagree. Perhaps the basic thing is: I don't > think we want to fork this code. We want to be a downstream of Linux > for it. If necessary we may edit, to an extent, the parts we import, > but hopefully we'll keep that to a minimum. I agree with this. I think I previously objected to suggesting to rename the data files Xconfig instead of Kconfiug on this basis for example. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
>>> On 30.11.15 at 18:16, wrote: > Jan Beulich writes ("Re: [PATCHv6] 01/28] build: import Kbuild/Kconfig from > Linux 4.2"): >> On 30.11.15 at 16:42, wrote: >> > I would go further than this, and wholesale import the relevant files >> > into a subdirectory in a single commit that contains nothing else, and >> > mention the specific git commit id and version number in the Linux >> > tree in the commit message. >> >> But what's the point of importing things we're not going to use? > > I thought I had explained that. > > A future re-import or merge will be much simpler if this import is a > simple wholesale copy of a directory or two or a simple glob pattern. > > The runes used to do the import could be in the commit message for > easy reproduction. > >> Imo such an import should be limited to those files that (later on) >> would actually get wired up. Hence the request to do a minimalistic >> import for this first round. > > This would involve a series of piece-by-piece imports. Any future > merge or re-import would involve retracing and re-executing those > steps. > > It appears that we do disagree. Perhaps the basic thing is: I don't > think we want to fork this code. We want to be a downstream of Linux > for it. If necessary we may edit, to an extent, the parts we import, > but hopefully we'll keep that to a minimum. But not forking doesn't mean importing the whole directory. Some Makefile adjustments will be necessary anyway, so removing some of the frontends wouldn't make things worse. We indeed should avoid editing files we import, but I don't see any bad in skipping some of the files. Reasons I'd like to avoid importing everything: - no introduction of new build dependencies (one of the frontends being written in C++ is the mildest form, requiring HOSTCXX to be set), - limiting the amount of new code that needs maintaining (no matter how simple a re-import, it still is work, and hence the less frequently we need to do this, the better; I assume you agree that the likelihood for changes that we want/need to pull in grows with the size of the code, and with what I propose the import size would shrink by almost 50%), unless you or Doug volunteer to look after this code on a regular basis, - avoiding code (scripts) that seem completely pointless in our environment (e.g. streamline_config.pl). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
>>> On 30.11.15 at 18:00, wrote: > So you want me to break this patch up to bring in the frontends > individually? Since there is a request to have KEXEC and the UARTs > configurable by the user I would bring in all these frontends. No, I want them to be left out entirely. But it looks like we first need to settle on what we actually (don't) want. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
On 11/30/15 11:19 AM, Ian Campbell wrote: > On Mon, 2015-11-30 at 11:00 -0600, Doug Goldstein wrote: >> Since there is a request to have KEXEC and the UARTs >> configurable by the user > > Who asked for this? > > I have quite a strong preference for not adding _any_ new[*] user > configurable options in this first pass, since I think those need to be > considered quite carefully whereas this first series should be largely > about the mechanics of introducing Kconfig files. > > Ian. > > [*] i.e. anything which is not already controllable by the current .config > driven thing. > The ARM UARTs are the take away I had from conversations with Julien Grall and reading past comments on the ML how people can change the ARM UARTs. Obviously if that's not desired I can drop that. I originally had them enabled as they are in config/arm{32,64}.mk and changed them to be user configurable later in the series. As far as KEXEC goes, its a user configurable option now in Rules.mk. You can build "make kexec=n" and it will disable it. I chose that one as an original example in v1 of how a user configurable option would look in this scheme. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
>>> On 01.12.15 at 21:04, wrote: > On 11/30/15 11:19 AM, Ian Campbell wrote: >> On Mon, 2015-11-30 at 11:00 -0600, Doug Goldstein wrote: >>> Since there is a request to have KEXEC and the UARTs >>> configurable by the user >> >> Who asked for this? >> >> I have quite a strong preference for not adding _any_ new[*] user >> configurable options in this first pass, since I think those need to be >> considered quite carefully whereas this first series should be largely >> about the mechanics of introducing Kconfig files. >> >> Ian. >> >> [*] i.e. anything which is not already controllable by the current .config >> driven thing. >> > > The ARM UARTs are the take away I had from conversations with Julien > Grall and reading past comments on the ML how people can change the ARM > UARTs. Obviously if that's not desired I can drop that. I originally had > them enabled as they are in config/arm{32,64}.mk and changed them to be > user configurable later in the series. > > As far as KEXEC goes, its a user configurable option now in Rules.mk. > You can build "make kexec=n" and it will disable it. I chose that one as > an original example in v1 of how a user configurable option would look > in this scheme. And note how Ian said "_any_ new". In fact as a follow-up we should convert other command line driven overrides to config options (debug, perfc, bigmem, shadow) imo. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
On Tue, 2015-12-01 at 14:04 -0600, Doug Goldstein wrote: > On 11/30/15 11:19 AM, Ian Campbell wrote: > > On Mon, 2015-11-30 at 11:00 -0600, Doug Goldstein wrote: > > > Since there is a request to have KEXEC and the UARTs > > > configurable by the user > > > > Who asked for this? > > > > I have quite a strong preference for not adding _any_ new[*] user > > configurable options in this first pass, since I think those need to be > > considered quite carefully whereas this first series should be largely > > about the mechanics of introducing Kconfig files. > > > > Ian. > > > > [*] i.e. anything which is not already controllable by the current > > .config > > driven thing. > > > > The ARM UARTs are the take away I had from conversations with Julien > Grall AIUI all Julien was asking for was that the same set of UARTs should be enabled for arm32 or arm64 both before and after this series, not that they should be user configurable (in this first pass at least). TBH I think this series is getting bogged down in trying to do much all at the same time, switching to Kconfig, making things newly user-selectable, arranging for out of tree builds, some of which are either (potentially) controversial or simply result in apparently unnecessary changes if the reviewer is only expecting a subset of those changes (especially those only expecting the first), leading to a few RTTs of back and forth with reviewers. I would encourage you to par this first series back to the simplest possible "switch to Kconfig, retaining the exact same set of user facing options as today" and avoid feature creep from people requesting new and exciting things which the switch to Kconfig makes possible. That way we can make progress on a mostly mechanical switch to Kconfig without continuously getting side tracked on questions like whether this or that should be user selectable or not. All of the "feature-creep" (which I don't mean in the pejorative sense) things can easily be done in a follow up series. > and reading past comments on the ML how people can change the ARM > UARTs. Obviously if that's not desired I can drop that. I originally had > them enabled as they are in config/arm{32,64}.mk and changed them to be > user configurable later in the series. > > As far as KEXEC goes, its a user configurable option now in Rules.mk. > You can build "make kexec=n" and it will disable it. I chose that one as > an original example in v1 of how a user configurable option would look > in this scheme. I don't have a problem with converting existing user-configurable options into Kconfig in the first pass, that seems natural/justifiable enough to me, and doing it in the final patch as you have done seems sensible. Treading very carefully around the creeping-featuritis trap (:-)), it does seem that if one of the user options from xen/Rules.mk is going to be converted then they all ought to be. Perhaps that's a topic for a followup series though. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
On Tue, 2015-12-01 at 04:00 -0700, Jan Beulich wrote: > But not forking doesn't mean importing the whole directory. Some > Makefile adjustments will be necessary anyway, so removing some > of the frontends wouldn't make things worse. We indeed should > avoid editing files we import, but I don't see any bad in skipping > some of the files. It is much easier to resync based on entire directories (my preference, if possible) than to fiddle around picking up individual files, deciding if a "new" file is actually new or just excluded last time for some reason etc. > Reasons I'd like to avoid importing everything: > - no introduction of new build dependencies (one of the frontends > being written in C++ is the mildest form, requiring HOSTCXX to be > set), Only if someone wants to use it. I see no harm in having such a frontend optionally available to those who are willing to meet its prerequisites in their build environment, that certainly doesn't mean it has to work for everyone nor that we should add a C++ compiler and QT environment to the standard set of Xen build deps. I believe this is the policy in the Linux tree too. > - limiting the amount of new code that needs maintaining (no > matter how simple a re-import, it still is work, and hence the less > frequently we need to do this, the better; I assume you agree > that the likelihood for changes that we want/need to pull in > grows with the size of the code, and with what I propose the > import size would shrink by almost 50%), unless you or Doug > volunteer to look after this code on a regular basis, I disagree with the (implied) conclusion here that you would somehow be personally on the hook for these regular updates if we would import only 50% of this kconfig code base, nor that there would therefore be some sort of additional personal burden on you if we take the whole thing. We should be arranging that the maintenance burden is ~0 by rejecting diversion and making the reimport process as brain-dead as possible. Nonetheless if you don't want this to formally come under the remit of "THE REST" then I'd happy to see an entry for xen/tools/kconfig in the MAINTAINERS listing whoever wants to step up (Doug, Ian and/or myself). But I honestly don't think this code is going to require much maintenance at all on our end, apart from the very occasional reimporting of the whole thing from Linux when we notice some major missing feature we care about, which is the case that Ian and I are arguing we should optimise for. And having put aside suggestions such as renaming Kconfig to Xconfig throughout I also don't foresee making very many large changes to this code base at all, there's simply no reason to do so, at least none which can't be pushed back on. At worst I would expect to see generic Kconfig feature requests which should redirected upstream and the results reimported. I think this is all backed up by the fact that after this initial import the remainder of this series consists of: $ git fetch https://github.com/cardoe/xen kconfig_v6 From https://github.com/cardoe/xen * branchkconfig_v6 -> FETCH_HEAD $ git diff --stat a28f2c4~1 FETCH_HEAD -- xen/scripts/kconfig xen/scripts/kconfig/.gitignore | 3 +++ xen/scripts/kconfig/Makefile | 77 + 2 files changed, 80 insertions(+) (The first of which seems like it ought to be fixed upstream instead and it might even be possible to avoid the latter and therefore avoid the consequential renaming of the upstream Makefile => Makefile.linux by using xen/scripts/Makefile.kconfig somehow). > - avoiding code (scripts) that seem completely pointless in our > environment (e.g. streamline_config.pl). I think the overhead of a few extra files of marginal usefulness is far outweighed by being able to just resync a whole directory. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
On 12/2/15 3:47 AM, Ian Campbell wrote: > On Tue, 2015-12-01 at 14:04 -0600, Doug Goldstein wrote: >> On 11/30/15 11:19 AM, Ian Campbell wrote: >>> On Mon, 2015-11-30 at 11:00 -0600, Doug Goldstein wrote: Since there is a request to have KEXEC and the UARTs configurable by the user >>> >>> Who asked for this? >>> >>> I have quite a strong preference for not adding _any_ new[*] user >>> configurable options in this first pass, since I think those need to be >>> considered quite carefully whereas this first series should be largely >>> about the mechanics of introducing Kconfig files. >>> >>> Ian. >>> >>> [*] i.e. anything which is not already controllable by the current >>> .config >>> driven thing. >>> >> >> The ARM UARTs are the take away I had from conversations with Julien >> Grall > > AIUI all Julien was asking for was that the same set of UARTs should be > enabled for arm32 or arm64 both before and after this series, not that they > should be user configurable (in this first pass at least). > > TBH I think this series is getting bogged down in trying to do much all at > the same time, switching to Kconfig, making things newly user-selectable, > arranging for out of tree builds, some of which are either (potentially) > controversial or simply result in apparently unnecessary changes if the > reviewer is only expecting a subset of those changes (especially those only > expecting the first), leading to a few RTTs of back and forth with > reviewers. > > I would encourage you to par this first series back to the simplest > possible "switch to Kconfig, retaining the exact same set of user facing > options as today" and avoid feature creep from people requesting new and > exciting things which the switch to Kconfig makes possible. That I can do. I always expected that I would have a follow on series (maybe even multiples) as I was able to tease out some of the dependencies better. e.g. converting some of include/asm-ARCH/config.h I would also happily add myself to MAINTAINERS for the Kconfig parts and would ensure some kind of periodic sync up with Linux upstream. I'm not looking at doing this conversion as a one and done but sticking around for the long haul. > > That way we can make progress on a mostly mechanical switch to Kconfig > without continuously getting side tracked on questions like whether this or > that should be user selectable or not. > > All of the "feature-creep" (which I don't mean in the pejorative sense) > things can easily be done in a follow up series. > >> and reading past comments on the ML how people can change the ARM >> UARTs. Obviously if that's not desired I can drop that. I originally had >> them enabled as they are in config/arm{32,64}.mk and changed them to be >> user configurable later in the series. >> >> As far as KEXEC goes, its a user configurable option now in Rules.mk. >> You can build "make kexec=n" and it will disable it. I chose that one as >> an original example in v1 of how a user configurable option would look >> in this scheme. > > I don't have a problem with converting existing user-configurable options > into Kconfig in the first pass, that seems natural/justifiable enough to > me, and doing it in the final patch as you have done seems sensible. > > Treading very carefully around the creeping-featuritis trap (:-)), it does > seem that if one of the user options from xen/Rules.mk is going to be > converted then they all ought to be. Perhaps that's a topic for a followup > series though. > > Ian. > Sure thing. I'll break that out. My plan was to convert them all. The patch that does KEXEC was actually from the original RFC/v1 series where it was just an example one and I never dropped it in later series because it was useful for testing. I've got a follow on that converts all the items in xen/Rules.mk that I have not posted to the list and I can move the KEXEC into that series. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
On 12/2/15 3:38 AM, Jan Beulich wrote: On 01.12.15 at 21:04, wrote: >> On 11/30/15 11:19 AM, Ian Campbell wrote: >>> On Mon, 2015-11-30 at 11:00 -0600, Doug Goldstein wrote: Since there is a request to have KEXEC and the UARTs configurable by the user >>> >>> Who asked for this? >>> >>> I have quite a strong preference for not adding _any_ new[*] user >>> configurable options in this first pass, since I think those need to be >>> considered quite carefully whereas this first series should be largely >>> about the mechanics of introducing Kconfig files. >>> >>> Ian. >>> >>> [*] i.e. anything which is not already controllable by the current .config >>> driven thing. >>> >> >> The ARM UARTs are the take away I had from conversations with Julien >> Grall and reading past comments on the ML how people can change the ARM >> UARTs. Obviously if that's not desired I can drop that. I originally had >> them enabled as they are in config/arm{32,64}.mk and changed them to be >> user configurable later in the series. >> >> As far as KEXEC goes, its a user configurable option now in Rules.mk. >> You can build "make kexec=n" and it will disable it. I chose that one as >> an original example in v1 of how a user configurable option would look >> in this scheme. > > And note how Ian said "_any_ new". > > In fact as a follow-up we should convert other command line driven > overrides to config options (debug, perfc, bigmem, shadow) imo. > > Jan > I've got that series queued up locally with plans to rebase it and post it once the initial series lands. I'll pull the KEXEC one into that series and drop it from the original. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2
On 12/2/15 4:39 AM, Ian Campbell wrote: > On Tue, 2015-12-01 at 04:00 -0700, Jan Beulich wrote: >> But not forking doesn't mean importing the whole directory. Some >> Makefile adjustments will be necessary anyway, so removing some >> of the frontends wouldn't make things worse. We indeed should >> avoid editing files we import, but I don't see any bad in skipping >> some of the files. > > It is much easier to resync based on entire directories (my preference, if > possible) than to fiddle around picking up individual files, deciding if a > "new" file is actually new or just excluded last time for some reason etc. > >> Reasons I'd like to avoid importing everything: >> - no introduction of new build dependencies (one of the frontends >> being written in C++ is the mildest form, requiring HOSTCXX to be >> set), > > Only if someone wants to use it. I see no harm in having such a frontend > optionally available to those who are willing to meet its prerequisites in > their build environment, that certainly doesn't mean it has to work for > everyone nor that we should add a C++ compiler and QT environment to the > standard set of Xen build deps. > > I believe this is the policy in the Linux tree too. > >> - limiting the amount of new code that needs maintaining (no >> matter how simple a re-import, it still is work, and hence the less >> frequently we need to do this, the better; I assume you agree >> that the likelihood for changes that we want/need to pull in >> grows with the size of the code, and with what I propose the >> import size would shrink by almost 50%), unless you or Doug >> volunteer to look after this code on a regular basis, > > I disagree with the (implied) conclusion here that you would somehow be > personally on the hook for these regular updates if we would import only > 50% of this kconfig code base, nor that there would therefore be some sort > of additional personal burden on you if we take the whole thing. We should > be arranging that the maintenance burden is ~0 by rejecting diversion and > making the reimport process as brain-dead as possible. > > Nonetheless if you don't want this to formally come under the remit of "THE > REST" then I'd happy to see an entry for xen/tools/kconfig in the > MAINTAINERS listing whoever wants to step up (Doug, Ian and/or myself). > > But I honestly don't think this code is going to require much maintenance > at all on our end, apart from the very occasional reimporting of the whole > thing from Linux when we notice some major missing feature we care about, > which is the case that Ian and I are arguing we should optimise for. > > And having put aside suggestions such as renaming Kconfig to Xconfig > throughout I also don't foresee making very many large changes to this code > base at all, there's simply no reason to do so, at least none which can't > be pushed back on. At worst I would expect to see generic Kconfig feature > requests which should redirected upstream and the results reimported. > > I think this is all backed up by the fact that after this initial import > the remainder of this series consists of: > > $ git fetch https://github.com/cardoe/xen kconfig_v6 > From https://github.com/cardoe/xen > * branchkconfig_v6 -> FETCH_HEAD > $ git diff --stat a28f2c4~1 FETCH_HEAD -- xen/scripts/kconfig > xen/scripts/kconfig/.gitignore | 3 +++ > xen/scripts/kconfig/Makefile | 77 > + > 2 files changed, 80 insertions(+) > > (The first of which seems like it ought to be fixed upstream instead and it > might even be possible to avoid the latter and therefore avoid the > consequential renaming of the upstream Makefile => Makefile.linux by using > xen/scripts/Makefile.kconfig somehow). I've done the Makefile -> Makefile.kconfig you suggested. As far as the .gitignore goes the Linux kernel ignores ALL .* files and explicitly un-ignores dot-files they want to keep. So not sure if they would accept a change upstream. > >> - avoiding code (scripts) that seem completely pointless in our >> environment (e.g. streamline_config.pl). > > I think the overhead of a few extra files of marginal usefulness is far > outweighed by being able to just resync a whole directory. > > Ian. > -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel