Re: [PATCH v4 4/5] nvme: wire-up uring-cmd support for io-passthru on char-device.

From: Christoph Hellwig <hch@lst.de>
To: Jens Axboe <axboe@kernel.dk>
Cc: Kanchan Joshi <joshi.k@samsung.com>,
	hch@lst.de, io-uring@vger.kernel.org,
	linux-nvme@lists.infradead.org, asml.silence@gmail.com,
	ming.lei@redhat.com, mcgrof@kernel.org, shr@fb.com,
	joshiiitr@gmail.com, anuj20.g@samsung.com, gost.dev@samsung.com
Subject: Re: [PATCH v4 4/5] nvme: wire-up uring-cmd support for io-passthru on char-device.
Date: Thu, 5 May 2022 15:42:56 +0200	[thread overview]
Message-ID: <20220505134256.GA13109@lst.de> (raw)
In-Reply-To: <f24d9e5e-34af-9035-ffbc-0a770db0cb20@kernel.dk>

On Thu, May 05, 2022 at 07:38:31AM -0600, Jens Axboe wrote:
> > +	req = nvme_alloc_user_request(q, &c, nvme_to_user_ptr(cmd->addr),
> > +			cmd->data_len, nvme_to_user_ptr(cmd->metadata),
> > +			cmd->metadata_len, 0, cmd->timeout_ms ?
> > +			msecs_to_jiffies(cmd->timeout_ms) : 0, 0, rq_flags,
> > +			blk_flags);
> 
> You need to be careful with reading/re-reading the shared memory. For
> example, you do:

Uh, yes.  With ioucmd->cmd pointing to the user space mapped SQ
we need to be very careful here.  To the point where I'd almost prfer
to memcpy it out first, altough there might be performance implications.

On something unrelated while looking over the code again:  the cast
when asssigning cmd in nvme_uring_cmd_io should not be needed any more.