[PATCH 2/2] aarch64: Accept string values for glibc.cpu.aarch64_gcs tunable

[Adhemerval Zanella Netto]

On 31/03/26 09:48, Yury Khrustalev wrote:
> On Tue, Mar 31, 2026 at 08:58:03AM -0300, Adhemerval Zanella Netto wrote:
>>
>> On 31/03/26 06:02, Yury Khrustalev wrote:
>>> On Mon, Mar 30, 2026 at 03:42:17PM -0300, Adhemerval Zanella Netto wrote:
>>>>
>>>> ...
>>>>
>>>>>> The '__tunable_print_error' function is exposed globally so that invalid
>>>>>> string inputs caught in the newly added 'aarch64_gcs' callback can trigger
>>>>>> standard tunable warnings.
>>>>>
>>>>> I think that this change is mostly cosmetic and I don't think it's worth
>>>>> doing given the changes it needs and the increased complexity of the code
>>>>> that parse this tunable value. GCS tunable should take as little code as
>>>>> possible to parse it by the nature of it.
>>>>
>>>> It is usability improvement, so not really only 'comestic'.  Sure it is not
>>>> strictly required, but it makes the usage and script that eventually need
>>>> to setup the GLIBC_TUNABLE more readable to what GCS semantic it wants to
>>>> use.
>>>
>>> Users would need to use either 1 or 0 which should be straightforward. I
>>> agree that enum values are easier to use but the amount of code that
>>> needs to run to support them seems to high.
>>
>> And "disabled" and "enforced" is way more clear than '0' or '1', specially
>> because the tunable also acceptss other values (which makes it not a boolean
>> switch, as bti).  
>>
>> Again, it is not clear why you are blocking this change, since it is a 
>> usability improvement.
> 
> Like I've mentioned earlier, I believe that the usability benefit that we may
> have from this patch does not outweigh the extent of changes in the generic
> code and the code that parses this particular tunable. I'd prefer to keep it
> simple unless there is a compelling reason to increase complexity.

But the complexity is just to add a list of possible value and string comparison,
which is done on multiple tunable checks.  

> 
>>
>> ...
>>
>>>>>
>>>>> If the value provided by the user is less than 0 or more than 3, it's an
>>>>> error, we should not implicitly convert it to one of the supported
>>>>> values.
>>>>
>>>> Indeed, it is a mistake in my reading of do_tunable_update_val. If the
>>>> value is out of range we should use the default one.
>>>
>>> The code in 'dl-gcs.c' treats any out of range value as an error, but it
>>> seems like 'TUNABLE_GET' returns default value in this case. I'm not
>>> sure when this change happened, it seems to add inconsistency between
>>> string and integer tunables. I think that silent change of behaviour is
>>> not ideal, it should be an error.
>>
>> It was always like that afaik, since the tunable is defind as an integer
>> and it uses the generic code that sets the default value in case of
>> out of range inputs.
>>
>> To proper validate it we will need either to change the generic tunable
>> parsing by either adding callback or an option to error in case of
>> out-of-range; or do it like this patch and parse on arch-specific callback.
> 
> OK, thanks for clarification.
>