Re: [PATCH v11 04/25] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages

[John Hubbard]

On 12/18/19 10:52 PM, Dan Williams wrote:
> On Wed, Dec 18, 2019 at 9:51 PM John Hubbard <jhubbard@xxxxxxxxxx> wrote:
> > On 12/18/19 9:27 PM, Dan Williams wrote:
> > ...
> > > > @@ -461,5 +449,5 @@ void __put_devmap_managed_page(struct page *page)
> > > >           page->mapping = NULL;
> > > >           page->pgmap->ops->page_free(page);
> > > >    }
> > > > -EXPORT_SYMBOL(__put_devmap_managed_page);
> > > > +EXPORT_SYMBOL(free_devmap_managed_page);
> > >
> > > This patch does not have a module consumer for
> > > free_devmap_managed_page(), so the export should move to the patch
> > > that needs the new export.
> >
> > Hi Dan,
> >
> > OK, I know that's a policy--although it seems quite pointless here given
> > that this is definitely going to need an EXPORT.
> >
> > At the moment, the series doesn't use it in any module at all, so I'll just
> > delete the EXPORT for now.
> >
> > > Also the only reason that put_devmap_managed_page() is EXPORT_SYMBOL
> > > instead of EXPORT_SYMBOL_GPL is that there was no practical way to
> > > hide the devmap details from evey module in the kernel that did
> > > put_page(). I would expect free_devmap_managed_page() to
> > > EXPORT_SYMBOL_GPL if it is not inlined into an existing exported
> > > static inline api.
> >
> > Sure, I'll change it to EXPORT_SYMBOL_GPL when the time comes. We do have
> > to be careful that we don't shut out normal put_page() types of callers,
> > but...glancing through the current callers, that doesn't look to be a problem.
> > Good. So it should be OK to do EXPORT_SYMBOL_GPL here.
> >
> > Are you *sure* you don't want to just pre-emptively EXPORT now, and save
> > looking at it again?
>
> I'm positive. There is enough history for "trust me the consumer is
> coming" turning out not to be true to justify the hassle in my mind. I
> do trust you, but things happen.

OK, it's deleted locally. Thanks for looking at the patch. I'll post a v12 series
that includes the change, once it looks like reviews are slowing down.


thanks,
--
John Hubbard
NVIDIA