Re: [Xen-devel] [PATCH 3/3] xen/include: Include xen/kconfig.h automatically

2017-02-16 Thread Jan Beulich
>>> On 16.02.17 at 12:59,  wrote:
> On 16/02/17 11:10, Jan Beulich wrote:
> On 16.02.17 at 12:01,  wrote:
>>> On 16/02/17 10:48, Jan Beulich wrote:
>>> On 16.02.17 at 11:40,  wrote:
> On 16/02/17 10:27, Jan Beulich wrote:
> On 15.02.17 at 19:10,  wrote:
>>> generated/autoconf.h is already included automatically so CONFIG_* 
>>> defines 
>>> are
>>> avaialble.  However, the companion macros such as IS_ENABLED() are not
>>> included.
>>>
>>> Include them uniformally everywhere.
>> Well, if you really think this is a good idea, I'm not going to stand in
>> the way, but why do we need this included everywhere? Many files
>> don't even care about any CONFIG_*, and hence even less so about
>> kconfig.h.
> I am sorry, but you are complaining if I include it unilaterally, and
> also complaining if I include kconfig.h in the specific location where I
> need it.  These are mutually exclusive.
 I don't understand - when did I complain about its inclusion where
 it's needed? Iirc my complaint was about you adding the inclusion
 to */config.h without that header actually using the macros. My
 point really is that ideally each C file would get as little cruft as
 possible, while at present quite a number of header are being
 included by virtually every source file.
>>> Your complaint was specifically about me adding it to paging.h so I
>>> could use IS_ENABLED() and not out-of-line a trivial function.
>> Oh, that one: There my view was the other way around: No need
>> to include yet another header in one which already gets included
>> everywhere, when the new function could easily be out of line (as
>> not being performance critical).
>>
>>> As for general availably, while I agree in general that we have far too
>>> much stuff included by default (I have some plans for that), the
>>> contents of kconfig.h is fairly small, and exactly the same category of
>>> information as config.h
>>>
>>> I am looking to push for the use of IS_ENABLED() in preference to #ifdef
>>> where possible, to reduce code-rot.
>> Which makes sense, but won't affect said source files not using any
>> CONFIG_* in the first place.
> 
> We already include CONFIG_* everywhere.  All this change does is
> consistently add IS_ENABLED() alongside, so it can be used when CONFIG_*
> are available.

The relevant aspect isn't CONFIG_* being available, but any of
them being actually used.

> If we have occasion in the future to reconsider having the CONFIG_*
> variables unilaterally included, then fine, but the current state of the
> code is the worst of all options.

I don't think so, but as said, I'm not meaning to stand in the way of
this patch going in (as making the current situation only marginally
worse).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 3/3] xen/include: Include xen/kconfig.h automatically

2017-02-16 Thread Julien Grall

Hi Jan,

On 16/02/17 11:10, Jan Beulich wrote:

On 16.02.17 at 12:01,  wrote:

On 16/02/17 10:48, Jan Beulich wrote:

On 16.02.17 at 11:40,  wrote:

On 16/02/17 10:27, Jan Beulich wrote:

On 15.02.17 at 19:10,  wrote:

generated/autoconf.h is already included automatically so CONFIG_* defines

are

avaialble.  However, the companion macros such as IS_ENABLED() are not
included.

Include them uniformally everywhere.

Well, if you really think this is a good idea, I'm not going to stand in
the way, but why do we need this included everywhere? Many files
don't even care about any CONFIG_*, and hence even less so about
kconfig.h.

I am sorry, but you are complaining if I include it unilaterally, and
also complaining if I include kconfig.h in the specific location where I
need it.  These are mutually exclusive.

I don't understand - when did I complain about its inclusion where
it's needed? Iirc my complaint was about you adding the inclusion
to */config.h without that header actually using the macros. My
point really is that ideally each C file would get as little cruft as
possible, while at present quite a number of header are being
included by virtually every source file.


Your complaint was specifically about me adding it to paging.h so I
could use IS_ENABLED() and not out-of-line a trivial function.


Oh, that one: There my view was the other way around: No need
to include yet another header in one which already gets included
everywhere, when the new function could easily be out of line (as
not being performance critical).


As for general availably, while I agree in general that we have far too
much stuff included by default (I have some plans for that), the
contents of kconfig.h is fairly small, and exactly the same category of
information as config.h

I am looking to push for the use of IS_ENABLED() in preference to #ifdef
where possible, to reduce code-rot.


Which makes sense, but won't affect said source files not using any
CONFIG_* in the first place.


