[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/6] accel: accel_available() function
From: |
Claudio Fontana |
Subject: |
Re: [PATCH v2 2/6] accel: accel_available() function |
Date: |
Thu, 26 Nov 2020 15:13:33 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 |
On 11/26/20 2:36 PM, Eduardo Habkost wrote:
> On Thu, Nov 26, 2020 at 10:14:31AM +0100, Claudio Fontana wrote:
>> Hi Eduardo,
>>
>> On 11/25/20 9:56 PM, Eduardo Habkost wrote:
>>> This function will be used to replace the xen_available() and
>>> kvm_available() functions from arch_init.c.
>>>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>> ---
>>> Cc: Richard Henderson <richard.henderson@linaro.org>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Claudio Fontana <cfontana@suse.de>
>>> Cc: Roman Bolshakov <r.bolshakov@yadro.com>
>>> ---
>>> include/sysemu/accel.h | 1 +
>>> accel/accel.c | 5 +++++
>>> 2 files changed, 6 insertions(+)
>>>
>>> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
>>> index e08b8ab8fa..a4a00c75c8 100644
>>> --- a/include/sysemu/accel.h
>>> +++ b/include/sysemu/accel.h
>>> @@ -67,6 +67,7 @@ typedef struct AccelClass {
>>> OBJECT_GET_CLASS(AccelClass, (obj), TYPE_ACCEL)
>>>
>>> AccelClass *accel_find(const char *opt_name);
>>> +bool accel_available(const char *name);
>>> int accel_init_machine(AccelState *accel, MachineState *ms);
>>>
>>> /* Called just before os_setup_post (ie just before drop OS privs) */
>>> diff --git a/accel/accel.c b/accel/accel.c
>>> index cb555e3b06..4a64a2b38a 100644
>>> --- a/accel/accel.c
>>> +++ b/accel/accel.c
>>> @@ -46,6 +46,11 @@ AccelClass *accel_find(const char *opt_name)
>>> return ac;
>>> }
>>>
>>> +bool accel_available(const char *name)
>>> +{
>>> + return accel_find(name) != NULL;
>>
>>
>> accel_find() in its implementation allocates and then frees memory to
>> generate the string,
>> the user of accel_available() might be unaware and overuse leading to
>> fragmentation/performance issues?
>
> Is that a real issue? We had only 3 users of kvm_available() and
> xen_available() since those functions were added 10 years ago.
>
> Do you have any suggestions on what we should do?
>
One option I see is simply to document the behavior where accel_available() is
declared in accel.h (ie do not use in fast path), as well as in accel_find()
actually,
so that both accel_find() and accel_available() are avoided in fast path and
avoid being called frequently at runtime.
Another option could be to remove the allocation completely, and use for
example accel_find(ACCEL_CLASS_NAME("tcg")),
or another option again would be to remove the allocation and use either a
fixed buffer + snprintf, or alloca -like builtin code to use the stack, ...
Not a big deal, but with a general utility and short name like
accel_available(name) it might be tempting to use this more in the future?
Ciao,
Claudio
- [PATCH v2 0/6] arch_init.c cleanup, Eduardo Habkost, 2020/11/25
- [PATCH v2 2/6] accel: accel_available() function, Eduardo Habkost, 2020/11/25
- Re: [PATCH v2 2/6] accel: accel_available() function, Claudio Fontana, 2020/11/26
- Re: [PATCH v2 2/6] accel: accel_available() function, Eduardo Habkost, 2020/11/26
- Re: [PATCH v2 2/6] accel: accel_available() function,
Claudio Fontana <=
- Re: [PATCH v2 2/6] accel: accel_available() function, Paolo Bonzini, 2020/11/26
- Re: [PATCH v2 2/6] accel: accel_available() function, Claudio Fontana, 2020/11/26
- Re: [PATCH v2 2/6] accel: accel_available() function, Eduardo Habkost, 2020/11/26
- Re: [PATCH v2 2/6] accel: accel_available() function, Claudio Fontana, 2020/11/27
- Re: [PATCH v2 2/6] accel: accel_available() function, Markus Armbruster, 2020/11/27
- Re: [PATCH v2 2/6] accel: accel_available() function, Claudio Fontana, 2020/11/27
- Re: [PATCH v2 2/6] accel: accel_available() function, Markus Armbruster, 2020/11/27
- Re: [PATCH v2 2/6] accel: accel_available() function, Paolo Bonzini, 2020/11/27
Re: [PATCH v2 2/6] accel: accel_available() function, Cornelia Huck, 2020/11/27
[PATCH v2 3/6] kvm: Remove kvm_available() function, Eduardo Habkost, 2020/11/25