Re: [PATCH] mm/vmstat: fix divide error at __fragmentation_index
From: Vlastimil Babka
Date: Thu Aug 03 2017 - 04:10:09 EST
Hi,
On 08/03/2017 09:59 AM, Wen Yang wrote:
> From: Jiang Biao <jiang.biao2@xxxxxxxxxx>
>
> When order is -1 or too big, *1UL << order* will be 0, which will
> cause divide error like this,
>
> divide error: 0000 [#1] SMP
> Call Trace:
> [<ffffffff81168423>] compaction_suitable+0x63/0xc0
> [<ffffffff81168a75>] compact_zone+0x35/0x950
> [<ffffffff811745b5>] ? free_percpu+0xb5/0x140
> [<ffffffff81092b23>] ? schedule_on_each_cpu+0x133/0x160
> [<ffffffff8116949c>] compact_node+0x10c/0x120
> [<ffffffff8116953c>] sysctl_compaction_handler+0x5c/0x90
> [<ffffffff811fa517>] proc_sys_call_handler+0x97/0xd0
> [<ffffffff811fa564>] proc_sys_write+0x14/0x20
> [<ffffffff81187368>] vfs_write+0xb8/0x1a0
> [<ffffffff81187c61>] sys_write+0x51/0x90
> [<ffffffff8100b052>] system_call_fastpath+0x16/0x1b
The trace seems to be from an old and non-mainline kernel, as it's the
same as you reported here:
https://bugzilla.kernel.org/show_bug.cgi?id=196555
In current mainline it seems to me that all callers of
__fragmentation_index() will only do so with a valid order.
I wouldn't mind making a non-hotpath code more robust, but probably in a
more obvious and self-reporting/documented way e.g. something like
if (WARN_ON_ONCE(order >= MAX_ORDER))
return 0;
> Signed-off-by: Wen Yang <wen.yang99@xxxxxxxxxx>
> Reviewed-by: Jiang Biao <jiang.biao2@xxxxxxxxxx>
> ---
> mm/vmstat.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 76f7367..2f9d012 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -870,6 +870,9 @@ static int __fragmentation_index(unsigned int order, struct contig_page_info *in
> {
> unsigned long requested = 1UL << order;
>
> + if (!requested)
> + return 0;
Seems the indentation is broken here (spaces vs tabs).
Thanks,
Vlastimil
> +
> if (!info->free_blocks_total)
> return 0;
>
>