Re: [PATCH] mm/page-writeback.c: fix sparse warnings

[Johannes Weiner <hannes@cmpxchg.org>]

On Thu, Apr 16, 2009 at 09:26:35PM -0400, H Hartley Sweeten wrote:
> Fix two sparse warnings in mm/page-writeback.c.

You fixed code rather than warnings, right? ;)

> get_dirty_limits() calls clip_bdi_dirty_limit() and task_dirty_limit()
> with variable pbdi_dirty as one of the arguments. This variable is
> an unsigned long * but both functions expect it to be a long *. This
> causes the following sparse warnings:
> 
>   warning: incorrect type in argument 3 (different signedness)
>      expected long *pbdi_dirty
>      got unsigned long *pbdi_dirty
>   warning: incorrect type in argument 2 (different signedness)
>      expected long *pdirty
>      got unsigned long *pbdi_dirty
> 
> Fix the warnings by changing the long * to unsigned long * in both
> functions.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> 
> ---
> 
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 30351f0..e40b3e3 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -265,18 +265,20 @@ static void bdi_writeout_fraction(struct
> backing_dev_info *bdi,
>   * This avoids exceeding the total dirty_limit when the floating
> averages
>   * fluctuate too quickly.
>   */
> -static void
> -clip_bdi_dirty_limit(struct backing_dev_info *bdi, long dirty, long
> *pbdi_dirty)

Your patches wrap, please fix your mail client to not do this.

> +static void clip_bdi_dirty_limit(struct backing_dev_info *bdi,
> +		unsigned long dirty, unsigned long *pbdi_dirty)
>  {
> -	long avail_dirty;
> +	unsigned long avail_dirty;
> +	unsigned long nr_pages;
>  
> -	avail_dirty = dirty -
> -		(global_page_state(NR_FILE_DIRTY) +
> +	nr_pages = global_page_state(NR_FILE_DIRTY) +
>  		 global_page_state(NR_WRITEBACK) +
>  		 global_page_state(NR_UNSTABLE_NFS) +
> -		 global_page_state(NR_WRITEBACK_TEMP));
> +		 global_page_state(NR_WRITEBACK_TEMP);
>  
> -	if (avail_dirty < 0)
> +	if (nr_pages < dirty)
> +		avail_dirty = dirty - nr_pages;
> +	else
>  		avail_dirty = 0;

No need for a new variable, especially if nr_pages really counts
available dirty pages.

	avail_dirty = global_page_state() + ...
	if (avail_dirty < dirty)
		avail_dirty = dirty - avail_dirty;
	else
		avail_dirty = 0;

Hm?