On 5/24/25 13:17, Danilo Krummrich wrote:
> On Fri, May 23, 2025 at 04:11:39PM +0200, Danilo Krummrich wrote:
>> On Fri, May 23, 2025 at 02:56:40PM +0200, Christian König wrote:
>>> + if (xas_nomem(&xas, GFP_KERNEL)) {
>>> + xa_lock(&job->dependencies);
>>> + goto retry;
>>
>> Please don't use a goto here, if we would have failed to allocate memory
>> here,
>> this would be an endless loop until we succeed eventually.
>
> I think I got confused by xas_nomem() returning true meaning that memory was
> needed and was successfully allocated (which can be a bit counterintuitive).
Yeah, I had to take a look at the samples/tests to understand that as well.
>
> So, your code here should be correct. However, I'd still remove the goto and
> just call xas_store() again. There's no reason to make this a loop and
> backwards
> goto is better avoided anyways. :)
I was considering that as well, but than abandoned this idea. The xarray()
sample code and test cases as well as the use cases where I took a look either
use a loop or a goto.
I'm not 100% sure why, my suspicion is that you need the loop when there can be
concurrent add/remove operations on the xarray, but I think we should stick
with the common approach.
Regards,
Christian.
