From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?Q2hyaXN0aWFuIEvDtm5pZw==?= Subject: Re: [PATCH 1/3] amdkfd: Don't clear *kfd2kgd on kfd_module_init Date: Sun, 21 Dec 2014 13:19:44 +0100 Message-ID: <5496BAE0.5090901@vodafone.de> References: <1419108374-7020-1-git-send-email-oded.gabbay@amd.com> <1419108374-7020-2-git-send-email-oded.gabbay@amd.com> <5496AEAD.3090003@vodafone.de> <5496B04C.50502@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from pegasos-out.vodafone.de (pegasos-out.vodafone.de [80.84.1.38]) by gabe.freedesktop.org (Postfix) with ESMTP id 0F75C89C2A for ; Sun, 21 Dec 2014 04:20:20 -0800 (PST) In-Reply-To: <5496B04C.50502@amd.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Oded Gabbay , dri-devel@lists.freedesktop.org Cc: Alexander.Deucher@amd.com, linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org QW0gMjEuMTIuMjAxNCB1bSAxMjozNCBzY2hyaWViIE9kZWQgR2FiYmF5Ogo+Cj4KPiBPbiAxMi8y MS8yMDE0IDAxOjI3IFBNLCBDaHJpc3RpYW4gS8O2bmlnIHdyb3RlOgo+PiBBbSAyMC4xMi4yMDE0 IHVtIDIxOjQ2IHNjaHJpZWIgT2RlZCBHYWJiYXk6Cj4+PiBXaGVuIGFtZGtmZCBhbmQgcmFkZW9u IGFyZSBjb21waWxlZCBpbnNpZGUgdGhlIGtlcm5lbCBpbWFnZSAobm90IGFzIAo+Pj4gbW9kdWxl cyksCj4+PiByYWRlb24gd2lsbCBsb2FkIGJlZm9yZSBhbWRrZmQgYW5kIHdpbGwgc2V0ICprZmQy a2dkIHRvIGl0cyBpbnRlcmZhY2UKPj4+IHN0cnVjdHVyZS4gVGhlcmVmb3JlLCB3ZSBtdXN0IG5v dCBzZXQgKmtmZDJrZ2QgdG8gTlVMTCB3aGVuIGFtZGtmZCAKPj4+IGlzIGxvYWRlZAo+Pj4gYmVj YXVzZSBpdCB3aWxsIG92ZXJyaWRlIHJhZGVvbidzIGluaXRpYWxpemF0aW9uIGFuZCBjYXVzZSBr ZXJuZWwgQlVHLgo+Pj4KPj4+IFNpZ25lZC1vZmYtYnk6IE9kZWQgR2FiYmF5IDxvZGVkLmdhYmJh eUBhbWQuY29tPgo+Pgo+PiBZb3Ugc2hvdWxkIHByb2JhYmx5IHJhdGhlciBmaXggdGhlIGRlcGVu ZGVuY3kgYmV0d2VlbiB0aGUgdHdvIG1vZHVsZXMgCj4+IHRvIGdldCBhbgo+PiBkZXRlcm1pbmVk IGxvYWQgb3JkZXIgaW5zdGVhZCBvZiBkb2luZyBzdWNoIG5hc3R5IHdvcmthcm91bmRzLgo+Pgo+ PiBDaHJpc3RpYW4uCj4KPiBUaGUgcHJvYmxlbSBpcyB0aGF0IHdoZW4gbW9kdWxlcyBhcmUgY29t cGlsZWQgaW5zaWRlIHRoZSBrZXJuZWwsIHRoZXJlIAo+IGlzIE5PIGRldGVybWluZWQgbG9hZCBv cmRlciBhbmQgdGhlcmUgaXMgbm8gbWVjaGFuaXNtIHRvIGVuZm9yY2UgdGhhdC4gCj4gSWYgdGhl cmUgaXMvd2FzIHN1Y2ggYSBtZWNoYW5pc20sIEkgd291bGQgb2YgY291cnNlIHByZWZlciB0byB1 c2UgaXQuCgpUaGVyZSBzaG91bGQgYmUgYW4gZGV0ZXJtaW5lZCBvcmRlciBiYXNlZCBvbiB0aGUg c3ltYm9sIHVzZSwgb3RoZXJ3aXNlIAppbml0aWFsaXppbmcgbW9zdCBvZiB0aGUga2VybmVsIG1v ZHVsZXMgd291bGRuJ3Qgd29yayBhcyBleHBlY3RlZC4gRm9yIApleGFtcGxlIHJhZGVvbiBkZXBl bmRzIG9uIHRoZSBkcm0gbW9kdWxlIG11c3QgYmUgbG9hZGVkIGJlZm9yZSByYWRlb24gaXMgCmxv YWRlZC4KCj4KPiBBY3R1YWxseSwgSSBkb24ndCB1bmRlcnN0YW5kIHdoeSB0aGUga2VybmVsIGRv ZXNuJ3QgZW5mb3JjZSB0aGUgb3JkZXIgCj4gYWNjb3JkaW5nIHRvIHRoZSB1c2Ugb2YgZXhwb3J0 ZWQgc3ltYm9scywgbGlrZSBpdCBkb2VzIHdpdGggbW9kdWxlcy4KClllYWgsIHRoYXQncyBpbmRl ZWQgcmF0aGVyIHN0cmFuZ2UuIFRoZXJlIG11c3QgYmUgc29tZXRoaW5nIGluIHRoZSAKYW1ka2Zk IGNvZGUgd2hpY2ggYnJva2UgdGhhdCBzb21laG93LgoKQXMgZmFyIGFzIEkgdW5kZXJzdGFuZCB5 b3UgdGhlIGRlc2lyZWQgaW5pdCBvcmRlciBpcyByYWRlb24gYW5kIAphbWRfaW9tbXVfdjIgZmly c3QgYW5kIHRoZW4gYW1ka2ZkLCByaWdodD8gU28gd2hhdCBoYXBwZW5zIHdoZW4geW91IGJvb3Qg CndpdGggcmFkZW9uLCBhbWRfaW9tbXVfdjIgYW5kIGFtZGtmZCBibGFja2xpc3RlZCBmb3IgYXV0 b21hdGljYWxseSBsb2FkIAphbmQgb25seSBsb2FkIGFtZGtmZCBtYW51YWxseT8KCj4gVGhlcmUg d2lsbCBhbHdheXMgYmUgZGVwZW5kZW5jaWVzIGJldHdlZW4ga2dkIChyYWRlb24pIGFuZCBhbWRr ZmQgYW5kIAo+IGJldHdlZW4gYW1ka2ZkIGFuZCBhbWRfaW9tbXVfdjIuIEkgZG9uJ3QgdGhpbmsg SSBjYW4gZWxpbWluYXRlIHRob3NlIAo+IGRlcGVuZGVuY2llcywgbm90IHdpdGhvdXQgYSB2ZXJ5 IGNvbXBsZXggc29sdXRpb24uIEFuZCB0aGUgZmFjdCB0aGF0IAo+IHRoaXMgY29tcGxleCBzb2x1 dGlvbiBvY2N1cnMgb25seSBpbiBhIHZlcnkgc3BlY2lmaWMgdXNlIGNhc2UgKGFsbCAKPiBtb2R1 bGVzIGNvbXBpbGVkIGluKSwgbWFrZXMgbWUgbGVzcyBpbmNsaW5lZCB0byBkbyB0aGF0Lgo+Cj4g U28gSSBkb24ndCBzZWUgaXQgYXMgYSAibmFzdHkgd29ya2Fyb3VuZCIuIEkgd291bGQgY2FsbCBp dCBqdXN0IGEgCj4gIndvcmthcm91bmQiIGZvciBhIHNwZWNpZmljIHVzZSBjYXNlLCB3aGljaCBz aG91bGQgYmUgc29sdmVkIGJ5IGEgCj4gZ2VuZXJpYyBzb2x1dGlvbiB0byB0aGUga2VybmVsIGVu Zm9yY2luZyBsb2FkIG9yZGVycy4KClRoZSBub3JtYWwga2VybmVsIG1vZHVsZSBoYW5kbGluZyBh bHJlYWR5IHNob3VsZCBwcm92aWRlIHRoZSBjb3JyZWN0IAppbml0IG9yZGVyLCBzbyBJIHdvdWxk IHN0aWxsIGNhbGwgdGhpcyBhIHJhdGhlciBuYXN0eSB3b3JrYXJvdW5kIGJlY2F1c2UgCndlIGNv dWxkbid0IGZpbmQgdGhlIHVuZGVybHlpbmcgcHJvYmxlbS4KCkNocmlzdGlhbi4KCj4KPiAgICAg T2RlZAo+Pgo+Pj4gLS0tCj4+PiAgIGRyaXZlcnMvZ3B1L2RybS9hbWQvYW1ka2ZkL2tmZF9tb2R1 bGUuYyB8IDUgKystLS0KPj4+ICAgMSBmaWxlIGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygrKSwgMyBk ZWxldGlvbnMoLSkKPj4+Cj4+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2FtZC9hbWRr ZmQva2ZkX21vZHVsZS5jCj4+PiBiL2RyaXZlcnMvZ3B1L2RybS9hbWQvYW1ka2ZkL2tmZF9tb2R1 bGUuYwo+Pj4gaW5kZXggOTVkNWFmMS4uMjM2NTYyZiAxMDA2NDQKPj4+IC0tLSBhL2RyaXZlcnMv Z3B1L2RybS9hbWQvYW1ka2ZkL2tmZF9tb2R1bGUuYwo+Pj4gKysrIGIvZHJpdmVycy9ncHUvZHJt L2FtZC9hbWRrZmQva2ZkX21vZHVsZS5jCj4+PiBAQCAtMzQsNyArMzQsNyBAQAo+Pj4gICAjZGVm aW5lIEtGRF9EUklWRVJfTUlOT1IgICAgNwo+Pj4gICAjZGVmaW5lIEtGRF9EUklWRVJfUEFUQ0hM RVZFTCAgICAwCj4+PiAtY29uc3Qgc3RydWN0IGtmZDJrZ2RfY2FsbHMgKmtmZDJrZ2Q7Cj4+PiAr Y29uc3Qgc3RydWN0IGtmZDJrZ2RfY2FsbHMgKmtmZDJrZ2QgPSBOVUxMOwo+Pj4gICBzdGF0aWMg Y29uc3Qgc3RydWN0IGtnZDJrZmRfY2FsbHMga2dkMmtmZCA9IHsKPj4+ICAgICAgIC5leGl0ICAg ICAgICA9IGtnZDJrZmRfZXhpdCwKPj4+ICAgICAgIC5wcm9iZSAgICAgICAgPSBrZ2Qya2ZkX3By b2JlLAo+Pj4gQEAgLTg0LDE0ICs4NCwxMyBAQCBFWFBPUlRfU1lNQk9MKGtnZDJrZmRfaW5pdCk7 Cj4+PiAgIHZvaWQga2dkMmtmZF9leGl0KHZvaWQpCj4+PiAgIHsKPj4+ICsgICAga2ZkMmtnZCA9 IE5VTEw7Cj4+PiAgIH0KPj4+ICAgc3RhdGljIGludCBfX2luaXQga2ZkX21vZHVsZV9pbml0KHZv aWQpCj4+PiAgIHsKPj4+ICAgICAgIGludCBlcnI7Cj4+PiAtICAgIGtmZDJrZ2QgPSBOVUxMOwo+ Pj4gLQo+Pj4gICAgICAgLyogVmVyaWZ5IG1vZHVsZSBwYXJhbWV0ZXJzICovCj4+PiAgICAgICBp ZiAoKHNjaGVkX3BvbGljeSA8IEtGRF9TQ0hFRF9QT0xJQ1lfSFdTKSB8fAo+Pj4gICAgICAgICAg IChzY2hlZF9wb2xpY3kgPiBLRkRfU0NIRURfUE9MSUNZX05PX0hXUykpIHsKPj4KCl9fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5n IGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwOi8vbGlzdHMuZnJlZWRl c2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753540AbaLUMUW (ORCPT ); Sun, 21 Dec 2014 07:20:22 -0500 Received: from pegasos-out.vodafone.de ([80.84.1.38]:43182 "EHLO pegasos-out.vodafone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751298AbaLUMUV (ORCPT ); Sun, 21 Dec 2014 07:20:21 -0500 X-Spam-Flag: NO X-Spam-Score: 0.157 Authentication-Results: rohrpostix2.prod.vfnet.de (amavisd-new); dkim=softfail (invalid, public key: DNS query timeout for mail._domainkey.vodafone.de) header.i=@vodafone.de X-DKIM: OpenDKIM Filter v2.6.8 pegasos-out.vodafone.de 8BAC2670D2 Message-ID: <5496BAE0.5090901@vodafone.de> Date: Sun, 21 Dec 2014 13:19:44 +0100 From: =?UTF-8?B?Q2hyaXN0aWFuIEvDtm5pZw==?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Oded Gabbay , dri-devel@lists.freedesktop.org CC: linux-kernel@vger.kernel.org, Alexander.Deucher@amd.com Subject: Re: [PATCH 1/3] amdkfd: Don't clear *kfd2kgd on kfd_module_init References: <1419108374-7020-1-git-send-email-oded.gabbay@amd.com> <1419108374-7020-2-git-send-email-oded.gabbay@amd.com> <5496AEAD.3090003@vodafone.de> <5496B04C.50502@amd.com> In-Reply-To: <5496B04C.50502@amd.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> >> 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. > > 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. As far as I understand you the desired init order is radeon and amd_iommu_v2 first and then amdkfd, right? So what happens when you boot with radeon, amd_iommu_v2 and amdkfd blacklisted for automatically load and only load amdkfd manually? > 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. 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)) { >>