Re: [PATCH v3 2/3] nvme: Pass pointers, not dma addresses, to nvme_get/set_features()

From: Jens Axboe
Date: Fri Sep 16 2016 - 11:35:12 EST

On 09/16/2016 09:13 AM, Andy Lutomirski wrote:
On Sep 16, 2016 1:41 AM, "Christoph Hellwig" <hch@xxxxxx> wrote:

On Thu, Sep 15, 2016 at 10:24:20PM -0700, Andy Lutomirski wrote:
Any user I can imagine that needs a buffer at all will want to pass
a pointer directly. There are no currently callers that use
buffers, so this change is painless, and it will make it much easier
to start using features that use buffers (e.g. APST).

Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>

Looks good mostly good, but a nitpick below:

+ /*
+ * Casting buffer to void* is safe here: __nvme_submit_sync_cmd knows
+ * that we're writing because it decodes the opcode.
+ */
+ ret = __nvme_submit_sync_cmd(dev->admin_q, &c, &cqe,
+ (void *)buffer, buflen, 0, NVME_QID_ANY, 0, 0);

Cant we just drop the const annotation to avoid these casts?

Then we'd have nvme_set_feature() taking a non-const pointer, which
would seem a little bit silly to me and might require the same cast
somewhere else down the road.

That'd still be a lot cleaner than the cast, which needs a comment to
explain why it's supposedly safe. Just drop the const, imho.

Jens Axboe