[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  • Date: Wed, 8 Sep 2021 12:14:57 +0100
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, "Tim Deegan" <tim@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 08 Sep 2021 11:15:08 +0000
  • Ironport-hdrordr: A9a23:mZRoAak58A2M27MqqKZ5KJyQSMDpDfLW3DAbv31ZSRFFG/Fw9/ rCoB3U73/JYVcqKRUdcLW7UpVoLkmyyXcY2+cs1NSZLWzbUQmTXeJfBOLZqlWNJ8SXzIVgPM xbAspD4bPLbGSTjazBkXSF+9RL+qj6zEh/792usEuETmtRGt9dBx8SMHf9LqXvLjM2fqbQEv Cnl6x6jgvlQ1s7ROKhCEIIWuDSzue77q4PMXY9dmcaABDlt0LR1ILH
  • Ironport-sdr: yu7wkN9+I7fVB+84sgevwm/Fcn1zI3OrEoXq/wC/7jI6xgO1bfOpqdAEb204briH+nN7MPbXEA Ayj3yk1pjMBDs1Qbvk/8Mdq9O8AxNu1Ftgv1oMCgprFWZI8RPkhFnbou0CHCJt2cbk04xnChtW b0Os76oZbM00MnPZFLyFKC/U4IJSJctxEvCkjNqK1hAr4/j9svpOHS2F9gam3sUiwVFYQNF2an YkUdl008MgR7OXXxu22u6yKlq10r6AEEqeE2cfVBIqr1x1hSSWdQUVUjRgcbPtbGkETki7ZX2c jo7GIGMHCJhozx+J8Cck7q09
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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@xxxxxxxxxx>
> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> 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? What kind of issue is there with those
tiny source files? 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." ?

Thanks,

-- 
Anthony PERARD



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.