Re: [XEN PATCH v7 05/51] x86/mm: avoid building multiple .o from a single .c file

[Jan Beulich <jbeulich@suse.com>]

On 08.09.2021 13:14, Anthony PERARD wrote:
> On Tue, Sep 07, 2021 at 08:14:14AM +0200, Jan Beulich wrote:
>> On 24.08.2021 12:49, Anthony PERARD wrote:
>>> This replace the use of a single .c file use for multiple .o file by
>>> creating multiple .c file including the first one.
>>>
>>> There's quite a few issues with trying to build more than one object
>>> file from a single source file: there's is a duplication of the make
>>> rules to generate those targets; there is an additional ".file" symbol
>>> added in order to differentiate between the object files; and the
>>> tools/symbols have an heuristic to try to pick up the right ".file".
>>>
>>> This patch adds new .c source file which avoid the need to add a
>>> second ".file" symbol and thus avoid the need to deal with those
>>> issues.
>>>
>>> Also remove __OBJECT_FILE__ from $(CC) command line as it isn't used
>>> anywhere anymore. And remove the macro "build-intermediate" since the
>>> generic rules for single targets can be used.
>>>
>>> And rename the objects in mm/hap/ to remove the extra "level".
>>>
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>
>> Hmm, when replying to 00/51 I didn't recall I gave an R-b for this one
>> already. I'd like to restrict it some: It should be taken to stand for
>> the technical correctness of the change. Nevertheless I'm not really
>> happy with the introduction of the various tiny source files. I've
>> meanwhile been wondering: Can't these be generated (in the build tree,
>> as soon as that's possible to be separate) rather than getting put in
>> the repo?
> 
> Do we really need to generated those never to be change tiny source
> file? Do we really need to make the build system a lot more complicated?
> 
> With those tiny source file in a different directory to the main source
> file, we need to add "-I" to CFLAGS. (source tree vs build tree.)
> 
> I don't like preparation phase, I'd rather do just-in-time generation of
> those tiny file (if we really have too...) as to let make organize
> itself on when to do things. That mean, extra targets on how to generate
> the files, and telling make that those would be intermediate target
> shouldn't be deleted to avoid the final object from been regenerated
> over and over again (I'm not sure why the rebuild of tiny source file
> happen when they are intermediate, maybe because FORCE prerequisite on
> %.o).
> 
> Can't we commit this patch as is?

I'll consider another time.

> What kind of issue is there with those tiny source files?

To me they're ugly and their presence is at least mildly confusing.
And apparently I'm not the only one thinking that way, or else such
tiny stubs would have been put there right when introducing these
multiply built sources.

> Should we add a warning in those tiny source files,
> something like "No modification of this file allowed, it's part of the
> build system." ?

Not sure this would make much of a difference. Them getting touched
later on is not a primary concern of mine. In fact once they're
there, I don't see why they couldn't be extended. At least in shadow
code there might be functions which could live there, reducing
#ifdef-ary.

Jan