* Possible usage of uninitalized task_ratelimit variable in mm/page-writeback.c @ 2011-11-07 8:18 Witold Baryluk 2011-11-07 9:17 ` Wu Fengguang 0 siblings, 1 reply; 4+ messages in thread From: Witold Baryluk @ 2011-11-07 8:18 UTC (permalink / raw) To: Wu Fengguang, linux-mm Hi, I found a minor issue when compiling kernel today CC mm/page-writeback.o mm/page-writeback.c: In function a??balance_dirty_pages_ratelimited_nra??: include/trace/events/writeback.h:281:1: warning: a??task_ratelimita?? may be used uninitialized in this function [-Wuninitialized] mm/page-writeback.c:1018:16: note: a??task_ratelimita?? was declared here Indeed in balance_dirty_pages a task_ratelimit may be not initialized (initialization skiped by goto pause;), and then used when calling tracing hook. Regards, Witek -- Witold Baryluk JID: witold.baryluk // jabster.pl -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Possible usage of uninitalized task_ratelimit variable in mm/page-writeback.c 2011-11-07 8:18 Possible usage of uninitalized task_ratelimit variable in mm/page-writeback.c Witold Baryluk @ 2011-11-07 9:17 ` Wu Fengguang 2011-11-07 11:05 ` Witold Baryluk 0 siblings, 1 reply; 4+ messages in thread From: Wu Fengguang @ 2011-11-07 9:17 UTC (permalink / raw) To: Witold Baryluk; +Cc: linux-mm@kvack.org, linux-fsdevel On Mon, Nov 07, 2011 at 04:18:24PM +0800, Witold Baryluk wrote: > Hi, > > I found a minor issue when compiling kernel today > > > CC mm/page-writeback.o > mm/page-writeback.c: In function a??balance_dirty_pages_ratelimited_nra??: > include/trace/events/writeback.h:281:1: warning: a??task_ratelimita?? may be used uninitialized in this function [-Wuninitialized] > mm/page-writeback.c:1018:16: note: a??task_ratelimita?? was declared here > > Indeed in balance_dirty_pages a task_ratelimit may be not initialized > (initialization skiped by goto pause;), and then used when calling > tracing hook. Witold, thanks for the report! This patch should fix the bug. Thanks, Fengguang --- writeback: fix uninitialized task_ratelimit In balance_dirty_pages() task_ratelimit may be not initialized (initialization skiped by goto pause;), and then used when calling tracing hook. Fix it by moving the task_ratelimit assignment before goto pause. Reported-by: Witold Baryluk <baryluk@smp.if.uj.edu.pl> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com> --- mm/page-writeback.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) --- linux.orig/mm/page-writeback.c 2011-11-07 17:07:04.080000043 +0800 +++ linux/mm/page-writeback.c 2011-11-07 17:08:43.232000031 +0800 @@ -1097,13 +1097,13 @@ static void balance_dirty_pages(struct a pos_ratio = bdi_position_ratio(bdi, dirty_thresh, background_thresh, nr_dirty, bdi_thresh, bdi_dirty); - if (unlikely(pos_ratio == 0)) { + task_ratelimit = (u64)dirty_ratelimit * + pos_ratio >> RATELIMIT_CALC_SHIFT; + if (unlikely(task_ratelimit == 0)) { pause = max_pause; goto pause; } - task_ratelimit = (u64)dirty_ratelimit * - pos_ratio >> RATELIMIT_CALC_SHIFT; - pause = (HZ * pages_dirtied) / (task_ratelimit | 1); + pause = (HZ * pages_dirtied) / task_ratelimit; if (unlikely(pause <= 0)) { trace_balance_dirty_pages(bdi, dirty_thresh, -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Possible usage of uninitalized task_ratelimit variable in mm/page-writeback.c 2011-11-07 9:17 ` Wu Fengguang @ 2011-11-07 11:05 ` Witold Baryluk 2011-11-07 11:15 ` Wu Fengguang 0 siblings, 1 reply; 4+ messages in thread From: Witold Baryluk @ 2011-11-07 11:05 UTC (permalink / raw) To: Wu Fengguang; +Cc: linux-mm@kvack.org, linux-fsdevel On 11-07 17:17, Wu Fengguang wrote: > On Mon, Nov 07, 2011 at 04:18:24PM +0800, Witold Baryluk wrote: > > Hi, > > > > I found a minor issue when compiling kernel today > > > > > > CC mm/page-writeback.o > > mm/page-writeback.c: In function a??balance_dirty_pages_ratelimited_nra??: > > include/trace/events/writeback.h:281:1: warning: a??task_ratelimita?? may be used uninitialized in this function [-Wuninitialized] > > mm/page-writeback.c:1018:16: note: a??task_ratelimita?? was declared here > > > > Indeed in balance_dirty_pages a task_ratelimit may be not initialized > > (initialization skiped by goto pause;), and then used when calling > > tracing hook. > > Witold, thanks for the report! This patch should fix the bug. > > Thanks, > Fengguang > --- > > writeback: fix uninitialized task_ratelimit > > In balance_dirty_pages() task_ratelimit may be not initialized > (initialization skiped by goto pause;), and then used when calling > tracing hook. > > Fix it by moving the task_ratelimit assignment before goto pause. > > Reported-by: Witold Baryluk <baryluk@smp.if.uj.edu.pl> > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com> > --- > mm/page-writeback.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > --- linux.orig/mm/page-writeback.c 2011-11-07 17:07:04.080000043 +0800 > +++ linux/mm/page-writeback.c 2011-11-07 17:08:43.232000031 +0800 > @@ -1097,13 +1097,13 @@ static void balance_dirty_pages(struct a > pos_ratio = bdi_position_ratio(bdi, dirty_thresh, > background_thresh, nr_dirty, > bdi_thresh, bdi_dirty); > - if (unlikely(pos_ratio == 0)) { > + task_ratelimit = (u64)dirty_ratelimit * > + pos_ratio >> RATELIMIT_CALC_SHIFT; > + if (unlikely(task_ratelimit == 0)) { > pause = max_pause; > goto pause; > } > - task_ratelimit = (u64)dirty_ratelimit * > - pos_ratio >> RATELIMIT_CALC_SHIFT; > - pause = (HZ * pages_dirtied) / (task_ratelimit | 1); > + pause = (HZ * pages_dirtied) / task_ratelimit; > if (unlikely(pause <= 0)) { > trace_balance_dirty_pages(bdi, > dirty_thresh, Thanks. This is very nice patch, fixes warning, and simplifies logic. I have no other objections. (I do not remember what have bigger priority, * or >> - i guess * - but additional paranthesis can help. :D ) Regards, Witek -- Witold Baryluk -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Possible usage of uninitalized task_ratelimit variable in mm/page-writeback.c 2011-11-07 11:05 ` Witold Baryluk @ 2011-11-07 11:15 ` Wu Fengguang 0 siblings, 0 replies; 4+ messages in thread From: Wu Fengguang @ 2011-11-07 11:15 UTC (permalink / raw) To: Witold Baryluk; +Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org On Mon, Nov 07, 2011 at 07:05:58PM +0800, Witold Baryluk wrote: > On 11-07 17:17, Wu Fengguang wrote: > > On Mon, Nov 07, 2011 at 04:18:24PM +0800, Witold Baryluk wrote: > > > Hi, > > > > > > I found a minor issue when compiling kernel today > > > > > > > > > CC mm/page-writeback.o > > > mm/page-writeback.c: In function a??balance_dirty_pages_ratelimited_nra??: > > > include/trace/events/writeback.h:281:1: warning: a??task_ratelimita?? may be used uninitialized in this function [-Wuninitialized] > > > mm/page-writeback.c:1018:16: note: a??task_ratelimita?? was declared here > > > > > > Indeed in balance_dirty_pages a task_ratelimit may be not initialized > > > (initialization skiped by goto pause;), and then used when calling > > > tracing hook. > > > > Witold, thanks for the report! This patch should fix the bug. > > > > Thanks, > > Fengguang > > --- > > > > writeback: fix uninitialized task_ratelimit > > > > In balance_dirty_pages() task_ratelimit may be not initialized > > (initialization skiped by goto pause;), and then used when calling > > tracing hook. > > > > Fix it by moving the task_ratelimit assignment before goto pause. > > > > Reported-by: Witold Baryluk <baryluk@smp.if.uj.edu.pl> > > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com> > > --- > > mm/page-writeback.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > --- linux.orig/mm/page-writeback.c 2011-11-07 17:07:04.080000043 +0800 > > +++ linux/mm/page-writeback.c 2011-11-07 17:08:43.232000031 +0800 > > @@ -1097,13 +1097,13 @@ static void balance_dirty_pages(struct a > > pos_ratio = bdi_position_ratio(bdi, dirty_thresh, > > background_thresh, nr_dirty, > > bdi_thresh, bdi_dirty); > > - if (unlikely(pos_ratio == 0)) { > > + task_ratelimit = (u64)dirty_ratelimit * > > + pos_ratio >> RATELIMIT_CALC_SHIFT; > > + if (unlikely(task_ratelimit == 0)) { > > pause = max_pause; > > goto pause; > > } > > - task_ratelimit = (u64)dirty_ratelimit * > > - pos_ratio >> RATELIMIT_CALC_SHIFT; > > - pause = (HZ * pages_dirtied) / (task_ratelimit | 1); > > + pause = (HZ * pages_dirtied) / task_ratelimit; > > if (unlikely(pause <= 0)) { > > trace_balance_dirty_pages(bdi, > > dirty_thresh, > > Thanks. > > This is very nice patch, fixes warning, > and simplifies logic. I have no other objections. > > (I do not remember what have bigger priority, * or >> > - i guess * - but additional paranthesis can help. :D ) Good point! Just added/removed parentheses according to your suggestions :) Thanks, Fengguang --- writeback: fix uninitialized task_ratelimit In balance_dirty_pages() task_ratelimit may be not initialized (initialization skiped by goto pause;), and then used when calling tracing hook. Fix it by moving the task_ratelimit assignment before goto pause. Reported-by: Witold Baryluk <baryluk@smp.if.uj.edu.pl> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com> --- mm/page-writeback.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) --- linux.orig/mm/page-writeback.c 2011-11-07 19:11:39.660000042 +0800 +++ linux/mm/page-writeback.c 2011-11-07 19:12:19.856000041 +0800 @@ -1095,13 +1095,13 @@ static void balance_dirty_pages(struct a pos_ratio = bdi_position_ratio(bdi, dirty_thresh, background_thresh, nr_dirty, bdi_thresh, bdi_dirty); - if (unlikely(pos_ratio == 0)) { + task_ratelimit = ((u64)dirty_ratelimit * pos_ratio) >> + RATELIMIT_CALC_SHIFT; + if (unlikely(task_ratelimit == 0)) { pause = max_pause; goto pause; } - task_ratelimit = (u64)dirty_ratelimit * - pos_ratio >> RATELIMIT_CALC_SHIFT; - pause = (HZ * pages_dirtied) / (task_ratelimit | 1); + pause = HZ * pages_dirtied / task_ratelimit; if (unlikely(pause <= 0)) { trace_balance_dirty_pages(bdi, dirty_thresh, -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-11-07 11:15 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-07 8:18 Possible usage of uninitalized task_ratelimit variable in mm/page-writeback.c Witold Baryluk 2011-11-07 9:17 ` Wu Fengguang 2011-11-07 11:05 ` Witold Baryluk 2011-11-07 11:15 ` Wu Fengguang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).