detatched should be detached, otherwise lgtm.
Reviewed-by: Michael Lyle <mlyle@xxxxxxxx>
On 02/05/2018 08:20 PM, Coly Li wrote:
> From: Tang Junhui <tang.junhui@xxxxxxxxxx>
>
> When we run IO in a detached device, and run iostat to shows IO status,
> normally it will show like bellow (Omitted some fields):
> Device: ... avgrq-sz avgqu-sz await r_await w_await svctm %util
> sdd ... 15.89 0.53 1.82 0.20 2.23 1.81 52.30
> bcache0 ... 15.89 115.42 0.00 0.00 0.00 2.40 69.60
> but after IO stopped, there are still very big avgqu-sz and %util
> values as bellow:
> Device: ... avgrq-sz avgqu-sz await r_await w_await svctm %util
> bcache0 ... 0 5326.32 0.00 0.00 0.00 0.00 100.10
>
> The reason for this issue is that, only generic_start_io_acct() called
> and no generic_end_io_acct() called for detached device in
> cached_dev_make_request(). See the code:
> //start generic_start_io_acct()
> generic_start_io_acct(q, rw, bio_sectors(bio), &d->disk->part0);
> if (cached_dev_get(dc)) {
> //will callback generic_end_io_acct()
> }
> else {
> //will not call generic_end_io_acct()
> }
>
> This patch calls generic_end_io_acct() in the end of IO for detached
> devices, so we can show IO state correctly.
>
> (Modified to use GFP_NOIO in kzalloc() by Coly Li)
>
> Signed-off-by: Tang Junhui <tang.junhui@xxxxxxxxxx>
> Reviewed-by: Coly Li <colyli@xxxxxxx>
> Reviewed-by: Hannes Reinecke <hare@xxxxxxxx>
> ---
> drivers/md/bcache/request.c | 58 +++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 51 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index 02296bda6384..e09c5ae745be 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -986,6 +986,55 @@ static void cached_dev_nodata(struct closure *cl)
> continue_at(cl, cached_dev_bio_complete, NULL);
> }
>
> +struct detached_dev_io_private {
> + struct bcache_device *d;
> + unsigned long start_time;
> + bio_end_io_t *bi_end_io;
> + void *bi_private;
> +};
> +
> +static void detatched_dev_end_io(struct bio *bio)
> +{
> + struct detached_dev_io_private *ddip;
> +
> + ddip = bio->bi_private;
> + bio->bi_end_io = ddip->bi_end_io;
> + bio->bi_private = ddip->bi_private;
> +
> + generic_end_io_acct(ddip->d->disk->queue,
> + bio_data_dir(bio),
> + &ddip->d->disk->part0, ddip->start_time);
> +
> + kfree(ddip);
> +
> + bio->bi_end_io(bio);
> +}
> +
> +static void detached_dev_do_request(struct bcache_device *d, struct bio *bio)
> +{
> + struct detached_dev_io_private *ddip;
> + struct cached_dev *dc = container_of(d, struct cached_dev, disk);
> +
> + /*
> + * no need to call closure_get(&dc->disk.cl),
> + * because upper layer had already opened bcache device,
> + * which would call closure_get(&dc->disk.cl)
> + */
> + ddip = kzalloc(sizeof(struct detached_dev_io_private), GFP_NOIO);
> + ddip->d = d;
> + ddip->start_time = jiffies;
> + ddip->bi_end_io = bio->bi_end_io;
> + ddip->bi_private = bio->bi_private;
> + bio->bi_end_io = detatched_dev_end_io;
> + bio->bi_private = ddip;
> +
> + if ((bio_op(bio) == REQ_OP_DISCARD) &&
> + !blk_queue_discard(bdev_get_queue(dc->bdev)))
> + bio->bi_end_io(bio);
> + else
> + generic_make_request(bio);
> +}
> +
> /* Cached devices - read & write stuff */
>
> static blk_qc_t cached_dev_make_request(struct request_queue *q,
> @@ -1028,13 +1077,8 @@ static blk_qc_t cached_dev_make_request(struct request_queue *q,
> else
> cached_dev_read(dc, s);
> }
> - } else {
> - if ((bio_op(bio) == REQ_OP_DISCARD) &&
> - !blk_queue_discard(bdev_get_queue(dc->bdev)))
> - bio_endio(bio);
> - else
> - generic_make_request(bio);
> - }
> + } else
> + detached_dev_do_request(d, bio);
>
> return BLK_QC_T_NONE;
> }
>
