Messages in this thread | | | Date | Fri, 10 Jan 2014 16:14:45 +0900 | From | Minchan Kim <> | Subject | Re: [PATCH -next] zram: do not pass rw argument to __zram_make_request() |
| |
On Thu, Jan 09, 2014 at 04:23:43PM +0300, Sergey Senozhatsky wrote: > Do not pass rw argument down the __zram_make_request() -> zram_bvec_rw() > chain, decode it in zram_bvec_rw() instead. Besides, this is the place > where we distinguish READ and WRITE bio data directions, so account zram > RW stats here, instead of __zram_make_request(). This also allows to > account a real number of zram READ/WRITE operations, not just requests > (RW request may end up in a number of zram RW ops with separate > locking, compression/decompression, slot free handling, etc).
Looks sane to me because other statistic variable accounts per bio, not request. But it changes current statistic's semantic so they could be regressed. Let's wait a bit for other's opinion.
Thanks.
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > > --- > > drivers/block/zram/zram_drv.c | 29 ++++++++++++----------------- > 1 file changed, 12 insertions(+), 17 deletions(-) > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c > index 833d24f..798793f 100644 > --- a/drivers/block/zram/zram_drv.c > +++ b/drivers/block/zram/zram_drv.c > @@ -534,22 +534,27 @@ static void handle_pending_slot_free(struct zram *zram) > } > > static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index, > - int offset, struct bio *bio, int rw) > + int offset, struct bio *bio) > { > int ret; > + int rw = bio_data_dir(bio); > + > + if (rw == READA) > + rw = READ; > > if (rw == READ) { > + atomic64_inc(&zram->stats.num_reads); > down_read(&zram->lock); > handle_pending_slot_free(zram); > ret = zram_bvec_read(zram, bvec, index, offset, bio); > up_read(&zram->lock); > } else { > + atomic64_inc(&zram->stats.num_writes); > down_write(&zram->lock); > handle_pending_slot_free(zram); > ret = zram_bvec_write(zram, bvec, index, offset); > up_write(&zram->lock); > } > - > return ret; > } > > @@ -680,22 +685,13 @@ out: > return ret; > } > > -static void __zram_make_request(struct zram *zram, struct bio *bio, int rw) > +static void __zram_make_request(struct zram *zram, struct bio *bio) > { > int offset; > u32 index; > struct bio_vec bvec; > struct bvec_iter iter; > > - switch (rw) { > - case READ: > - atomic64_inc(&zram->stats.num_reads); > - break; > - case WRITE: > - atomic64_inc(&zram->stats.num_writes); > - break; > - } > - > index = bio->bi_iter.bi_sector >> SECTORS_PER_PAGE_SHIFT; > offset = (bio->bi_iter.bi_sector & > (SECTORS_PER_PAGE - 1)) << SECTOR_SHIFT; > @@ -714,16 +710,15 @@ static void __zram_make_request(struct zram *zram, struct bio *bio, int rw) > bv.bv_len = max_transfer_size; > bv.bv_offset = bvec.bv_offset; > > - if (zram_bvec_rw(zram, &bv, index, offset, bio, rw) < 0) > + if (zram_bvec_rw(zram, &bv, index, offset, 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, bio, rw) < 0) > + if (zram_bvec_rw(zram, &bv, index + 1, 0, bio) < 0) > goto out; > } else > - if (zram_bvec_rw(zram, &bvec, index, offset, bio, rw) > - < 0) > + if (zram_bvec_rw(zram, &bvec, index, offset, bio) < 0) > goto out; > > update_position(&index, &offset, &bvec); > @@ -753,7 +748,7 @@ static void zram_make_request(struct request_queue *queue, struct bio *bio) > goto error; > } > > - __zram_make_request(zram, bio, bio_data_dir(bio)); > + __zram_make_request(zram, bio); > up_read(&zram->init_lock); > > return; > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
-- Kind regards, Minchan Kim
| |