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 )
Regards,
Witek
--
Witold Baryluk
--
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
