On 4/15/25 7:13 PM, Ming Lei wrote:
> On Mon, Apr 14, 2025 at 02:39:33PM -0600, Jens Axboe wrote:
>> On 4/14/25 1:58 PM, Uday Shankar wrote:
>>> +static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_device *ub,
>>> + struct ublk_queue *ubq, struct ublk_io *io,
>>> + const struct ublksrv_io_cmd *ub_cmd,
>>> + unsigned int issue_flags)
>>> +{
>>> + int ret = 0;
>>> +
>>> + if (issue_flags & IO_URING_F_NONBLOCK)
>>> + return -EAGAIN;
>>> +
>>> + mutex_lock(&ub->mutex);
>>
>> This looks like overkill, if we can trylock the mutex that should surely
>> be fine? And I would imagine succeed most of the time, hence making the
>> inline/fastpath fine with F_NONBLOCK?
>
> The mutex is the innermost lock and it won't block for handling FETCH
> command, which is just called during queue setting up stage, so I think
> trylock isn't necessary, but also brings complexity.
Then the NONBLOCK check can go away, and a comment added instead on why
it's fine. Or maybe even a WARN_ON_ONCE() if trylock fails or something.
Otherwise it's going to look like a code bug.
--
Jens Axboe
