Re: [PATCH 06/33] mm: give the 'ret' variable a better name __do_page_cache_readahead

[Christoph Hellwig]

On Wed, May 09, 2018 at 08:45:01AM -0700, Matthew Wilcox wrote:
> On Wed, May 09, 2018 at 09:48:03AM +0200, Christoph Hellwig wrote:
> > It counts the number of pages acted on, so name it nr_pages to make that
> > obvious.
> > 
> > Signed-off-by: Christoph Hellwig 
> 
> Yes!
> 
> Also, it can't return an error, so how about changing it to unsigned int?
> And deleting the error check from the caller?

I'll take a look at that.

Re: [PATCH 06/33] mm: give the 'ret' variable a better name __do_page_cache_readahead

[Matthew Wilcox]

On Wed, May 09, 2018 at 09:48:03AM +0200, Christoph Hellwig wrote:
> It counts the number of pages acted on, so name it nr_pages to make that
> obvious.
> 
> Signed-off-by: Christoph Hellwig 

Yes!

Also, it can't return an error, so how about changing it to unsigned int?
And deleting the error check from the caller?

>  mm/readahead.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/readahead.c b/mm/readahead.c
> index 539bbb6c1fad..16d0cb1e2616 100644
> --- a/mm/readahead.c
> +++ b/mm/readahead.c
> @@ -156,7 +156,7 @@ int __do_page_cache_readahead(struct address_space 
> *mapping, struct file *filp,
>   unsigned long end_index;/* The last page we want to read */
>   LIST_HEAD(page_pool);
>   int page_idx;
> - int ret = 0;
> + int nr_pages = 0;
>   loff_t isize = i_size_read(inode);
>   gfp_t gfp_mask = readahead_gfp_mask(mapping);
>  
> @@ -187,7 +187,7 @@ int __do_page_cache_readahead(struct address_space 
> *mapping, struct file *filp,
>   list_add(>lru, _pool);
>   if (page_idx == nr_to_read - lookahead_size)
>   SetPageReadahead(page);
> - ret++;
> + nr_pages++;
>   }
>  
>   /*
> @@ -195,11 +195,11 @@ int __do_page_cache_readahead(struct address_space 
> *mapping, struct file *filp,
>* uptodate then the caller will launch readpage again, and
>* will then handle the error.
>*/
> - if (ret)
> - read_pages(mapping, filp, _pool, ret, gfp_mask);
> + if (nr_pages)
> + read_pages(mapping, filp, _pool, nr_pages, gfp_mask);
>   BUG_ON(!list_empty(_pool));
>  out:
> - return ret;
> + return nr_pages;
>  }
>  
>  /*
> -- 
> 2.17.0
>