* fix loop discard regression
@ 2016-08-04 14:09 Christoph Hellwig
2016-08-04 14:10 ` [PATCH 1/2] loop: don't try to use AIO for discards Christoph Hellwig
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Christoph Hellwig @ 2016-08-04 14:09 UTC (permalink / raw)
To: axboe; +Cc: ming.lei, mchristi, david, linux-block, linux-fsdevel
The first patch fixes a regression where discard request were accidentally
marked as aio after the req_op conversion. This would have been mostly
harmless except for the god-awful parsing of request types later in the
actual I/O handler which turns them into a write, so the second patch
rewrites that handler using a proper switch statement as well.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] loop: don't try to use AIO for discards
2016-08-04 14:09 fix loop discard regression Christoph Hellwig
@ 2016-08-04 14:10 ` Christoph Hellwig
2016-08-04 14:24 ` Ming Lei
2016-08-04 14:10 ` [PATCH 2/2] loop: make do_req_filebacked more robust Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2016-08-04 14:10 UTC (permalink / raw)
To: axboe; +Cc: ming.lei, mchristi, david, linux-block, linux-fsdevel
Fix a fat-fingered conversion to the req_op accessors, and also
use a switch statement to make it more obvious what is being checked.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-by: Dave Chinner <david@fromorbit.com>
Fixes: c2df40 ("drivers: use req op accessor");
---
drivers/block/loop.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 075377e..91c2c88 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1659,11 +1659,15 @@ static int loop_queue_rq(struct blk_mq_hw_ctx *hctx,
if (lo->lo_state != Lo_bound)
return -EIO;
- if (lo->use_dio && (req_op(cmd->rq) != REQ_OP_FLUSH ||
- req_op(cmd->rq) == REQ_OP_DISCARD))
- cmd->use_aio = true;
- else
+ switch (req_op(cmd->rq)) {
+ case REQ_OP_FLUSH:
+ case REQ_OP_DISCARD:
cmd->use_aio = false;
+ break;
+ default:
+ cmd->use_aio = lo->use_dio;
+ break;
+ }
queue_kthread_work(&lo->worker, &cmd->work);
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] loop: make do_req_filebacked more robust
2016-08-04 14:09 fix loop discard regression Christoph Hellwig
2016-08-04 14:10 ` [PATCH 1/2] loop: don't try to use AIO for discards Christoph Hellwig
@ 2016-08-04 14:10 ` Christoph Hellwig
2016-08-04 14:35 ` fix loop discard regression Jens Axboe
2016-08-04 18:55 ` Ross Zwisler
3 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2016-08-04 14:10 UTC (permalink / raw)
To: axboe; +Cc: ming.lei, mchristi, david, linux-block, linux-fsdevel
Use a switch statement to iterate over the possible operations and
error out if it's an incorrect one.
---
drivers/block/loop.c | 55 +++++++++++++++++++++-------------------------------
1 file changed, 22 insertions(+), 33 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 91c2c88..c9f2107 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -510,14 +510,10 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
return 0;
}
-
-static inline int lo_rw_simple(struct loop_device *lo,
- struct request *rq, loff_t pos, bool rw)
+static int do_req_filebacked(struct loop_device *lo, struct request *rq)
{
struct loop_cmd *cmd = blk_mq_rq_to_pdu(rq);
-
- if (cmd->use_aio)
- return lo_rw_aio(lo, cmd, pos, rw);
+ loff_t pos = ((loff_t) blk_rq_pos(rq) << 9) + lo->lo_offset;
/*
* lo_write_simple and lo_read_simple should have been covered
@@ -528,37 +524,30 @@ static inline int lo_rw_simple(struct loop_device *lo,
* of the req at one time. And direct read IO doesn't need to
* run flush_dcache_page().
*/
- if (rw == WRITE)
- return lo_write_simple(lo, rq, pos);
- else
- return lo_read_simple(lo, rq, pos);
-}
-
-static int do_req_filebacked(struct loop_device *lo, struct request *rq)
-{
- loff_t pos;
- int ret;
-
- pos = ((loff_t) blk_rq_pos(rq) << 9) + lo->lo_offset;
-
- if (op_is_write(req_op(rq))) {
- if (req_op(rq) == REQ_OP_FLUSH)
- ret = lo_req_flush(lo, rq);
- else if (req_op(rq) == REQ_OP_DISCARD)
- ret = lo_discard(lo, rq, pos);
- else if (lo->transfer)
- ret = lo_write_transfer(lo, rq, pos);
+ switch (req_op(rq)) {
+ case REQ_OP_FLUSH:
+ return lo_req_flush(lo, rq);
+ case REQ_OP_DISCARD:
+ return lo_discard(lo, rq, pos);
+ case REQ_OP_WRITE:
+ if (lo->transfer)
+ return lo_write_transfer(lo, rq, pos);
+ else if (cmd->use_aio)
+ return lo_rw_aio(lo, cmd, pos, WRITE);
else
- ret = lo_rw_simple(lo, rq, pos, WRITE);
-
- } else {
+ return lo_write_simple(lo, rq, pos);
+ case REQ_OP_READ:
if (lo->transfer)
- ret = lo_read_transfer(lo, rq, pos);
+ return lo_read_transfer(lo, rq, pos);
+ else if (cmd->use_aio)
+ return lo_rw_aio(lo, cmd, pos, READ);
else
- ret = lo_rw_simple(lo, rq, pos, READ);
+ return lo_read_simple(lo, rq, pos);
+ default:
+ WARN_ON_ONCE(1);
+ return -EIO;
+ break;
}
-
- return ret;
}
struct switch_request {
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] loop: don't try to use AIO for discards
2016-08-04 14:10 ` [PATCH 1/2] loop: don't try to use AIO for discards Christoph Hellwig
@ 2016-08-04 14:24 ` Ming Lei
0 siblings, 0 replies; 8+ messages in thread
From: Ming Lei @ 2016-08-04 14:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Mike Christie, Dave Chinner, linux-block, Linux FS Devel
On Thu, Aug 4, 2016 at 10:10 PM, Christoph Hellwig <hch@lst.de> wrote:
> Fix a fat-fingered conversion to the req_op accessors, and also
> use a switch statement to make it more obvious what is being checked.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reported-by: Dave Chinner <david@fromorbit.com>
> Fixes: c2df40 ("drivers: use req op accessor");
> ---
> drivers/block/loop.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 075377e..91c2c88 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1659,11 +1659,15 @@ static int loop_queue_rq(struct blk_mq_hw_ctx *hctx,
> if (lo->lo_state != Lo_bound)
> return -EIO;
>
> - if (lo->use_dio && (req_op(cmd->rq) != REQ_OP_FLUSH ||
> - req_op(cmd->rq) == REQ_OP_DISCARD))
> - cmd->use_aio = true;
> - else
> + switch (req_op(cmd->rq)) {
> + case REQ_OP_FLUSH:
> + case REQ_OP_DISCARD:
> cmd->use_aio = false;
> + break;
> + default:
> + cmd->use_aio = lo->use_dio;
> + break;
> + }
Reviewed-by: Ming Lei <ming.lei@canonical.com>
>
> queue_kthread_work(&lo->worker, &cmd->work);
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: fix loop discard regression
2016-08-04 14:09 fix loop discard regression Christoph Hellwig
2016-08-04 14:10 ` [PATCH 1/2] loop: don't try to use AIO for discards Christoph Hellwig
2016-08-04 14:10 ` [PATCH 2/2] loop: make do_req_filebacked more robust Christoph Hellwig
@ 2016-08-04 14:35 ` Jens Axboe
2016-08-04 18:55 ` Ross Zwisler
3 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2016-08-04 14:35 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: ming.lei, mchristi, david, linux-block, linux-fsdevel
On 08/04/2016 08:09 AM, Christoph Hellwig wrote:
> The first patch fixes a regression where discard request were accidentally
> marked as aio after the req_op conversion. This would have been mostly
> harmless except for the god-awful parsing of request types later in the
> actual I/O handler which turns them into a write, so the second patch
> rewrites that handler using a proper switch statement as well.
Applied, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: fix loop discard regression
2016-08-04 14:09 fix loop discard regression Christoph Hellwig
` (2 preceding siblings ...)
2016-08-04 14:35 ` fix loop discard regression Jens Axboe
@ 2016-08-04 18:55 ` Ross Zwisler
2016-08-05 7:39 ` Christoph Hellwig
3 siblings, 1 reply; 8+ messages in thread
From: Ross Zwisler @ 2016-08-04 18:55 UTC (permalink / raw)
To: Christoph Hellwig
Cc: axboe, ming.lei, Mike Christie, Dave Chinner, linux-block, linux-fsdevel
On Thu, Aug 4, 2016 at 8:09 AM, Christoph Hellwig <hch@lst.de> wrote:
> The first patch fixes a regression where discard request were accidentally
> marked as aio after the req_op conversion. This would have been mostly
> harmless except for the god-awful parsing of request types later in the
> actual I/O handler which turns them into a write, so the second patch
> rewrites that handler using a proper switch statement as well.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Also, your 2nd patch is missing a signed-off-by.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: fix loop discard regression
2016-08-04 18:55 ` Ross Zwisler
@ 2016-08-05 7:39 ` Christoph Hellwig
2016-08-05 13:51 ` Jens Axboe
0 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2016-08-05 7:39 UTC (permalink / raw)
To: Ross Zwisler
Cc: Christoph Hellwig, axboe, ming.lei, Mike Christie, Dave Chinner,
linux-block, linux-fsdevel
On Thu, Aug 04, 2016 at 12:55:26PM -0600, Ross Zwisler wrote:
> On Thu, Aug 4, 2016 at 8:09 AM, Christoph Hellwig <hch@lst.de> wrote:
> > The first patch fixes a regression where discard request were accidentally
> > marked as aio after the req_op conversion. This would have been mostly
> > harmless except for the god-awful parsing of request types later in the
> > actual I/O handler which turns them into a write, so the second patch
> > rewrites that handler using a proper switch statement as well.
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-block" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>
> Also, your 2nd patch is missing a signed-off-by.
Signed-off-by: Christoph Hellwig <hch@lst.de>
in case that Jens wants to bother with force-updating the tree.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: fix loop discard regression
2016-08-05 7:39 ` Christoph Hellwig
@ 2016-08-05 13:51 ` Jens Axboe
0 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2016-08-05 13:51 UTC (permalink / raw)
To: Christoph Hellwig, Ross Zwisler
Cc: ming.lei, Mike Christie, Dave Chinner, linux-block, linux-fsdevel
On 08/05/2016 01:39 AM, Christoph Hellwig wrote:
> On Thu, Aug 04, 2016 at 12:55:26PM -0600, Ross Zwisler wrote:
>> On Thu, Aug 4, 2016 at 8:09 AM, Christoph Hellwig <hch@lst.de> wrote:
>>> The first patch fixes a regression where discard request were accidentally
>>> marked as aio after the req_op conversion. This would have been mostly
>>> harmless except for the god-awful parsing of request types later in the
>>> actual I/O handler which turns them into a write, so the second patch
>>> rewrites that handler using a proper switch statement as well.
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-block" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>>
>> Also, your 2nd patch is missing a signed-off-by.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> in case that Jens wants to bother with force-updating the tree.
Not really, was hoping to flush it out today. Don't see it as a huge
problem...
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-08-05 13:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-04 14:09 fix loop discard regression Christoph Hellwig
2016-08-04 14:10 ` [PATCH 1/2] loop: don't try to use AIO for discards Christoph Hellwig
2016-08-04 14:24 ` Ming Lei
2016-08-04 14:10 ` [PATCH 2/2] loop: make do_req_filebacked more robust Christoph Hellwig
2016-08-04 14:35 ` fix loop discard regression Jens Axboe
2016-08-04 18:55 ` Ross Zwisler
2016-08-05 7:39 ` Christoph Hellwig
2016-08-05 13:51 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).