blk-throttle: fix a typo in blk_throtl_init()

PATCH In review Namhyung Kim 2 participants 7 messages May 11, 2011

Subsystems (2)

Block layer cgroup blkio

lore

Share & tools

lore

Namhyung Kim

author

2011-05-11 15:58 UTC #0 lore raw

PATCH 1/1 blk-throttle: fix a typo in blk_throtl_init()

s/td/tg/

Signed-off-by Namhyung Kim <[email protected]>

Cc Vivek Goyal <[email protected]>

---
 block/blk-throttle.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

block/blk-throttle.c +1 -1

index 0475a22a420d..e7986b8a0085 100644
@@ -1072,7 +1072,7 @@ int blk_throtl_init(struct request_queue *q)
 	/* Practically unlimited BW */
 	tg->bps[0] = tg->bps[1] = -1;
 	tg->iops[0] = tg->iops[1] = -1;
-	td->limits_changed = false;
+	tg->limits_changed = false;
 
 	/*
 	 * Set root group reference to 2. One reference will be dropped when

Signature · 1 line

1.7.5

Vivek Goyal

2011-05-11 15:38 UTC #1 lore raw

Quoted 13 lines · Namhyung Kim

s/td/tg/
Signed-off-by Namhyung Kim <[email protected]>
Cc Vivek Goyal <[email protected]>
---
 block/blk-throttle.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
block/blk-throttle.c +0 -0
index 0475a22a420d..e7986b8a0085 100644

block/blk-throttle.c int blk_throtl_init(struct request_queue *q) · line 1076

Quoted 6 lines

block/blk-throttle.c +1 -1

@@ -1072,7 +1072,7 @@ int blk_throtl_init(struct request_queue *q)
 	/* Practically unlimited BW */
 	tg->bps[0] = tg->bps[1] = -1;
 	tg->iops[0] = tg->iops[1] = -1;
-	td->limits_changed = false;
+	tg->limits_changed = false;

Thanks Namhyung. I noticed it recently too. There is one more such typo in throtl_find_alloc_tg(). Can you fix that too and repost the patch.

Thanks
Vivek

Namhyung Kim

author

2011-05-11 16:16 UTC #2 lore raw

PATCH 1/1 blk-throttle: fix typos on struct throtl_grp init code

s/td/tg/

Signed-off-by Namhyung Kim <[email protected]>

Cc Vivek Goyal <[email protected]>

---
 block/blk-throttle.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

block/blk-throttle.c +2 -2

