All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oded Gabbay <oded.gabbay@amd.com>
To: "Christian König" <deathsimple@vodafone.de>,
	"Dave Airlie" <airlied@gmail.com>
Cc: "Deucher, Alexander" <Alexander.Deucher@amd.com>,
	"Elifaz, Dana" <Dana.Elifaz@amd.com>,
	LKML <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 1/3] amdkfd: Don't clear *kfd2kgd on kfd_module_init
Date: Mon, 22 Dec 2014 12:22:18 +0200	[thread overview]
Message-ID: <5497F0DA.6050805@amd.com> (raw)
In-Reply-To: <5497E3D7.1060405@amd.com>



On 12/22/2014 11:26 AM, Oded Gabbay wrote:
>
>
> On 12/22/2014 10:57 AM, Christian König wrote:
>> Am 22.12.2014 um 08:43 schrieb Oded Gabbay:
>>>
>>>
>>> On 12/22/2014 09:40 AM, Dave Airlie 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.
>>>>>>
>>>>> I thought about it, but because this problem was originally reported by a
>>>>> user that told us he couldn't use modules because of his setup, I decided
>>>>> not to.
>>>>> I assume there are other users out there who needs this option (compiled
>>>>> everything in the kernel - embedded ?), so I don't want to make their life
>>>>> harder.
>>>>>
>>>>> In addition, saying it is a workaround for one release is true in case
>>>>> moving iommu subsystem in front of gpu subsystem is acceptable and doesn't
>>>>> cause other problems, unknown at this point.
>>>>>
>>>>> Bottom line, my personal preference is to help the users _now_ and if a
>>>>> better fix is found in the future, change the code accordingly.
>>>>
>>>> My guess is moving the iommu subsystem in front of the GPU would be rational.
>>>>
>>>> It does seem like it would generally have a depend in that order.
>>>>
>>>> Dave.
>>>>
>>> Dave,
>>> I agree, but don't you think it is too risky for -rc stages ?
>>> If not, I can try it and if it works on KV, I can submit a patch.
>>> But if you do think it is risky, what do you recommend for 3.19 ? Do the fix I
>>> suggested or disable build-in compilation option ?
>>
>> I would say create the patch of changing the order (should be trivial), describe
>> in detail in the commit message what this is supposed to fix and why such an
>> severe change was done in -rc1 and submit it upstream.
>>
>> We can still revert it in -rc2 if it breaks anything.
>>
>> Christian.
>>
>>>
>>>     Oded
>>
>
> OK, I'll try it on my machine and if it works, I will send the patch to the list.
>
>      Oded
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

So I checked it and all my HSA tests are passing on KV machine.
I will send the patches today. Please discard the current patch-set.

	Oded
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Oded Gabbay <oded.gabbay@amd.com>
To: "Christian König" <deathsimple@vodafone.de>,
	"Dave Airlie" <airlied@gmail.com>
Cc: "Deucher, Alexander" <Alexander.Deucher@amd.com>,
	"Elifaz, Dana" <Dana.Elifaz@amd.com>,
	LKML <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 1/3] amdkfd: Don't clear *kfd2kgd on kfd_module_init
Date: Mon, 22 Dec 2014 12:22:18 +0200	[thread overview]
Message-ID: <5497F0DA.6050805@amd.com> (raw)
In-Reply-To: <5497E3D7.1060405@amd.com>



