Re: [PATCH v4 7/8] libnvdimm: Adding blk-mq support to the pmem driver

[Christoph Hellwig <hch@infradead.org>]

On Mon, Aug 07, 2017 at 09:39:53AM -0700, Dave Jiang wrote:
> Adding blk-mq support to the pmem driver in addition to the direct bio
> support.

Can you explain why this is only done for pmem and not btt and nd_blk?

> This allows for hardware offloading via DMA engines. By default
> the bio method will be enabled. The blk-mq support can be turned on via
> module parameter queue_mode=1.

Any way to auto-discovery the right mode?  Also I'd actually much
prefer for this to be a separate driver instead of having two entirely
different I/O paths in the same module.

> +struct pmem_cmd {
> +	struct request *rq;
> +};

There is no point in having private data that just has a struct
request backpointer.  Just pass the request along.

>  
> -static void pmem_release_queue(void *q)
> +static void pmem_release_queue(void *data)
>  {
> -	blk_cleanup_queue(q);
> +	struct pmem_device *pmem = (struct pmem_device *)data;

No need for the cast.

> +static int pmem_handle_cmd(struct pmem_cmd *cmd)

Please merge this into the queue_rq handler.

> +	struct request *req = cmd->rq;
> +	struct request_queue *q = req->q;
> +	struct pmem_device *pmem = q->queuedata;
> +	struct nd_region *nd_region = to_region(pmem);
> +	struct bio_vec bvec;
> +	struct req_iterator iter;
> +	int rc = 0;
> +
> +	if (req->cmd_flags & REQ_FLUSH)
> +		nvdimm_flush(nd_region);

For a blk-mq driver you need to check for REQ_OP_FLUSH, .e.g.

	switch (req_op(req)) {
	case REQ_FLUSH:
		ret = nvdimm_flush(nd_region);
		break;
	case REQ_OP_READ:
		ret = pmem_do_io(req, false);
		break;
	case REQ_OP_WRITE:
		ret = pmem_do_io(req, true);
		if (req->cmd_flags & REQ_FUA)
			nvdimm_flush(nd_region);
		break;
	default:
		ret = BLK_STS_NOTSUPP;
		break;
	}

> +		pmem->tag_set.queue_depth = 64;

Where does this magic number come from?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm