From: Christoph Hellwig <hch@infradead.org>
To: Richard Weinberger <richard@nod.at>
Cc: axboe@fb.com, dedekind1@gmail.com, tom.leiming@gmail.com,
linux-kernel@vger.kernel.org, hch@infradead.org,
linux-mtd@lists.infradead.org, computersforpeace@gmail.com,
dwmw2@infradead.org
Subject: Re: [PATCH 2/2] UBI: Block: Add blk-mq support
Date: Sat, 10 Jan 2015 10:58:51 -0800 [thread overview]
Message-ID: <20150110185851.GA3945@infradead.org> (raw)
In-Reply-To: <1420900188-15781-2-git-send-email-richard@nod.at>
> +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.
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: Richard Weinberger <richard@nod.at>
Cc: dedekind1@gmail.com, dwmw2@infradead.org,
computersforpeace@gmail.com, linux-mtd@lists.infradead.org,
linux-kernel@vger.kernel.org, hch@infradead.org, axboe@fb.com,
tom.leiming@gmail.com
Subject: Re: [PATCH 2/2] UBI: Block: Add blk-mq support
Date: Sat, 10 Jan 2015 10:58:51 -0800 [thread overview]
Message-ID: <20150110185851.GA3945@infradead.org> (raw)
In-Reply-To: <1420900188-15781-2-git-send-email-richard@nod.at>
> +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.
next prev parent reply other threads:[~2015-01-10 18:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-10 14:29 [PATCH 1/2] UBI: Add initial support for scatter gather Richard Weinberger
2015-01-10 14:29 ` Richard Weinberger
2015-01-10 14:29 ` [PATCH 2/2] UBI: Block: Add blk-mq support Richard Weinberger
2015-01-10 14:29 ` Richard Weinberger
2015-01-10 18:58 ` Christoph Hellwig [this message]
2015-01-10 18:58 ` Christoph Hellwig
2015-01-10 21:52 ` Richard Weinberger
2015-01-10 21:52 ` Richard Weinberger
-- strict thread matches above, loose matches on Subject: below --
2014-11-24 16:04 UBI: Add support for scatter gather and blk-mq Richard Weinberger
2014-11-24 16:04 ` [PATCH 2/2] UBI: Block: Add blk-mq support Richard Weinberger
2014-11-24 16:04 ` Richard Weinberger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150110185851.GA3945@infradead.org \
--to=hch@infradead.org \
--cc=axboe@fb.com \
--cc=computersforpeace@gmail.com \
--cc=dedekind1@gmail.com \
--cc=dwmw2@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=richard@nod.at \
--cc=tom.leiming@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.