Re: [PATCH] mm/migrate_device: Try to handle swapcache pages.

[Mika Penttilä]

Hi,


On 8.5.2023 8.14, Huang, Ying wrote:
> Mika Penttilä <mpenttil@xxxxxxxxxx> writes:
>
> > H,
> >
> >
> > On 8.5.2023 5.26, Huang, Ying wrote:
> > > mpenttil@xxxxxxxxxx writes:
> > >
> > > > From: Mika Penttil <mpenttil@xxxxxxxxxx>
> > > >
> > > > Migrating file pages and swapcache pages into device memory is not supported.
> > > > The decision is done based on page_mapping(). For now, swapcache pages are not migrated.
> > > >
> > > > Things can however be improved, for swapcache pages. Try to get rid of the swap cache,
> > > > and if successful, go ahead as with other anonymous pages.
> > > >
> > > > As a detail, do not mess with shmem pages, as they handle swap internally.
> > > >
> > > > Cc: Alistair Popple <apopple@xxxxxxxxxx>
> > > > Cc: John Hubbard <jhubbard@xxxxxxxxxx>
> > > > Cc: Ralph Campbell <rcampbell@xxxxxxxxxx>
> > > > Signed-off-by: Mika Penttil <mpenttil@xxxxxxxxxx>
> > > > ---
> > > >    mm/migrate_device.c | 12 ++++++++++--
> > > >    1 file changed, 10 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> > > > index d30c9de60b0d..e8169c58c341 100644
> > > > --- a/mm/migrate_device.c
> > > > +++ b/mm/migrate_device.c
> > > > @@ -12,6 +12,7 @@
> > > >    #include <linux/mmu_notifier.h>
> > > >    #include <linux/oom.h>
> > > >    #include <linux/pagewalk.h>
> > > > +#include <linux/shmem_fs.h>
> > > >    #include <linux/rmap.h>
> > > >    #include <linux/swapops.h>
> > > >    #include <asm/tlbflush.h>
> > > > @@ -750,10 +751,17 @@ static void __migrate_device_pages(unsigned long *src_pfns,
> > > >    			/*
> > > >    			 * For now only support anonymous memory migrating to
> > > >    			 * device private or coherent memory.
> > > > +			 *
> > > > +			 * Try to get rid of swap cache if possible.
> > > > +			 * Leave shmem pages alone, they handle swap internally
> > > > +			 *
> > > >    			 */
> > > >    			if (mapping) {
> > > > -				src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
> > > > -				continue;
> > > > +				if (shmem_mapping(mapping) ||
> > > > +				    !folio_free_swap(page_folio(page))) {
> > > Should we filter out file pages firstly?
> >
> > folio_free_swap() checks first and returns false if not swapcache page
> > (ie including normal file pages) so think that is enough,
>
> You are right.  folio_free_swap() will call folio_test_swapcache(), and
> which will check folio_test_swapbacked().
>
> And, IIUC, we don't need to check shmem too.  Because folio_free_swap()
> will not return true for shmem pages.

Ah yes true, for not written out shmem page folio_free_swap() returns 
false (because !swapcache). And written out (but in swap cache) returns 
false because folio_swapped() == true

> > but maybe for clarity and not depending on it, could be good
> > explicitly check for swapcache before calling folio_free_swap().
>
> Yes.  That will be more clear.  I suggest to use folio_test_anon(), that
> will filter out shmem pages too.

Agreed.

> > > > +					src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
> > > > +					continue;
> > > > +				}
> > > >    			}
> > > >    		} else if (is_zone_device_page(newpage)) {
> > > >    			/*
>
> Best Regards,
> Huang, Ying

Thanks,
Mika