Re: [PATCH 00/23] further population of xen/lib/

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 01/04/2021 14:43, Jan Beulich wrote:
> On 01.04.2021 13:54, Julien Grall wrote:
> > On 01/04/2021 11:14, Jan Beulich wrote:
> > > This is to dissolve / move xen/common/lib.c and xen/common/string.c.
> > > One benefit of moving these functions into an archive is that we can
> > > drop some of the related __HAVE_ARCH_* #define-s: By living in an
> > > archive, the per-arch functions will preempt any loading of the
> > > respective functions (objects) from the archive. (Down the road we
> > > may want to move the per-arch functions into archives as well, at
> > > which point the per-arch archive(s) would need to be specified ahead
> > > of the common one(s) to the linker.)
> > While I think it is a good idea to move code in xen/lib, I am not
> > convinced that having a single function per file is that beneficial.
> >
> > Do you have numbers showing how much Xen will shrink after this series?
> In the default build, from all I was able to tell, there's one function
> that's unused (strspn(), as mentioned in the respective patch description).
> I don't think I've been claiming any space savings here, though, so I
You didn't. I was trying to guess why you wrote this series given that 
your cover letter doesn't provide a lot of benefits (other than dropping 
__HAVE_ARCH_*).
> wonder why you make this a criteria at all.
Because this is the main reason I would be willing to ack this series. 
This outweight the increase number of files with just a single function 
implemented.
> The functions being one per
> CU is such that they can be individually overridden by an arch, without
> pulling in dead code.
I would agree with functions like memcpy/memset() because you can gain a 
lot to outweight the implementation in assembly. I am not convinced this 
would be true for functions such as strlen().
So overall, the number of functions requiring overriding will likely be 
pretty limited and #ifdef would be IMHO tolerable.
Although, I would be OK with creating a file per function that are 
already overrided. For all the others, I think this is just pointless.
Cheers,

--
Julien Grall