[PATCH] blk-throttle: fix a typo in blk_throtl_init()

[PATCH] blk-throttle: fix a typo in blk_throtl_init()

[Namhyung Kim]

s/td/tg/

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
---
 block/blk-throttle.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 0475a22a420d..e7986b8a0085 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -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
-- 
1.7.5

Re: [PATCH] blk-throttle: fix a typo in blk_throtl_init()

[Vivek Goyal]

On Wed, May 11, 2011 at 10:01:02PM +0900, Namhyung Kim wrote:
> s/td/tg/
> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> ---
>  block/blk-throttle.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
> index 0475a22a420d..e7986b8a0085 100644
> --- a/block/blk-throttle.c
> +++ b/block/blk-throttle.c
> @@ -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

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

[Namhyung Kim]

s/td/tg/

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
---
 block/blk-throttle.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 0475a22a420d..d1e317e0fc34 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -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
-- 
1.7.5

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

[Vivek Goyal]

On Wed, May 11, 2011 at 10:20:45PM +0900, Namhyung Kim wrote:
> s/td/tg/

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

Otherwise looks good to me.

Acked-by: Vivek Goyal <vgoyal@redhat.com>

Vivek

> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> ---
>  block/blk-throttle.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
> index 0475a22a420d..d1e317e0fc34 100644
> --- a/block/blk-throttle.c
> +++ b/block/blk-throttle.c
> @@ -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
> -- 
> 1.7.5

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

[Namhyung Kim]

2011-05-11 (수), 09:30 -0400, Vivek Goyal:
> On Wed, May 11, 2011 at 10:20:45PM +0900, Namhyung Kim wrote:
> > 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?


> Otherwise looks good to me.
> 
> Acked-by: Vivek Goyal <vgoyal@redhat.com>
> 
> Vivek

Thank you for the review and the comment.


-- 
Regards,
Namhyung Kim

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

[Vivek Goyal]

On Wed, May 11, 2011 at 11:14:02PM +0900, Namhyung Kim wrote:
> 2011-05-11 (수), 09:30 -0400, Vivek Goyal:
> > On Wed, May 11, 2011 at 10:20:45PM +0900, Namhyung Kim wrote:
> > > 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?

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


> 
> 
> > Otherwise looks good to me.
> > 
> > Acked-by: Vivek Goyal <vgoyal@redhat.com>
> > 
> > Vivek
> 
> Thank you for the review and the comment.
> 
> 
> -- 
> Regards,
> Namhyung Kim
> 

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

[Namhyung Kim]

2011-05-11 (수), 10:28 -0400, Vivek Goyal:
> On Wed, May 11, 2011 at 11:14:02PM +0900, Namhyung Kim wrote:
> > 2011-05-11 (수), 09:30 -0400, Vivek Goyal:
> > > On Wed, May 11, 2011 at 10:20:45PM +0900, Namhyung Kim wrote:
> > > > 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?
> 
> 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
> 

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

Thanks.


-- 
Regards,
Namhyung Kim