Re: [PATCH 2/2] UBI: Block: Add blk-mq support

[Christoph Hellwig]

> +struct ubiblock_pdu {
> +	struct request *req;

No need to store the request, you can trivially get at it using
blk_mq_rq_from_pdu().

> +	struct ubiblock *dev;

Why do you need the dev pointer?  You can always trivially get
it using req->queuedata.

> +static void ubiblock_do_work(struct work_struct *work)
> +{
> +	int ret;
> +	struct ubiblock_pdu *pdu = container_of(work, struct ubiblock_pdu, work);
> +
> +	ret = ubiblock_read(pdu);
> +	blk_mq_end_request(pdu->req, ret ?: 0);

Why not just pass ret as-is?

> +	if (blk_rq_pos(req) + blk_rq_cur_sectors(req) >
> +	    get_capacity(req->rq_disk))
> +		return BLK_MQ_RQ_QUEUE_ERROR;

The upper layers take are of this check.

> +	pdu->usgl.list_pos = 0;
> +	pdu->usgl.page_pos = 0;

Having a helper to initialize a ubi_sgl would be nicer than having
to open code it ike here.

> +
> +	blk_mq_start_request(req);
> +	ret = blk_rq_map_sg(hctx->queue, req, pdu->usgl.sg);
> +
> +	queue_work(dev->wq, &pdu->work);

Why don't you move these calls into the work queue as well?  The
queue_rq call would literally just become a queue_work call.

And given that this is a fairly common patter this should allow
us to refactor / optimize this case a bit later on.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/