Hi Jens,
It seems you miss this.
Could you handle this?
Thanks.
On Thu, Mar 9, 2017 at 2:28 PM, Minchan Kim <minchan@xxxxxxxxxx> wrote:
< snip>
> Jens, Could you replace the one merged with this? And I don't want
> to add stable mark in this patch because I feel it need enough
> testing in 64K page system I don't have. ;(
>
> From bb73e75ab0e21016f60858fd61e7dc6a6813e359 Mon Sep 17 00:00:00 2001
> From: Minchan Kim <minchan@xxxxxxxxxx>
> Date: Thu, 9 Mar 2017 14:00:40 +0900
> Subject: [PATCH] zram: handle multiple pages attached bio's bvec
>
> Johannes Thumshirn reported system goes the panic when using NVMe over
> Fabrics loopback target with zram.
>
> The reason is zram expects each bvec in bio contains a single page
> but nvme can attach a huge bulk of pages attached to the bio's bvec
> so that zram's index arithmetic could be wrong so that out-of-bound
> access makes panic.
>
> This patch solves the problem via removing the limit(a bvec should
> contains a only single page).
>
> Cc: Hannes Reinecke <hare@xxxxxxxx>
> Reported-by: Johannes Thumshirn <jthumshirn@xxxxxxx>
> Tested-by: Johannes Thumshirn <jthumshirn@xxxxxxx>
> Reviewed-by: Johannes Thumshirn <jthumshirn@xxxxxxx>
> Signed-off-by: Johannes Thumshirn <jthumshirn@xxxxxxx>
> Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx>
> ---
> I don't add stable mark intentionally because I think it's rather risky
> without enough testing on 64K page system(ie, partial IO part).
>
> Thanks for the help, Johannes and Hannes!!
>
> drivers/block/zram/zram_drv.c | 37 ++++++++++---------------------------
> 1 file changed, 10 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 01944419b1f3..fefdf260503a 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -137,8 +137,7 @@ static inline bool valid_io_request(struct zram *zram,
>
> static void update_position(u32 *index, int *offset, struct bio_vec *bvec)
> {
> - if (*offset + bvec->bv_len >= PAGE_SIZE)
> - (*index)++;
> + *index += (*offset + bvec->bv_len) / PAGE_SIZE;
> *offset = (*offset + bvec->bv_len) % PAGE_SIZE;
> }
>
> @@ -838,34 +837,20 @@ static void __zram_make_request(struct zram *zram, struct bio *bio)
> }
>
> bio_for_each_segment(bvec, bio, iter) {
> - int max_transfer_size = PAGE_SIZE - offset;
> -
> - if (bvec.bv_len > max_transfer_size) {
> - /*
> - * zram_bvec_rw() can only make operation on a single
> - * zram page. Split the bio vector.
> - */
> - struct bio_vec bv;
> -
> - bv.bv_page = bvec.bv_page;
> - bv.bv_len = max_transfer_size;
> - bv.bv_offset = bvec.bv_offset;
> + struct bio_vec bv = bvec;
> + unsigned int remained = bvec.bv_len;
>
> + do {
> + bv.bv_len = min_t(unsigned int, PAGE_SIZE, remained);
> if (zram_bvec_rw(zram, &bv, index, offset,
> - op_is_write(bio_op(bio))) < 0)
> + op_is_write(bio_op(bio))) < 0)
> goto out;
>
> - bv.bv_len = bvec.bv_len - max_transfer_size;
> - bv.bv_offset += max_transfer_size;
> - if (zram_bvec_rw(zram, &bv, index + 1, 0,
> - op_is_write(bio_op(bio))) < 0)
> - goto out;
> - } else
> - if (zram_bvec_rw(zram, &bvec, index, offset,
> - op_is_write(bio_op(bio))) < 0)
> - goto out;
> + bv.bv_offset += bv.bv_len;
> + remained -= bv.bv_len;
>
> - update_position(&index, &offset, &bvec);
> + update_position(&index, &offset, &bv);
> + } while (remained);
> }
>
> bio_endio(bio);
> @@ -882,8 +867,6 @@ static blk_qc_t zram_make_request(struct request_queue *queue, struct bio *bio)
> {
> struct zram *zram = queue->queuedata;
>
> - blk_queue_split(queue, &bio, queue->bio_split);
> -
> if (!valid_io_request(zram, bio->bi_iter.bi_sector,
> bio->bi_iter.bi_size)) {
> atomic64_inc(&zram->stats.invalid_io);
> --
> 2.7.4
>
>
--
Kind regards,
Minchan Kim
