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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.04.2021 16:04, Julien Grall wrote:
> 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().

strlen() is actually a pretty good candidate for overriding, and
we may even want to on x86 with newer hardware's "Fast Short REP
CMPSB/SCASB".

> 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.

Well, I don't see a reason to special case individual functions.
Plus any reasonable static library should imo have one (global)
function per object file in the normal case; there may be very
few exceptions. Drawing an ad hoc boundary at what currently has
an override somewhere doesn't look very attractive to me. Plus
to be honest while I would find it unfair to ask to further
split things if I did just a partial conversion (i.e. invest yet
more time), I find it rather odd to be asked to undo some of the
splitting when I've already taken the extra time to make things
consistent.

Jan