At least on ARM, we need CONFIG_* everywhere as the definitions of types 
and structure will change whether you are compiling for ARM64 or ARM.


I would expect the same for common code.

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 3/3] xen/include: Include xen/kconfig.h automatically

2017-02-16 Thread Andrew Cooper
On 16/02/17 11:10, Jan Beulich wrote:
 On 16.02.17 at 12:01,  wrote:
>> On 16/02/17 10:48, Jan Beulich wrote:
>> On 16.02.17 at 11:40,  wrote:
 On 16/02/17 10:27, Jan Beulich wrote:
 On 15.02.17 at 19:10,  wrote:
>> generated/autoconf.h is already included automatically so CONFIG_* 
>> defines 
>> are
>> avaialble.  However, the companion macros such as IS_ENABLED() are not
>> included.
>>
>> Include them uniformally everywhere.
> Well, if you really think this is a good idea, I'm not going to stand in
> the way, but why do we need this included everywhere? Many files
> don't even care about any CONFIG_*, and hence even less so about
> kconfig.h.
 I am sorry, but you are complaining if I include it unilaterally, and
 also complaining if I include kconfig.h in the specific location where I
 need it.  These are mutually exclusive.
>>> I don't understand - when did I complain about its inclusion where
>>> it's needed? Iirc my complaint was about you adding the inclusion
>>> to */config.h without that header actually using the macros. My
>>> point really is that ideally each C file would get as little cruft as
>>> possible, while at present quite a number of header are being
>>> included by virtually every source file.
>> Your complaint was specifically about me adding it to paging.h so I
>> could use IS_ENABLED() and not out-of-line a trivial function.
> Oh, that one: There my view was the other way around: No need
> to include yet another header in one which already gets included
> everywhere, when the new function could easily be out of line (as
> not being performance critical).
>
>> As for general availably, while I agree in general that we have far too
>> much stuff included by default (I have some plans for that), the
>> contents of kconfig.h is fairly small, and exactly the same category of
>> information as config.h
>>
>> I am looking to push for the use of IS_ENABLED() in preference to #ifdef
>> where possible, to reduce code-rot.
> Which makes sense, but won't affect said source files not using any
> CONFIG_* in the first place.

We already include CONFIG_* everywhere.  All this change does is
consistently add IS_ENABLED() alongside, so it can be used when CONFIG_*
are available.

If we have occasion in the future to reconsider having the CONFIG_*
variables unilaterally included, then fine, but the current state of the
code is the worst of all options.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 3/3] xen/include: Include xen/kconfig.h automatically

2017-02-16 Thread Jan Beulich
>>> On 16.02.17 at 12:01,  wrote:
> On 16/02/17 10:48, Jan Beulich wrote:
> On 16.02.17 at 11:40,  wrote:
>>> On 16/02/17 10:27, Jan Beulich wrote:
>>> On 15.02.17 at 19:10,  wrote:
> generated/autoconf.h is already included automatically so CONFIG_* 
> defines 
> are
> avaialble.  However, the companion macros such as IS_ENABLED() are not
> included.
>
> Include them uniformally everywhere.
 Well, if you really think this is a good idea, I'm not going to stand in
 the way, but why do we need this included everywhere? Many files
 don't even care about any CONFIG_*, and hence even less so about
 kconfig.h.
>>> I am sorry, but you are complaining if I include it unilaterally, and
>>> also complaining if I include kconfig.h in the specific location where I
>>> need it.  These are mutually exclusive.
>> I don't understand - when did I complain about its inclusion where
>> it's needed? Iirc my complaint was about you adding the inclusion
>> to */config.h without that header actually using the macros. My
>> point really is that ideally each C file would get as little cruft as
>> possible, while at present quite a number of header are being
>> included by virtually every source file.
> 
> Your complaint was specifically about me adding it to paging.h so I
> could use IS_ENABLED() and not out-of-line a trivial function.

Oh, that one: There my view was the other way around: No need
to include yet another header in one which already gets included
everywhere, when the new function could easily be out of line (as
not being performance critical).

> As for general availably, while I agree in general that we have far too
> much stuff included by default (I have some plans for that), the
> contents of kconfig.h is fairly small, and exactly the same category of
> information as config.h
> 
> I am looking to push for the use of IS_ENABLED() in preference to #ifdef
> where possible, to reduce code-rot.

Which makes sense, but won't affect said source files not using any
CONFIG_* in the first place.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 3/3] xen/include: Include xen/kconfig.h automatically

