Re: [PATCH v2] mm: fix long time stall from mm_populate

[Minchan Kim]

On Mon, Mar 02, 2020 at 05:22:03PM -0800, Matthew Wilcox wrote:
> On Mon, Mar 02, 2020 at 04:26:38PM -0800, Minchan Kim wrote:
> > @@ -1196,6 +1196,7 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors)
> >  	struct vm_area_struct *vma = NULL;
> >  	int locked = 0;
> >  	long ret = 0;
> > +	bool tried = false;
> 
> How about ...
> 
> 	int *lockedp = &locked;
> 
> >  
> >  	end = start + len;
> >  
> > @@ -1226,14 +1227,18 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors)
> >  		 * double checks the vma flags, so that it won't mlock pages
> >  		 * if the vma was already munlocked.
> >  		 */
> > -		ret = populate_vma_page_range(vma, nstart, nend, &locked);
> > +		ret = populate_vma_page_range(vma, nstart, nend,
> > +						tried ? NULL : &locked);
> 
> 		ret = populate_vma_page_range(vma, nstart, nend, lockedp);
> 
> >  		if (ret < 0) {
> >  			if (ignore_errors) {
> >  				ret = 0;
> >  				continue;	/* continue at next VMA */
> >  			}
> >  			break;
> > -		}
> > +		} else if (ret == 0)
> > +			tried = true;
> > +		else
> > +			tried = false;
> 
> 		} else if (ret == 0)
> 			lockedp = NULL;
> 
> Maybe there's a better name than lockedp.

Thanks for the review, Matthew.

It changes the behavior from mine in that it never set lockedp as "non-NULL"
since it had retried so that we lose fault retrying optimization for
successive addresses

I understand the code is not pretty there so that hard to follow it but
I am not sure the your suggestion makes it better.