On 12/22/2014 11:26 AM, Oded Gabbay wrote:
>
>
> On 12/22/2014 10:57 AM, Christian König wrote:
>> Am 22.12.2014 um 08:43 schrieb Oded Gabbay:
>>>
>>>
>>> On 12/22/2014 09:40 AM, Dave Airlie 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.
>>>>>>
>>>>> I thought about it, but because this problem was originally reported by a
>>>>> user that told us he couldn't use modules because of his setup, I decided
>>>>> not to.
>>>>> I assume there are other users out there who needs this option (compiled
>>>>> everything in the kernel - embedded ?), so I don't want to make their life
>>>>> harder.
>>>>>
>>>>> In addition, saying it is a workaround for one release is true in case
>>>>> moving iommu subsystem in front of gpu subsystem is acceptable and doesn't
>>>>> cause other problems, unknown at this point.
>>>>>
>>>>> Bottom line, my personal preference is to help the users _now_ and if a
>>>>> better fix is found in the future, change the code accordingly.
>>>>
>>>> My guess is moving the iommu subsystem in front of the GPU would be rational.
>>>>
>>>> It does seem like it would generally have a depend in that order.
>>>>
>>>> Dave.
>>>>
>>> Dave,
>>> I agree, but don't you think it is too risky for -rc stages ?
>>> If not, I can try it and if it works on KV, I can submit a patch.
>>> But if you do think it is risky, what do you recommend for 3.19 ? Do the fix I
>>> suggested or disable build-in compilation option ?
>>
>> I would say create the patch of changing the order (should be trivial), describe
>> in detail in the commit message what this is supposed to fix and why such an
>> severe change was done in -rc1 and submit it upstream.
>>
>> We can still revert it in -rc2 if it breaks anything.
>>
>> Christian.
>>
>>>
>>>     Oded
>>
>
> OK, I'll try it on my machine and if it works, I will send the patch to the list.
>
>      Oded
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

So I checked it and all my HSA tests are passing on KV machine.
I will send the patches today. Please discard the current patch-set.

	Oded

  reply	other threads:[~2014-12-22 10:22 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-20 20:46 [PATCH 0/3] Use workqueue for device init in amdkfd Oded Gabbay
2014-12-20 20:46 ` Oded Gabbay
2014-12-20 20:46 ` [PATCH 1/3] amdkfd: Don't clear *kfd2kgd on kfd_module_init Oded Gabbay
2014-12-20 20:46   ` Oded Gabbay
2014-12-20 21:25   ` Greg KH
2014-12-20 21:25     ` Greg KH
2014-12-21 11:12     ` Oded Gabbay
2014-12-21 11:12       ` Oded Gabbay
2014-12-21 11:27   ` Christian König
2014-12-21 11:27     ` Christian König
2014-12-21 11:34     ` Oded Gabbay
2014-12-21 11:34       ` Oded Gabbay
2014-12-21 12:19       ` Christian König
2014-12-21 12:19         ` Christian König
2014-12-21 13:06         ` Oded Gabbay
2014-12-21 13:06           ` Oded Gabbay
2014-12-21 13:24           ` Oded Gabbay
2014-12-21 13:24             ` Oded Gabbay
2014-12-21 15:57             ` Christian König
2014-12-21 15:57               ` Christian König
2014-12-21 16:03               ` Oded Gabbay
2014-12-21 16:03                 ` Oded Gabbay
2014-12-21 16:10                 ` Christian König
2014-12-21 16:10                   ` Christian König
2014-12-22  7:34                   ` Oded Gabbay
2014-12-22  7:34                     ` Oded Gabbay
2014-12-22  7:40                     ` Dave Airlie
2014-12-22  7:40                       ` Dave Airlie
2014-12-22  7:43                       ` Oded Gabbay
2014-12-22  7:43                         ` Oded Gabbay
2014-12-22  8:57                         ` Christian König
2014-12-22  8:57                           ` Christian König
2014-12-22  9:26                           ` Oded Gabbay
2014-12-22  9:26                             ` Oded Gabbay
2014-12-22 10:22                             ` Oded Gabbay [this message]
2014-12-22 10:22                               ` Oded Gabbay
2014-12-20 20:46 ` [PATCH 2/3] amdkfd: Track when amdkfd init is complete Oded Gabbay
2014-12-20 20:46   ` Oded Gabbay
2014-12-20 20:46 ` [PATCH 3/3] amdkfd: Use workqueue for GPU init Oded Gabbay
2014-12-20 20:46   ` Oded Gabbay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5497F0DA.6050805@amd.com \
    --to=oded.gabbay@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Dana.Elifaz@amd.com \
    --cc=airlied@gmail.com \
    --cc=deathsimple@vodafone.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.