On 2/27/06, Rafael J. Wysocki <[email protected]> wrote:
> Make shrink_all_memory() overflow-resistant.
>
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> include/linux/swap.h | 2 +-
> mm/vmscan.c | 9 +++++----
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> Index: linux-2.6.16-rc4-mm2/mm/vmscan.c
> ===================================================================
> --- linux-2.6.16-rc4-mm2.orig/mm/vmscan.c
> +++ linux-2.6.16-rc4-mm2/mm/vmscan.c
> @@ -1785,18 +1785,19 @@ void wakeup_kswapd(struct zone *zone, in
> * Try to free `nr_pages' of memory, system-wide. Returns the number of freed
> * pages.
> */
> -int shrink_all_memory(unsigned long nr_pages)
> +unsigned long shrink_all_memory(unsigned int nr_pages)
What about the callers of shrink_all_memory() who currently expect it
to return an int, have you checked how they will react to you changing
the return type (and signedness) ?
> {
> pg_data_t *pgdat;
> - unsigned long nr_to_free = nr_pages;
> - int ret = 0;
> + long long nr_to_free = nr_pages;
'nr_pages' passed to the function is an unsigned int, why this change?
also, nr_to_free is later passed to balance_pgdat() as the second
argument and balance_pgdat() expects to be passed an int.
> + unsigned long ret = 0;
> struct reclaim_state reclaim_state = {
> .reclaimed_slab = 0,
> };
>
> current->reclaim_state = &reclaim_state;
> for_each_pgdat(pgdat) {
> - int freed;
> + unsigned long freed;
balance_pgdat() returns a plain int, so what's the point of making
'freed' an unsigned long?
> +
> freed = balance_pgdat(pgdat, nr_to_free, 0);
> ret += freed;
> nr_to_free -= freed;
> Index: linux-2.6.16-rc4-mm2/include/linux/swap.h
> ===================================================================
> --- linux-2.6.16-rc4-mm2.orig/include/linux/swap.h
> +++ linux-2.6.16-rc4-mm2/include/linux/swap.h
> @@ -173,7 +173,7 @@ extern void swap_setup(void);
>
> /* linux/mm/vmscan.c */
> extern unsigned long try_to_free_pages(struct zone **, gfp_t);
> -extern int shrink_all_memory(unsigned long nr_pages);
> +extern unsigned long shrink_all_memory(unsigned int nr_pages);
> extern int vm_swappiness;
>
> #ifdef CONFIG_NUMA
>
This patch may be correct or it may be wrong, I've not spend enough
time staring at it and follow the code it calls and gets called by to
say which, but I for one would like an explanation of why these
changes are made and why they are correct.
There's a distinct lack of a changelog/explanation with this patch.
Or maybe I'm just dense and can't see the obvious correctness, but if
that's the case I'd still like to be enlightened :)
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
