Messages in this thread | | | Date | Fri, 28 Jul 2023 14:22:17 +0100 | From | Will Deacon <> | Subject | Re: [PATCH v5] perf: arm_cspmu: Separate Arm and vendor module |
| |
On Wed, Jul 05, 2023 at 05:47:45AM -0500, Besar Wicaksono wrote: > Arm Coresight PMU driver consists of main standard code and > vendor backend code. Both are currently built as a single module. > This patch adds vendor registration API to separate the two to > keep things modular. The main driver requests each known backend > module during initialization and defer device binding process. > The backend module then registers an init callback to the main > driver and continue the device driver binding process. > > Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com> > --- > > Changes from v4: > * Fix warning reported by kernel test robot > v4: https://lore.kernel.org/linux-arm-kernel/20230620041438.32514-1-bwicaksono@nvidia.com/T/#u
One minor comment below, but this mostly looks good to me. I'd like Suzuki's Ack before I queue it, though.
> + /* Load implementer module and initialize the callbacks. */ > + if (match) { > + mutex_lock(&arm_cspmu_lock); > + > + if (match->impl_init_ops) { > + if (try_module_get(match->module)) { > + cspmu->impl.match = match; > + ret = match->impl_init_ops(cspmu); > + module_put(match->module);
Why is it safe to drop the module reference here? If I'm understanding the flow correctly, ->impl_init_ops() will populate more function pointers in the cspmu->impl.ops structure, and we don't appear to take a module reference when calling those.
What happens if the backend module is unloaded while the core module is executed those functions?
Cheers,
Will
| |