On Thursday 15 November 2012 05:34 PM, Arnd Bergmann wrote: > On Thursday 15 November 2012, Vineet Gupta wrote: >> On Monday 12 November 2012 07:25 PM, Arnd Bergmann wrote: >>> On Monday 12 November 2012, Vineet.Gupta1@xxxxxxxxxxxx wrote: >>>> +EXPORT_SYMBOL(take_snap); >>>> + >>>> ... >>>> +EXPORT_SYMBOL(take_snap2); >>> Where are these functions called? >> These are called from various parts of ARCH code, such as before >> handling signal or TLB flush etc. >> >>> Shouldn't this all just be part of the perf module? >> These are for low level ARCH specific event snapshotting. Maybe >> perf/ftrace already have some of these in the generic code which >> eventually call the ARCH APIs. Our current perf support is just husk of >> an implementation. Once we have the full perf / ftrace support this >> module - except for the even t capture part could just go away. >> OTOH, I've not seen much usage of this from loadable modules - so if you >> deem correct, I can even remove the export. >> >>> If not, can you make the exports GPL-only? >> Am I right in understanding that this is more related to discouraging >> non GPL modules "in general" than having to do with port itself. > Mostly yes. I understand that there are some reasons why people want > to mark symbols as generally available, e.g. for standard interfaces > that have traditionally been available to every module. My rule is > usually that any newly introduced symbols should be EXPORT_SYMBOL_GPL > by default, unless there is a (documented) reason to use EXPORT_SYMBOL > instead. This is particularly true for low-level interfaces like the > one here. > > If you can just remove the export, that is probably the best solution. Will do ! > On a related note, any global symbol (exported or not) should normally > have a prefix that identifies the subsystem it belongs to. A global > identifier like "take_snap" can potentially conflict with symbols in > other parts of the kernel. Agreed - this contraption anyways was a quick hack to debug the "rooky" issues we had initially - things are pretty stable now - so I'm planning to drop this patch entirely. When we do something like LTTng etc we'll revisit the low level instrumentation. Thx, -Vineet -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
