Am 21.12.2014 um 17:03 schrieb Oded Gabbay: > > > On 12/21/2014 05:57 PM, Christian König wrote: >>> There should be, but when the modules are compiled in, they are >>> loaded based on >>> link order only, if they are in the same group, and the groups are >>> loaded by a >>> pre-defined order. >> Is that really still up to date? I've seen effort to change that >> something like >> 10+ years ago when Rusty reworked the module system. And it is >> comming up on the >> lists again from time to time. > From what I can see in the Makefile rules, code and google, yes, > that's still the situation. If someone will prove me wrong I will be > more than happy to correct my code. >> >>> I don't want to move iommu before gpu, so I don't have a solution >>> for the >>> order between amdkfd and amd_iommu_v2. >> Why not? That's still better than creating a kernel workqueue, >> scheduling one >> work item on it, rescheduling the task until everything is completed >> and you can >> continue. > Because I don't know the consequences of moving an entire subsystem in > front of another one. In addition, even if everyone agrees, I'm pretty > sure that Linus won't be happy to do that in -rc stages. So maybe this > is something to consider for 3.20 merge window, but I would still like > to provide a solution for 3.19.
Yeah, true indeed. How about depending on everything being compiled as module for 3.19 then? Still better than having such a hack in the driver for as a temporary workaround for one release. Christian. > > Oded >> >> Christian. >> >> Am 21.12.2014 um 14:24 schrieb Oded Gabbay: >>> >>> >>> On 12/21/2014 03:06 PM, Oded Gabbay wrote: >>>> >>>> >>>> On 12/21/2014 02:19 PM, Christian König wrote: >>>>> Am 21.12.2014 um 12:34 schrieb Oded Gabbay: >>>>>> >>>>>> >>>>>> On 12/21/2014 01:27 PM, Christian König wrote: >>>>>>> Am 20.12.2014 um 21:46 schrieb Oded Gabbay: >>>>>>>> When amdkfd and radeon are compiled inside the kernel image >>>>>>>> (not as >>>>>>>> modules), >>>>>>>> radeon will load before amdkfd and will set *kfd2kgd to its >>>>>>>> interface >>>>>>>> structure. Therefore, we must not set *kfd2kgd to NULL when >>>>>>>> amdkfd is loaded >>>>>>>> because it will override radeon's initialization and cause >>>>>>>> kernel BUG. >>>>>>>> >>>>>>>> Signed-off-by: Oded Gabbay <oded.gabbay at amd.com> >>>>>>> >>>>>>> You should probably rather fix the dependency between the two >>>>>>> modules to >>>>>>> get an >>>>>>> determined load order instead of doing such nasty workarounds. >>>>>>> >>>>>>> Christian. >>>>>> >>>>>> The problem is that when modules are compiled inside the kernel, >>>>>> there is NO >>>>>> determined load order and there is no mechanism to enforce that. >>>>>> If there >>>>>> is/was such a mechanism, I would of course prefer to use it. >>>>> >>>>> There should be an determined order based on the symbol use, >>>>> otherwise >>>>> initializing most of the kernel modules wouldn't work as expected. >>>>> For example >>>>> radeon depends on the drm module must be loaded before radeon is >>>>> loaded. >>>> There should be, but when the modules are compiled in, they are >>>> loaded based on >>>> link order only, if they are in the same group, and the groups are >>>> loaded by a >>>> pre-defined order. >>>> The groups are: pure, core, postcore, arch, subsys, fs, device >>>> (which represents >>>> all the modules) and late. See init.h >>>> >>>> So radeon, amdkfd and amd_iommu_v2 are all in device group, and in >>>> the group >>>> they are ordered by their link order. >>>> >>>> Yes, radeon loads after drm, because drm*.o are before radeon*.o in >>>> the >>>> Makefile. See >>>> http://stackoverflow.com/questions/5669647/linux-order-of-statically-linked-module-loading >>>> >>>> >>>> >>>> >>> >>> So I tried moving amdkfd inside the Makefile before radeon, and that >>> made >>> amdkfd load before radeon did. This solves my first problem - order >>> between >>> amdkfd and radeon. However, amd_iommu_v2 doesn't belong to the drm >>> Makefile, >>> and I don't want to move iommu before gpu, so I don't have a >>> solution for the >>> order between amdkfd and amd_iommu_v2. >>> >>> Oded >>>> >>>> >>>>> >>>>>> >>>>>> Actually, I don't understand why the kernel doesn't enforce the >>>>>> order >>>>>> according to the use of exported symbols, like it does with modules. >>>>> >>>>> Yeah, that's indeed rather strange. There must be something in the >>>>> amdkfd code >>>>> which broke that somehow. >>>> IMO, that's a far-fetched guess. Could you point to something more >>>> specific ? >>>> >>>>> >>>>> As far as I understand you the desired init order is radeon and >>>>> amd_iommu_v2 >>>>> first and then amdkfd, right? >>>> Actually no. The preferred order is amd_iommu_v2, amdkfd and radeon >>>> last. This >>>> is the order that happens when all three are built as modules. More >>>> accurately, >>>> radeon inits, but its init triggers amdkfd init, which triggers >>>> amd_iommu_v2 >>>> init. So before radeon reaches its probe stage, all the modules were >>>> initialized. >>>> >>>> So what happens when you boot with radeon, >>>>> amd_iommu_v2 and amdkfd blacklisted for automatically load and >>>>> only load amdkfd >>>>> manually? >>>> As said above, that's ok. >>>>> >>>>>> There will always be dependencies between kgd (radeon) and amdkfd >>>>>> and between >>>>>> amdkfd and amd_iommu_v2. I don't think I can eliminate those >>>>>> dependencies, not >>>>>> without a very complex solution. And the fact that this complex >>>>>> solution >>>>>> occurs only in a very specific use case (all modules compiled >>>>>> in), makes me >>>>>> less inclined to do that. >>>>>> >>>>>> So I don't see it as a "nasty workaround". I would call it just a >>>>>> "workaround" >>>>>> for a specific use case, which should be solved by a generic >>>>>> solution to the >>>>>> kernel enforcing load orders. >>>>> >>>>> The normal kernel module handling already should provide the >>>>> correct init >>>>> order, >>>>> so I would still call this a rather nasty workaround because we >>>>> couldn't find >>>>> the underlying problem. >>>> Well, the normal kernel module handling doesn't work when all >>>> modules are >>>> compiled in. I'm not a huge expert on this issue so I had Joerg >>>> Roedel help me >>>> analyze this (thanks Joerg) and he agreed that there is no >>>> enforcement of order >>>> in this case. >>>> >>>>> >>>>> Christian. >>>>> >>>>>> >>>>>> Oded >>>>>>> >>>>>>>> --- >>>>>>>> drivers/gpu/drm/amd/amdkfd/kfd_module.c | 5 ++--- >>>>>>>> 1 file changed, 2 insertions(+), 3 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_module.c >>>>>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_module.c >>>>>>>> index 95d5af1..236562f 100644 >>>>>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_module.c >>>>>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_module.c >>>>>>>> @@ -34,7 +34,7 @@ >>>>>>>> #define KFD_DRIVER_MINOR 7 >>>>>>>> #define KFD_DRIVER_PATCHLEVEL 0 >>>>>>>> -const struct kfd2kgd_calls *kfd2kgd; >>>>>>>> +const struct kfd2kgd_calls *kfd2kgd = NULL; >>>>>>>> static const struct kgd2kfd_calls kgd2kfd = { >>>>>>>> .exit = kgd2kfd_exit, >>>>>>>> .probe = kgd2kfd_probe, >>>>>>>> @@ -84,14 +84,13 @@ EXPORT_SYMBOL(kgd2kfd_init); >>>>>>>> void kgd2kfd_exit(void) >>>>>>>> { >>>>>>>> + kfd2kgd = NULL; >>>>>>>> } >>>>>>>> static int __init kfd_module_init(void) >>>>>>>> { >>>>>>>> int err; >>>>>>>> - kfd2kgd = NULL; >>>>>>>> - >>>>>>>> /* Verify module parameters */ >>>>>>>> if ((sched_policy < KFD_SCHED_POLICY_HWS) || >>>>>>>> (sched_policy > KFD_SCHED_POLICY_NO_HWS)) { >>>>>>> >>>>> >>>> _______________________________________________ >>>> dri-devel mailing list >>>> dri-devel at lists.freedesktop.org >>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> -- >> To unsubscribe from this list: send the line "unsubscribe >> linux-kernel" in >> the body of a message to majordomo at vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/
