On Tue, Jul 19, 2022 at 10:04:57PM +1000, Alistair Popple wrote:
>
> Dan Carpenter <dan.carpenter@xxxxxxxxxx> writes:
>
> > Hello Alistair Popple,
> >
> > The patch b05a79d4377f: "mm/gup: migrate device coherent pages when
> > pinning instead of failing" from Jul 15, 2022, leads to the following
> > Smatch static checker warning:
> >
> > mm/migrate_device.c:842 migrate_device_coherent_page()
> > warn: duplicate check 'src_pfn & (1 << 1)' (previous on line 832)
> >
> > mm/migrate_device.c
> > 810 int migrate_device_coherent_page(struct page *page)
> > 811 {
> > 812 unsigned long src_pfn, dst_pfn = 0;
> > 813 struct migrate_vma args;
> > 814 struct page *dpage;
> > 815
> > 816 WARN_ON_ONCE(PageCompound(page));
> > 817
> > 818 lock_page(page);
> > 819 src_pfn = migrate_pfn(page_to_pfn(page)) | MIGRATE_PFN_MIGRATE;
> > 820 args.src = &src_pfn;
> > 821 args.dst = &dst_pfn;
> > 822 args.cpages = 1;
> > 823 args.npages = 1;
> > 824 args.vma = NULL;
> > 825
> > 826 /*
> > 827 * We don't have a VMA and don't need to walk the page tables to find
> > 828 * the source page. So call migrate_vma_unmap() directly to unmap the
> > 829 * page as migrate_vma_setup() will fail if args.vma == NULL.
> > 830 */
> > 831 migrate_vma_unmap(&args);
> > 832 if (!(src_pfn & MIGRATE_PFN_MIGRATE))
> > 833 return -EBUSY;
> >
> > Return here.
> >
> > 834
> > 835 dpage = alloc_page(GFP_USER | __GFP_NOWARN);
> > 836 if (dpage) {
> > 837 lock_page(dpage);
> > 838 dst_pfn = migrate_pfn(page_to_pfn(dpage));
> > 839 }
> > 840
> > 841 migrate_vma_pages(&args);
> > --> 842 if (src_pfn & MIGRATE_PFN_MIGRATE)
> > 843 copy_highpage(dpage, page);
> >
> > No need to check again. Was dst_pfn intended?
>
> No. The check is necessary and correct. migrate_vma_pages() may clear
> MIGRATE_PFN_MIGRATE from src_pfn if the page could not be migrated for
> some reason.
Ah. Sorry. I missed that we save args.src = &src_pfn at the top. I'll
update the checker for this. It might take a week or two to get around
to it...
regards,
dan carpenter