index 0475a22a420d..d1e317e0fc34 100644
@@ -201,7 +201,7 @@ static struct throtl_grp * throtl_find_alloc_tg(struct throtl_data *td,
 	RB_CLEAR_NODE(&tg->rb_node);
 	bio_list_init(&tg->bio_lists[0]);
 	bio_list_init(&tg->bio_lists[1]);
-	td->limits_changed = false;
+	tg->limits_changed = false;
 
 	/*
 	 * Take the initial reference that will be released on destroy

@@ -1072,7 +1072,7 @@ int blk_throtl_init(struct request_queue *q)
 	/* Practically unlimited BW */
 	tg->bps[0] = tg->bps[1] = -1;
 	tg->iops[0] = tg->iops[1] = -1;
-	td->limits_changed = false;
+	tg->limits_changed = false;
 
 	/*
 	 * Set root group reference to 2. One reference will be dropped when

Signature · 1 line

1.7.5

Vivek Goyal

Acked-by on v1 1/1

2011-05-11 16:17 UTC #3 lore raw

Re: [PATCH] blk-throttle: fix typos on struct throtl_grp init code

Quoted 1 line · Namhyung Kim

s/td/tg/

I think jens would like to have little more changelog than that.

Otherwise looks good to me.

Acked-by Vivek Goyal <[email protected]>

Vivek

Quoted 31 lines

Signed-off-by Namhyung Kim <[email protected]>

Cc Vivek Goyal <[email protected]>

---
 block/blk-throttle.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

block/blk-throttle.c +2 -2

index 0475a22a420d..d1e317e0fc34 100644
@@ -201,7 +201,7 @@ static struct throtl_grp * throtl_find_alloc_tg(struct throtl_data *td,
 	RB_CLEAR_NODE(&tg->rb_node);
 	bio_list_init(&tg->bio_lists[0]);
 	bio_list_init(&tg->bio_lists[1]);
-	td->limits_changed = false;
+	tg->limits_changed = false;
 
 	/*
 	 * Take the initial reference that will be released on destroy

@@ -1072,7 +1072,7 @@ int blk_throtl_init(struct request_queue *q)
 	/* Practically unlimited BW */
 	tg->bps[0] = tg->bps[1] = -1;
 	tg->iops[0] = tg->iops[1] = -1;
-	td->limits_changed = false;
+	tg->limits_changed = false;
 
 	/*
 	 * Set root group reference to 2. One reference will be dropped when

Signature · 1 line

1.7.5

Namhyung Kim

author

2011-05-11 16:03 UTC #4 lore raw

Re: [PATCH] blk-throttle: fix typos on struct throtl_grp init code

2011-05-11 (수), 09:30 -0400, Vivek Goyal:

Quoted 5 lines · Namhyung Kim

Quoted 1 line
s/td/tg/
I think jens would like to have little more changelog than that.

OK, let me clarify this first: My first patch was trivial but second one seems not. AFAICS it could affect when the bps/iops bandwidth change applies - maybe delayed to next @tg->disptime? - if there are concurrent cgroup init and limit change tasks, right? Or do you have something need to be included in the changelog?

Quoted 5 lines

Otherwise looks good to me.
Acked-by Vivek Goyal <[email protected]>
Vivek

Thank you for the review and the comment.

Signature · 2 lines

Regards,
Namhyung Kim

Vivek Goyal

2011-05-11 16:23 UTC #5 lore raw

Re: [PATCH] blk-throttle: fix typos on struct throtl_grp init code

Quoted 4 lines · Namhyung Kim

2011-05-11 (수), 09:30 -0400, Vivek Goyal:
Quoted 3 lines · Namhyung Kim
Quoted 1 line
s/td/tg/

Quoted 8 lines

Quoted 2 lines
I think jens would like to have little more changelog than that.
OK, let me clarify this first: My first patch was trivial but second one seems not. AFAICS it could affect when the bps/iops bandwidth change applies - maybe delayed to next @tg->disptime? - if there are concurrent cgroup init and limit change tasks, right? Or do you have something need to be included in the changelog?

Just say that when a new group is initialized, set tg->limits_changed = false instead of setting td->limits_changed to false.

We do memset 0 on all newly allocated objects to this might not be required at all.

So how about just getting rid of td->limits_changed assignments and not do explicit tg->limits_changed as that is implicit in zeroing of newly allocated object.

Thanks
Vivek

Quoted 15 lines

Quoted 7 lines
Otherwise looks good to me.
Acked-by Vivek Goyal <[email protected]>
Vivek
Thank you for the review and the comment.
Signature · 2 lines
Regards,
Namhyung Kim

Namhyung Kim

author

2011-05-11 15:38 UTC #6 lore raw

Re: [PATCH] blk-throttle: fix typos on struct throtl_grp init code

2011-05-11 (수), 10:28 -0400, Vivek Goyal:

Quoted 20 lines · Namhyung Kim

Quoted 4 lines
2011-05-11 (수), 09:30 -0400, Vivek Goyal:
Quoted 3 lines · Namhyung Kim
Quoted 1 line
s/td/tg/
Quoted 8 lines
Quoted 2 lines
I think jens would like to have little more changelog than that.
OK, let me clarify this first: My first patch was trivial but second one seems not. AFAICS it could affect when the bps/iops bandwidth change applies - maybe delayed to next @tg->disptime? - if there are concurrent cgroup init and limit change tasks, right? Or do you have something need to be included in the changelog?
Just say that when a new group is initialized, set tg->limits_changed = false instead of setting td->limits_changed to false.
We do memset 0 on all newly allocated objects to this might not be required at all.

Quoted 7 lines

So how about just getting rid of td->limits_changed assignments and not do explicit tg->limits_changed as that is implicit in zeroing of newly allocated object.
Thanks
Vivek

Sounds good to me. I'll cook a new patch :)

Thanks.

Signature · 2 lines

Regards,
Namhyung Kim