Re: [PATCH bpf-next 2/3] libbpf: split bpf object load into prepare/load

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 01/03/2025 08:12, Eduard Zingerman wrote:
> On Fri, 2025-02-28 at 17:52 +0000, Mykyta Yatsenko wrote:
>
> [...]
>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 9ced1ce2334c..dd2f64903c3b 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -4858,7 +4858,7 @@ bool bpf_map__autocreate(const struct bpf_map *map)
>>   
>>   int bpf_map__set_autocreate(struct bpf_map *map, bool autocreate)
>>   {
>> -	if (map->obj->state >= OBJ_LOADED)
>> +	if (map->obj->state >= OBJ_PREPARED)
>>   		return libbpf_err(-EBUSY);
> I looked through logic in patches #1 and #2 and changes look correct.
> Running tests under valgrind does not show issues with this feature.
> The only ask from my side is to consider doing ==/!= comparisons in
> cases like above. E.g. it seems that `map->obj->state != OBJ_OPENED`
> is a bit simpler to understand when reading condition above.
> Or maybe that's just me.
I'm not sure about this one.  >= or < checks for state relative to 
operand more
flexibly,for example `map->obj->state >= OBJ_PREPARED` is read as
"is the object in at least PREPARED state". Perhaps, if we add more states,
these >,< checks will not require any changes, while ==, != may.
I guess this also depends on what we actually want to check here, is it that
state at least PREPARED or the state is not initial OPENED.
Not a strong opinion, though, happy to flip code to ==, !=.
>   
>>   	map->autocreate = autocreate;
> [...]
>