[0/6] Minor improvements for pagewalk code

Message

Rolf Eike Beer Aug. 22, 2022, 12:59 p.m. UTC

For some project I had to use the pagewalk API for certain things and during 
this have read through the code multiple times. Our usage has changed several 
times depending on our current state of research as well.

During all of this I have made some tweaks to the code to be able to follow it 
better when hunting my own problems, and not call into some things that I 
actually don't need. The patches are more or less independent of each other. 
Especially the last one may heavily depend on personal taste, so if you don't 
like it, just ignore it.

I would welcome if you could just pick those that you think are fitting and 
provide feedback on either of the remaining ones. At the end none of them 
should make any functional difference.

Regards,

Eike

Comments

Andrew Morton Aug. 22, 2022, 8:53 p.m. UTC | #1

On Mon, 22 Aug 2022 15:00:05 +0200 Rolf Eike Beer <[e-mail]> wrote:

> The err variable only needs to be checked when it was assigned directly
> before, it is not carried on to any later checks. Move the checks into the
> same "if" conditions where they are assigned. Also just return the error at
> the relevant places. While at it move these err variables to a more local
> scope at some places.
> 
> ...
>
> @@ -593,16 +608,15 @@ int walk_page_mapping(struct address_space *mapping, pgoff_t first_index,
>  		walk.mm = vma->vm_mm;
>  
>  		err = walk_page_test(vma->vm_start, vma->vm_end, &walk);
> -		if (err > 0) {
> -			err = 0;
> -			break;
> -		} else if (err < 0)
> -			break;
> +		if (err > 0)
> +			return 0;
> +		else if (err < 0)
> +			return err;
>  
>  		err = __walk_page_range(start_addr, end_addr, &walk);
>  		if (err)
> -			break;
> +			return err;
>  	}
>  
> -	return err;
> +	return 0;
>  }

I'm not really a fan of multiple return points - it tends to lead to
locking/resource leaks as the code evolves.  I don't really think it's
worth redoing the patch for this reason though; the rest looks good.