2017-02-16 Thread Andrew Cooper
On 16/02/17 10:48, Jan Beulich wrote:
 On 16.02.17 at 11:40,  wrote:
>> On 16/02/17 10:27, Jan Beulich wrote:
>> On 15.02.17 at 19:10,  wrote:
 generated/autoconf.h is already included automatically so CONFIG_* defines 
 are
 avaialble.  However, the companion macros such as IS_ENABLED() are not
 included.

 Include them uniformally everywhere.
>>> Well, if you really think this is a good idea, I'm not going to stand in
>>> the way, but why do we need this included everywhere? Many files
>>> don't even care about any CONFIG_*, and hence even less so about
>>> kconfig.h.
>> I am sorry, but you are complaining if I include it unilaterally, and
>> also complaining if I include kconfig.h in the specific location where I
>> need it.  These are mutually exclusive.
> I don't understand - when did I complain about its inclusion where
> it's needed? Iirc my complaint was about you adding the inclusion
> to */config.h without that header actually using the macros. My
> point really is that ideally each C file would get as little cruft as
> possible, while at present quite a number of header are being
> included by virtually every source file.

Your complaint was specifically about me adding it to paging.h so I
could use IS_ENABLED() and not out-of-line a trivial function.

As for general availably, while I agree in general that we have far too
much stuff included by default (I have some plans for that), the
contents of kconfig.h is fairly small, and exactly the same category of
information as config.h

I am looking to push for the use of IS_ENABLED() in preference to #ifdef
where possible, to reduce code-rot.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 3/3] xen/include: Include xen/kconfig.h automatically

2017-02-16 Thread Jan Beulich
>>> On 16.02.17 at 11:40,  wrote:
> On 16/02/17 10:27, Jan Beulich wrote:
> On 15.02.17 at 19:10,  wrote:
>>> generated/autoconf.h is already included automatically so CONFIG_* defines 
>>> are
>>> avaialble.  However, the companion macros such as IS_ENABLED() are not
>>> included.
>>>
>>> Include them uniformally everywhere.
>> Well, if you really think this is a good idea, I'm not going to stand in
>> the way, but why do we need this included everywhere? Many files
>> don't even care about any CONFIG_*, and hence even less so about
>> kconfig.h.
> 
> I am sorry, but you are complaining if I include it unilaterally, and
> also complaining if I include kconfig.h in the specific location where I
> need it.  These are mutually exclusive.

I don't understand - when did I complain about its inclusion where
it's needed? Iirc my complaint was about you adding the inclusion
to */config.h without that header actually using the macros. My
point really is that ideally each C file would get as little cruft as
possible, while at present quite a number of header are being
included by virtually every source file.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 3/3] xen/include: Include xen/kconfig.h automatically

2017-02-16 Thread Andrew Cooper
On 16/02/17 10:27, Jan Beulich wrote:
 On 15.02.17 at 19:10,  wrote:
>> generated/autoconf.h is already included automatically so CONFIG_* defines 
>> are
>> avaialble.  However, the companion macros such as IS_ENABLED() are not
>> included.
>>
>> Include them uniformally everywhere.
> Well, if you really think this is a good idea, I'm not going to stand in
> the way, but why do we need this included everywhere? Many files
> don't even care about any CONFIG_*, and hence even less so about
> kconfig.h.

I am sorry, but you are complaining if I include it unilaterally, and
also complaining if I include kconfig.h in the specific location where I
need it.  These are mutually exclusive.

I do prefer this approach, overall.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 3/3] xen/include: Include xen/kconfig.h automatically

2017-02-16 Thread Jan Beulich
>>> On 15.02.17 at 19:10,  wrote:
> generated/autoconf.h is already included automatically so CONFIG_* defines are
> avaialble.  However, the companion macros such as IS_ENABLED() are not
> included.
> 
> Include them uniformally everywhere.

Well, if you really think this is a good idea, I'm not going to stand in
the way, but why do we need this included everywhere? Many files
don't even care about any CONFIG_*, and hence even less so about
kconfig.h.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 3/3] xen/include: Include xen/kconfig.h automatically

2017-02-16 Thread Julien Grall

Hi Andrew,

On 15/02/2017 18:10, Andrew Cooper wrote:

generated/autoconf.h is already included automatically so CONFIG_* defines are
avaialble.  However, the companion macros such as IS_ENABLED() are not


NIT: s/avaialble/available/


included.

Include them uniformally everywhere.

Signed-off-by: Andrew Cooper 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel