question about block-throttle on data device of dm-thin pool

question about block-throttle on data device of dm-thin pool

[Hou Tao]

Hi, all.

I am trying to test block-throttle on dm-thin devices. I find the throttling
on dm-thin device is OK, but the throttling doesn't work for the data device
of dm-thin pool.

The following is my test case:
#!/bin/sh

dmsetup create pool --table '0 41943040 thin-pool /dev/vdb /dev/vda \
	128 6553 1 skip_block_zeroing
dmsetup message /dev/mapper/pool 0 'create_thin 1'
dmsetup create thin_1 --table '0 41943040 thin /dev/mapper/pool 1'

mp=/thin_1
mkfs.xfs /dev/mapper/thin_1
mount /dev/mapper/thin_1 $mp

cg=/sys/fs/cgroup/blkio/test
mkdir -p $cg
# get the block device id of the data device
data_dev=$(dmsetup table /dev/mapper/pool | awk '{print $5}')
echo "${data_dev} 1048576" > $cg/blkio.throttle.write_bps_device
echo $$ > $cg/cgroup.procs
dd if=/dev/zero of=$mp/zero bs=1M count=1 oflag=direct

I read the dm-thin code roughly and find out that most bios are submitted
by the workqueue of thin pool instead of the dd process which initiates the
O_DIRECT write operations. The bios belong to the block cgroup "blkcg_root"
instead of the created block cgroup "test" in test case, so the write
limitation of blkcg "test" doesn't work.

In order to make the throttling work out, can we save the original block
cgroup info of the deferred bios and use the saved block cgroup info to
submit the bios ? Is the method reasonable and is there a better way to
complete the throttling on the data device of the thin pool ?

Thanks.

Tao

Re: question about block-throttle on data device of dm-thin pool

[Vivek Goyal]

On Tue, Jan 10, 2017 at 02:47:02PM +0800, Hou Tao wrote:
> Hi, all.
> 
> I am trying to test block-throttle on dm-thin devices. I find the throttling
> on dm-thin device is OK, but the throttling doesn't work for the data device
> of dm-thin pool.
> 
> The following is my test case:
> #!/bin/sh
> 
> dmsetup create pool --table '0 41943040 thin-pool /dev/vdb /dev/vda \
> 	128 6553 1 skip_block_zeroing
> dmsetup message /dev/mapper/pool 0 'create_thin 1'
> dmsetup create thin_1 --table '0 41943040 thin /dev/mapper/pool 1'
> 
> mp=/thin_1
> mkfs.xfs /dev/mapper/thin_1
> mount /dev/mapper/thin_1 $mp
> 
> cg=/sys/fs/cgroup/blkio/test
> mkdir -p $cg
> # get the block device id of the data device
> data_dev=$(dmsetup table /dev/mapper/pool | awk '{print $5}')
> echo "${data_dev} 1048576" > $cg/blkio.throttle.write_bps_device
> echo $$ > $cg/cgroup.procs
> dd if=/dev/zero of=$mp/zero bs=1M count=1 oflag=direct
> 
> I read the dm-thin code roughly and find out that most bios are submitted
> by the workqueue of thin pool instead of the dd process which initiates the
> O_DIRECT write operations. The bios belong to the block cgroup "blkcg_root"
> instead of the created block cgroup "test" in test case, so the write
> limitation of blkcg "test" doesn't work.
> 
> In order to make the throttling work out, can we save the original block
> cgroup info of the deferred bios and use the saved block cgroup info to
> submit the bios ? Is the method reasonable and is there a better way to
> complete the throttling on the data device of the thin pool ?

I thought we had a patches where bio_clone_bioset() also retained
cgroup information of original bio.

commit 20bd723ec6a3261df5e02250cd3a1fbb09a343f2
Author: Paolo Valente <paolo.valente@linaro.org>
Date:   Wed Jul 27 07:22:05 2016 +0200

    block: add missing group association in bio-cloning functions

Are you running new enough kernel. If not, may be there are still
some places either in generic code or dm code where cloned/newly
created bios are attributed to root cgroup and not to the cgroup
of bio which caused creation of that new bio.

Vivek

Re: question about block-throttle on data device of dm-thin pool

[Hou Tao]

On 2017/1/11 3:42, Vivek Goyal wrote:
> On Tue, Jan 10, 2017 at 02:47:02PM +0800, Hou Tao wrote:
>> Hi, all.
>>
>> I am trying to test block-throttle on dm-thin devices. I find the throttling
>> on dm-thin device is OK, but the throttling doesn't work for the data device
>> of dm-thin pool.
>>
>> The following is my test case:
>> #!/bin/sh
>>
>> dmsetup create pool --table '0 41943040 thin-pool /dev/vdb /dev/vda \
>> 	128 6553 1 skip_block_zeroing
>> dmsetup message /dev/mapper/pool 0 'create_thin 1'
>> dmsetup create thin_1 --table '0 41943040 thin /dev/mapper/pool 1'
>>
>> mp=/thin_1
>> mkfs.xfs /dev/mapper/thin_1
>> mount /dev/mapper/thin_1 $mp
>>
>> cg=/sys/fs/cgroup/blkio/test
>> mkdir -p $cg
>> # get the block device id of the data device
>> data_dev=$(dmsetup table /dev/mapper/pool | awk '{print $5}')
>> echo "${data_dev} 1048576" > $cg/blkio.throttle.write_bps_device
>> echo $$ > $cg/cgroup.procs
>> dd if=/dev/zero of=$mp/zero bs=1M count=1 oflag=direct
>>
>> I read the dm-thin code roughly and find out that most bios are submitted
>> by the workqueue of thin pool instead of the dd process which initiates the
>> O_DIRECT write operations. The bios belong to the block cgroup "blkcg_root"
>> instead of the created block cgroup "test" in test case, so the write
>> limitation of blkcg "test" doesn't work.
>>
>> In order to make the throttling work out, can we save the original block
>> cgroup info of the deferred bios and use the saved block cgroup info to
>> submit the bios ? Is the method reasonable and is there a better way to
>> complete the throttling on the data device of the thin pool ?
> 
> I thought we had a patches where bio_clone_bioset() also retained
> cgroup information of original bio.
> 
> commit 20bd723ec6a3261df5e02250cd3a1fbb09a343f2
> Author: Paolo Valente <paolo.valente@linaro.org>
> Date:   Wed Jul 27 07:22:05 2016 +0200
> 
>     block: add missing group association in bio-cloning functions
> 
> Are you running new enough kernel. If not, may be there are still
> some places either in generic code or dm code where cloned/newly
> created bios are attributed to root cgroup and not to the cgroup
> of bio which caused creation of that new bio.
>
> Vivek
>
Hi, Vivek.

Thanks for your reply.

The version of the used linux kernel is 4.10-rc3.

bio_clone_blkcg_association() works only when the bi_css of source bio has
been initializaed before. In my test case, the bios submitted to dm-thin
device will not by throttled, so the bi_css is NULL.

Maybe we need assign the bi_css field of bio by usingbio_associate_current()
when the bi_css field is NULL and the target may defer the submit of the bio.
Something likes the following patch does:

diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index d1c05c1..97392a9 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -4177,6 +4177,7 @@ static int thin_ctr(struct dm_target *ti, unsigned argc, char **argv)
 static int thin_map(struct dm_target *ti, struct bio *bio)
 {
        bio->bi_iter.bi_sector = dm_target_offset(ti, bio->bi_iter.bi_sector);
+       dm_retain_bio_blkcg(bio);

        return thin_bio_map(ti, bio);
 }
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 3086da5..f129ca4 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1290,9 +1290,10 @@ static blk_qc_t dm_make_request(struct request_queue *q, struct bio *bio)
        if (unlikely(test_bit(DMF_BLOCK_IO_FOR_SUSPEND, &md->flags))) {
                dm_put_live_table(md, srcu_idx);

-               if (!(bio->bi_opf & REQ_RAHEAD))
+               if (!(bio->bi_opf & REQ_RAHEAD)) {
+                       dm_retain_bio_blkcg(bio);
                        queue_io(md, bio);
-               else
+               } else
                        bio_io_error(bio);
                return BLK_QC_T_NONE;
        }
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index f0aad08..e86c74f 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -214,4 +214,10 @@ void dm_free_md_mempools(struct dm_md_mempools *pools);
  */
 unsigned dm_get_reserved_bio_based_ios(void);

+static inline void dm_retain_bio_blkcg(struct bio *bio)
+{
+       if (!bio->bi_css)
+               bio_associate_current(bio);
+}
+
 #endif