Re: [PATCH 17/17] nvme: enable non-inline passthru commands

[Jens Axboe <axboe@kernel.dk>]

On 3/24/22 3:09 PM, Clay Mayers wrote:
>> From: Kanchan Joshi
>> Sent: Tuesday, March 8, 2022 7:21 AM
>> To: axboe@kernel.dk; hch@lst.de; kbusch@kernel.org;
>> asml.silence@gmail.com
>> Cc: io-uring@vger.kernel.org; linux-nvme@lists.infradead.org; linux-
>> block@vger.kernel.org; sbates@raithlin.com; logang@deltatee.com;
>> pankydev8@gmail.com; javier@javigon.com; mcgrof@kernel.org;
>> a.manzanares@samsung.com; joshiiitr@gmail.com; anuj20.g@samsung.com
>> Subject: [PATCH 17/17] nvme: enable non-inline passthru commands
>>
>> From: Anuj Gupta <anuj20.g@samsung.com>
>>
>> On submission,just fetch the commmand from userspace pointer and reuse
>> everything else. On completion, update the result field inside the passthru
>> command.
>>
>> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
>> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
>> ---
>>  drivers/nvme/host/ioctl.c | 29 +++++++++++++++++++++++++----
>>  1 file changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c index
>> 701feaecabbe..ddb7e5864be6 100644
>> --- a/drivers/nvme/host/ioctl.c
>> +++ b/drivers/nvme/host/ioctl.c
>> @@ -65,6 +65,14 @@ static void nvme_pt_task_cb(struct io_uring_cmd
>> *ioucmd)
>>  	}
>>  	kfree(pdu->meta);
>>
>> +	if (ioucmd->flags & IO_URING_F_UCMD_INDIRECT) {
>> +		struct nvme_passthru_cmd64 __user *ptcmd64 = ioucmd-
>>> cmd;
>> +		u64 result = le64_to_cpu(nvme_req(req)->result.u64);
>> +
>> +		if (put_user(result, &ptcmd64->result))
>> +			status = -EFAULT;
> 
> When the thread that submitted the io_uring_cmd has exited, the CB is
> called by a system worker instead so put_user() fails.  The cqe is
> still completed and the process sees a failed i/o status, but the i/o
> did not fail.  The same is true for meta data being returned in patch
> 5.
> 
> I can't say if it's a requirement to support this case.  It does break
> our current proto-type but we can adjust.

Just don't do that then - it's all very much task based. If the task
goes away and completions haven't been reaped, don't count on anything
sane happening in terms of them completing successfully or not.

The common case for this happening is offloading submit to a submit
thread, which is utterly pointless with io_uring anyway.

-- 
Jens Axboe