On 24.01.2024 10:34, Oleksii wrote: > On Tue, 2024-01-23 at 14:37 +0100, Jan Beulich wrote: >> On 23.01.2024 13:34, Oleksii wrote: >>> On Tue, 2024-01-23 at 12:14 +0100, Jan Beulich wrote: >>>> On 22.12.2023 16:13, Oleksii Kurochko wrote: >>>>> --- a/xen/common/Kconfig >>>>> +++ b/xen/common/Kconfig >>>>> @@ -47,6 +47,9 @@ config ARCH_MAP_DOMAIN_PAGE >>>>> config GENERIC_BUG_FRAME >>>>> bool >>>>> >>>>> +config GENERIC_FIND_NEXT_BIT >>>>> + bool >>>> >>>> There's no need for this, as ... >>>> >>>>> --- a/xen/lib/Makefile >>>>> +++ b/xen/lib/Makefile >>>>> @@ -3,6 +3,7 @@ obj-$(CONFIG_X86) += x86/ >>>>> lib-y += bsearch.o >>>>> lib-y += ctors.o >>>>> lib-y += ctype.o >>>>> +lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find-next-bit.o >>>> >>>> ... you're moving this to lib/. Or have you encountered any issue >>>> with building this uniformly, and you forgot to mention this in >>>> the description? >>> I didn't check. My intention was to provide opportunity to check if >>> an >>> architecture want to use generic version or not. Otherwise, I >>> expected >>> that we will have multiple definiotion of the funcion. >>> >>> But considering that they are all defined under #ifdef...#endif we >>> can >>> remove the declaration of the config GENERIC_FIND_NEXT_BIT. >> >> What #ifdef / #endif would matter here? Whats in lib/ is intended to >> be >> generic anyway. And what is in the resulting lib.a won't be used by >> an >> arch if it has an arch-specific implementation. > If what is implemented in lib.a won't be used by an arch if it has an > arch-specific implementation then, for sure, I have to drop > CONFIG_GENERIC_FIND_NEXT_BIT. > But I am not really understand if lib.a is linked with Xen, then it > should be an issue then if some arch implement find-next-bit function > we will have to multiple definitions ( one in lib.a and one arch > specific ). Probably, I have to look at how it is done.
You're aware how linking works? Objects are pulled out of archives only if there's no other definition for a to-be-resolved symbol provided by a particular object in the archive. >> Problems could arise if >> an arch had an inline function colliding with the out-of-line one. >> But >> that's about the old case where I could see a need to make the >> building >> of one of the objects conditional. And you'll note that withing this >> Makefile there are pretty few conditionals. > Could you please clarify What does it mean "out-of-line" ? "not inline" >>>>> --- /dev/null >>>>> +++ b/xen/lib/find-next-bit.c >>>>> [...] >>>> >>>> I was going to ask that you convince git to actually present a >>>> proper >>>> diff, to make visible what changes. But other than the >>>> description >>>> says >>>> you don't really move the file, you copy it. Judging from further >>>> titles >>>> there's also nowhere you'd make Arm actually use this now generic >>>> code. >>> I wanted to do it separately, outside this patch series to simplify >>> review and not have Arm specific changes in RISC-V patch series. >> >> Then do it the other way around: Make a separate _prereq_ change >> truly >> moving the file. > So this one patch should be separated by 2? One which moves find-next- > bit.c from Arm to xen/lib, and second where xen/lib/Makefile is > updated. No, that would break the Arm build. I suggested breaking out this patch from the series, and then doing what the description says: Actually move the file. I don't think I suggested splitting the patch. Even the breaking out of the series was only because you said "I wanted to do it separately, outside this patch series". Jan
