From: Julien Grall <julien@xen.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
George Dunlap <george.dunlap@citrix.com>,
Ian Jackson <iwj@xenproject.org>,
Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 00/23] further population of xen/lib/
Date: Thu, 1 Apr 2021 15:04:02 +0100 [thread overview]
Message-ID: <6a38f0db-938b-fd13-48e6-6b538c85fe42@xen.org> (raw)
In-Reply-To: <4f745d03-baa8-e9e6-692c-f9c9f401b766@suse.com>
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
next prev parent reply other threads:[~2021-04-01 14:04 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-01 10:14 [PATCH 00/23] further population of xen/lib/ Jan Beulich
2021-04-01 10:19 ` [PATCH 01/23] lib: move muldiv64() Jan Beulich
2021-04-15 10:00 ` Ping (x86): " Jan Beulich
2021-04-15 12:46 ` Roger Pau Monné
2021-04-01 10:19 ` [PATCH 02/23] lib: move 64-bit div/mod compiler helpers Jan Beulich
2021-04-01 14:56 ` Julien Grall
2021-04-01 15:23 ` Jan Beulich
2021-04-06 19:34 ` Julien Grall
2021-04-07 8:33 ` Jan Beulich
2021-04-07 14:06 ` Julien Grall
2021-04-01 10:20 ` [PATCH 03/23] string: drop redundant declarations Jan Beulich
2021-04-01 14:59 ` Julien Grall
2021-04-01 10:20 ` [PATCH 04/23] lib: move memset() Jan Beulich
2021-04-01 10:21 ` [PATCH 05/23] lib: move memcpy() Jan Beulich
2021-04-01 10:21 ` [PATCH 06/23] lib: move memmove() Jan Beulich
2021-04-01 10:22 ` [PATCH 07/23] lib: move memcmp() Jan Beulich
2021-04-01 10:22 ` [PATCH 08/23] lib: move memchr() Jan Beulich
2021-04-01 10:23 ` [PATCH 09/23] lib: move memchr_inv() Jan Beulich
2021-04-01 10:23 ` [PATCH 10/23] lib: move strlen() Jan Beulich
2021-04-01 10:23 ` [PATCH 11/23] lib: move strnlen() Jan Beulich
2021-04-01 10:24 ` [PATCH 12/23] lib: move strcmp() Jan Beulich
2021-04-01 10:25 ` [PATCH 13/23] lib: move strncmp() Jan Beulich
2021-04-01 10:25 ` [PATCH 14/23] lib: move strlcpy() Jan Beulich
2021-04-01 10:25 ` [PATCH 15/23] lib: move strlcat() Jan Beulich
2021-04-01 10:25 ` [PATCH 16/23] lib: move strchr() Jan Beulich
2021-04-01 10:26 ` [PATCH 17/23] lib: move strrchr() Jan Beulich
2021-04-01 10:26 ` [PATCH 18/23] lib: move strstr() Jan Beulich
2021-04-01 10:26 ` [PATCH 19/23] lib: move strcasecmp() Jan Beulich
2021-04-01 10:27 ` [PATCH 20/23] lib: move/rename strnicmp() to strncasecmp() Jan Beulich
2021-04-01 10:27 ` [PATCH 21/23] lib: move strspn() Jan Beulich
2021-04-01 10:28 ` [PATCH 22/23] lib: move strpbrk() Jan Beulich
2021-04-01 10:28 ` [PATCH 23/23] lib: move strsep() Jan Beulich
2021-04-01 11:54 ` [PATCH 00/23] further population of xen/lib/ Julien Grall
2021-04-01 13:43 ` Jan Beulich
2021-04-01 14:04 ` Julien Grall [this message]
2021-04-01 14:27 ` Jan Beulich
2021-04-01 14:55 ` Julien Grall
2021-04-01 15:25 ` Jan Beulich
2021-04-01 15:32 ` Julien Grall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6a38f0db-938b-fd13-48e6-6b538c85fe42@xen.org \
--to=julien@xen.org \
--cc=andrew.cooper3@citrix.com \
--cc=george.dunlap@citrix.com \
--cc=iwj@xenproject.org \
--cc=jbeulich@suse.com \
--cc=sstabellini@kernel.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).