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 ‘balance_dirty_pages_ratelimited_nr’:
> > > include/trace/events/writeback.h:281:1: warning: ‘task_ratelimit’ may be used uninitialized in this function [-Wuninitialized]
> > > mm/page-writeback.c:1018:16: note: ‘task_ratelimit’ 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@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxx>
Signed-off-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
---
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 from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
