Re: [PATCH 4/4] io_uring: reuse buffer updates for registration

[Jens Axboe <axboe@kernel.dk>]

On 4/4/25 3:38 PM, Pavel Begunkov wrote:
> On 4/4/25 17:38, Jens Axboe wrote:
>> On 4/4/25 10:22 AM, Pavel Begunkov wrote:
>>> @@ -316,17 +318,26 @@ static int __io_sqe_buffers_update(struct io_ring_ctx *ctx,
>>>               err = PTR_ERR(node);
>>>               break;
>>>           }
>>> -        i = array_index_nospec(up->offset + done, ctx->buf_table.nr);
>>> -        io_reset_rsrc_node(ctx, &ctx->buf_table, i);
>>> -        ctx->buf_table.nodes[i] = node;
>>> +        i = array_index_nospec(up->offset + done, buf_table->nr);
>>> +        io_reset_rsrc_node(ctx, buf_table, i);
>>> +        buf_table->nodes[i] = node;
>>>           if (ctx->compat)
>>>               user_data += sizeof(struct compat_iovec);
>>>           else
>>>               user_data += sizeof(struct iovec);
>>>       }
>>> +    if (last_error)
>>> +        *last_error = err;
>>>       return done ? done : err;
>>>   }
>>>   +static int __io_sqe_buffers_update(struct io_ring_ctx *ctx,
>>> +                   struct io_uring_rsrc_update2 *up,
>>> +                   unsigned int nr_args)
>>> +{
>>> +    return io_buffer_table_update(ctx, &ctx->buf_table, up, nr_args, NULL);
>>> +}
>>
>> Minor style preference, but just do:
>>
>>     unsigned last_err;
>>     return io_buffer_table_update(ctx, &ctx->buf_table, up, nr_args, &last_err);
>>
>> and skip that last_error could be conditionally set?
> 
> I can't say I see how that's better though

I think so though, it makes the store unconditional and there's no risk
of anything forgetting that last_error is sometimes valid, sometimes
not. The last part is the most annoying to me, it's just more fragile
imho.

-- 
Jens Axboe