Re: [PATCH v2] zram: support REQ_DISCARD

[Joonsoo Kim]

2014-02-26 23:06 GMT+09:00 Jerome Marchand <jmarchan@xxxxxxxxxx>:
> On 02/26/2014 02:57 PM, Sergey Senozhatsky wrote:
>> On (02/26/14 14:44), Jerome Marchand wrote:
>>> On 02/26/2014 02:16 PM, Sergey Senozhatsky wrote:
>>>> Hello,
>>>>
>>>> On (02/26/14 14:23), Joonsoo Kim wrote:
>>>>> zram is ram based block device and can be used by backend of filesystem.
>>>>> When filesystem deletes a file, it normally doesn't do anything on data
>>>>> block of that file. It just marks on metadata of that file. This behavior
>>>>> has no problem on disk based block device, but has problems on ram based
>>>>> block device, since we can't free memory used for data block. To overcome
>>>>> this disadvantage, there is REQ_DISCARD functionality. If block device
>>>>> support REQ_DISCARD and filesystem is mounted with discard option,
>>>>> filesystem sends REQ_DISCARD to block device whenever some data blocks are
>>>>> discarded. All we have to do is to handle this request.
>>>>>
>>>>> This patch implements to flag up QUEUE_FLAG_DISCARD and handle this
>>>>> REQ_DISCARD request. With it, we can free memory used by zram if it isn't
>>>>> used.
>>>>>
>>>>> v2: handle unaligned case commented by Jerome
>>>>>
>>>>> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
>>>>>
>>>>> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
>>>>> index 5ec61be..5364c1e 100644
>>>>> --- a/drivers/block/zram/zram_drv.c
>>>>> +++ b/drivers/block/zram/zram_drv.c
>>>>> @@ -501,6 +501,36 @@ static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
>>>>>    return ret;
>>>>>  }
>>>>>
>>>>> +static void zram_bio_discard(struct zram *zram, struct bio *bio)
>>>>> +{
>>>>> +  u32 index = bio->bi_iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
>>>>> +  size_t n = bio->bi_iter.bi_size;
>>>>> +  size_t misalign;
>>>>> +
>>>>> +  /*
>>>>> +   * On some arch, logical block (4096) aligned request couldn't be
>>>>> +   * aligned to PAGE_SIZE, since their PAGE_SIZE aren't 4096.
>>>>> +   * Therefore we should handle this misaligned case here.
>>>>> +   */
>>>>> +  misalign = (bio->bi_iter.bi_sector &
>>>>> +                  (SECTORS_PER_PAGE - 1)) << SECTOR_SHIFT;
>>>>> +  if (misalign) {
>>>>> +          if (n < misalign)
>>>>> +                  return;
>>>>> +
>>>>> +          n -= misalign;
>>>>> +          index++;
>>>>> +  }
>>>>> +
>>>>> +  while (n >= PAGE_SIZE) {
>>>>> +          write_lock(&zram->meta->tb_lock);
>>>>> +          zram_free_page(zram, index);
>>>>> +          write_unlock(&zram->meta->tb_lock);
>>>>> +          index++;
>>>>> +          n -= PAGE_SIZE;
>>>>> +  }
>>>>> +}
>>>>> +
>>>>
>>>> a side note, do we need zram_bio_discard() function? I mean, can we handle
>>>> discard request in zram_bvec_rw(), where we already know index, etc. (passed
>>>> from __zram_make_request())?
>>>>

Hello, Sergey.
Sorry for late response.

I think that introducing new function is better idea, since
discard_request is significantly
different with rw request. First of all, it doesn't use bvec. So splitting code
in __zram_make_request() would not work properly for it. And
zram_bvec_rw() is bvec handler
and deals with PAGE_SIZE unit request which is not appropriate for
discard request.

But, it is good to use common index, offset, so I will move down position of
zram_bio_discard().

Thanks for comment!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/