Re: [PATCH 1/5] mm: migrate: Move the page count validation to the proper place

[Baolin Wang]

Hi Matthew,

> On Thu, Aug 05, 2021 at 11:05:56PM +0800, Baolin Wang wrote:
> > We've got the expected count for anonymous page or file page by
> > expected_page_refs() at the beginning of migrate_page_move_mapping(),
> > thus we should move the page count validation a little forward to
> > reduce duplicated code.
>
> Please add an explanation to the changelog for why it's safe to pull
> this out from under the i_pages lock.

Sure. In folio_migrate_mapping(), we are sure that the migration page 
was isolated from lru list and locked, so I think there are no race to 
get the page count without i_pages lock. Please correct me if I missed 
something else. Thanks.

> > Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
> > ---
> >   mm/migrate.c | 10 ++++------
> >   1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/mm/migrate.c b/mm/migrate.c
> > index 239b238..5559571 100644
> > --- a/mm/migrate.c
> > +++ b/mm/migrate.c
> > @@ -386,11 +386,10 @@ int folio_migrate_mapping(struct address_space *mapping,
> >   	int expected_count = expected_page_refs(mapping, &folio->page) + extra_count;
> >   	long nr = folio_nr_pages(folio);
> >   
> > -	if (!mapping) {
> > -		/* Anonymous page without mapping */
> > -		if (folio_ref_count(folio) != expected_count)
> > -			return -EAGAIN;
> > +	if (folio_ref_count(folio) != expected_count)
> > +		return -EAGAIN;
> >   
> > +	if (!mapping) {
> >   		/* No turning back from here */
> >   		newfolio->index = folio->index;
> >   		newfolio->mapping = folio->mapping;
> > @@ -404,8 +403,7 @@ int folio_migrate_mapping(struct address_space *mapping,
> >   	newzone = folio_zone(newfolio);
> >   
> >   	xas_lock_irq(&xas);
> > -	if (folio_ref_count(folio) != expected_count ||
> > -	    xas_load(&xas) != folio) {
> > +	if (xas_load(&xas) != folio) {
> >   		xas_unlock_irq(&xas);
> >   		return -EAGAIN;
> >   	}
> > --
> > 1.8.3.1