Re: [PATCH v4 2/6] io_uring/zcrx: return error from io_zcrx_map_area_*

[David Wei <dw@davidwei.uk>]

On 2025-07-02 15:50, Pavel Begunkov wrote:
> On 7/2/25 23:27, David Wei wrote:
>> On 2025-07-02 07:29, Pavel Begunkov wrote:
>>> io_zcrx_map_area_*() helpers return the number of processed niovs, which
>>> we use to unroll some of the mappings for user memory areas. It's
>>> unhandy, and dmabuf doesn't care about it. Return an error code instead
>>> and move failure partial unmapping into io_zcrx_map_area_umem().
>>>
>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>> ---
>>>   io_uring/zcrx.c | 27 ++++++++++++++-------------
>>>   1 file changed, 14 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
>>> index 99a253c1c6c5..2cde88988260 100644
>>> --- a/io_uring/zcrx.c
>>> +++ b/io_uring/zcrx.c
>>
>> ...
>>
>>> @@ -254,29 +254,30 @@ static int io_zcrx_map_area_umem(struct io_zcrx_ifq *ifq, struct io_zcrx_area *a
>>>               break;
>>>           }
>>>       }
>>> -    return i;
>>> +
>>> +    if (i != area->nia.num_niovs) {
>>> +        __io_zcrx_unmap_area(ifq, area, i);
>>> +        return -EINVAL;
>>> +    }
>>> +    return 0;
>>>   }
>>
>> Does io_release_dmabuf() still need to be called in
>> io_zcrx_map_area_dmabuf() in case of failure, if say
>> net_mp_niov_set_dma_addr() fails in the loop?
> 
> It doesn't. The function only sets niov addresses, and it's fine
> to leave garbage on failure. In contrast, the umem version sets
> up dma mappings and needs to unmap if there is an error.
> 

Yeah, I see. And it's replaced with sg_table helpers anyway in patch 4.

Reviewed-by: David Wei <dw@davidwei.uk>