On 06/15/2018 10:22 PM, Dan Williams wrote: > On Fri, Jun 15, 2018 at 9:43 PM, John Hubbard <jhubbard@xxxxxxxxxx> wrote: >> On 06/13/2018 12:51 PM, Dan Williams wrote: >>> [ adding Andrew, Christoph, and linux-mm ] >>> >>> On Wed, Jun 13, 2018 at 12:33 PM, Joe Gorse <jhgorse@xxxxxxxxx> wrote: [snip] >>>> >>>> P.S. The build failure, for the morbidly curious: >>>>> FATAL: modpost: GPL-incompatible module libafs.ko uses GPL-only symbol >>>>> '__put_devmap_managed_page' >>>>> scripts/Makefile.modpost:92: recipe for target '__modpost' failed >>>>> make[6]: *** [__modpost] Error 1 >>> >>> I think the right answer here is to make __put_devmap_managed_page() >>> EXPORT_SYMBOL(), since features like devm_memremap_pages() want to >>> change the behavior of all users of put_page(). It again holds that >>> devm_memremap_pages() needs to become EXPORT_SYMBOL_GPL() because it, >>> not put_page(), is the interface that is leaking control of core >>> kernel state/infrastructure to its users. >>> >> >> Hi Dan and all, >> >> It looks like put_page() also picks up one more GPL symbol: >> devmap_managed_key. >> >> put_page >> put_devmap_managed_page >> devmap_managed_key >> >> __put_devmap_managed_page >> >> >> So if the goal is to restore put_page() to be effectively EXPORT_SYMBOL >> again, then I think there would also need to be either a non-inlined >> wrapper for devmap_managed_key (awkward for a static key), or else make >> it EXPORT_SYMBOL, or maybe something else that's less obvious to me at the >> moment. > > Right, certainly flipping the key is a kernel internal detail since it > is giving the dev_pagemap owner purview over all kernel page events, > but put_page() users are silent consumers. And you're right there's > currently no good way I see to export the 'producer' and 'consumer' > side of the key with different export types. > It's hard to imagine how anyone could end up using devmap_managed_key in an out-of-tree driver, given that other related symbols are already locked down with EXPORT_SYMBOL_GPL. So one easy fix might be to just make it EXPORT_SYMBOL, with a comment that explains the intent, and just call it a day: /* This is intended to be EXPORT_SYMBOL_GPL, but actually doing so would * cause a problem for put_page(). */ EXPORT_SYMBOL(devmap_managed_key); ...I'm sort of winging it here... :)
