回复: Re: 回复: Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2
My plan is to obtain the process priority and convert it into the drm-scheduler's priority, so that the user is unaware and does not need to use environment variables to set it.
This is my patch:--- a/amdgpu/amdgpu_cs.c+++ b/amdgpu/amdgpu_cs.c@@ -31,6 +31,7 @@#if HAVE_ALLOCA_H# include #endif+#include #include "xf86drm.h"#include "amdgpu_drm.h"@@ -72,6 +73,14 @@ drm_public int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,}}+ int nice = getpriority(PRIO_PROCESS, 0);+ if (errno != EPERM)+ {+ if (nice > 0)+ priority = AMDGPU_CTX_PRIORITY_LOW;+ else if (nice < 0)+ priority = AMDGPU_CTX_PRIORITY_HIGH;+ }gpu_context = calloc(1, sizeof(struct amdgpu_context));if (!gpu_context)return -ENOMEM;----
主 题：Re: 回复: Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2 日 期：2024-01-09 17:36 发件人：maraeo 收件人：Christian König;
int p = -1.
unsigned u = p;
int p2 = u;
p2 is -1.
Marek
On Tue, Jan 9, 2024, 03:26 Christian König <christian.koe...@amd.com> wrote:
Am 09.01.24 um 09:09 schrieb 李真能:
Thanks!
What about the second patch?
The second patch: amdgpu: change proirity value to be consistent with kernel.
As I want to pass AMDGPU_CTX_PRIORITY_LOW to kernel module drm-scheduler, if these two patches are not applyed,
It can not pass LOW priority to drm-scheduler.
Do you have any other suggestion?
Well what exactly is the problem? Just use AMD_PRIORITY=-512.As far as I can see that is how it is supposed to be used.Regards,Christian.
主 题：Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2 日 期：2024-01-09 15:15 发件人：Christian König 收件人：李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;
Am 09.01.24 um 02:50 schrieb 李真能:
When the priority value is passed to the kernel, the kernel compares it with the following values:
#define AMDGPU_CTX_PRIORITY_VERY_LOW -1023#define AMDGPU_CTX_PRIORITY_LOW -512#define AMDGPU_CTX_PRIORITY_NORMAL 0#define AMDGPU_CTX_PRIORITY_HIGH 512#define AMDGPU_CTX_PRIORITY_VERY_HIGH 1023
If priority is uint32_t, we can't set LOW and VERY_LOW value to kernel context priority,
Well that's nonsense.How the kernel handles the values and how userspace handles them are two separate things. You just need to make sure that it's always 32 bits.In other words if you have signed or unsigned data type in userspace is irrelevant for the kernel.
You can refer to the kernel function amdgpu_ctx_priority_permit, if priority is greater
than 0, and this process has not CAP_SYS_NICE capibility or DRM_MASTER permission,
this process will be exited.
Correct, that's intentional.Regards,Christian.
主 题：Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2 日 期：2024-01-09 00:28 发件人：Christian König 收件人：李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;
Am 08.01.24 um 10:40 schrieb Zhenneng Li:> In order to pass the correct priority parameter to the kernel,> we must change priority type from uint32_t to int32_t.Hui what? Why should it matter if the parameter is signed or not?That doesn't seem to make sense.Regards,Christian.>> Signed-off-by: Zhenneng Li > ---> amdgpu/amdgpu.h | 2 +-> amdgpu/amdgpu_cs.c | 2 +-> 2 files changed, 2 insertions(+), 2 deletions(-)>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h> index 9bdbf366..f46753f3 100644> --- a/amdgpu/amdgpu.h> +++ b/amdgpu/amdgpu.h> @@ -896,7 +896,7 @@ int amdgpu_bo_list_update(amdgpu_bo_list_handle handle,> *> */> int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,> - uint32_t priority,> + int32_t priority,> amdgpu_context_handle *context);> /**> * Create GPU execution Context> diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c> index 49fc16c3..eb72c638 100644> --- a/amdgpu/amdgpu_cs.c> +++ b/amdgpu/amdgpu_cs.c> @@ -49,7 +49,7 @@ static int amdgpu_cs_reset_sem(amdgpu_semaphore_handle sem);> * \return 0 on success otherwise POSIX Error code> */> drm_public int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,> - uint32_t priority,> + int32_t priority,> amdgpu_context_handle *context)> {> struct amdgpu_context *gpu_context;
Re: 回复: Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2
int p = -1.
unsigned u = p;
int p2 = u;
p2 is -1.
Marek
On Tue, Jan 9, 2024, 03:26 Christian König wrote:
> Am 09.01.24 um 09:09 schrieb 李真能:
>
> Thanks!
>
> What about the second patch?
>
> The second patch: amdgpu: change proirity value to be consistent with
> kernel.
>
> As I want to pass AMDGPU_CTX_PRIORITY_LOW to kernel module drm-scheduler,
> if these two patches are not applyed,
>
> It can not pass LOW priority to drm-scheduler.
>
> Do you have any other suggestion?
>
>
> Well what exactly is the problem? Just use AMD_PRIORITY=-512.
>
> As far as I can see that is how it is supposed to be used.
>
> Regards,
> Christian.
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> *主 题：*Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of
> amdgpu_cs_ctx_create2
> *日 期：*2024-01-09 15:15
> *发件人：*Christian König
> *收件人：*李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;
>
> Am 09.01.24 um 02:50 schrieb 李真能:
>
> When the priority value is passed to the kernel, the kernel compares it
> with the following values:
>
> #define AMDGPU_CTX_PRIORITY_VERY_LOW-1023
> #define AMDGPU_CTX_PRIORITY_LOW -512
> #define AMDGPU_CTX_PRIORITY_NORMAL 0
> #define AMDGPU_CTX_PRIORITY_HIGH512
> #define AMDGPU_CTX_PRIORITY_VERY_HIGH 1023
>
> If priority is uint32_t, we can't set LOW and VERY_LOW value to kernel
> context priority,
>
> Well that's nonsense.
>
> How the kernel handles the values and how userspace handles them are two
> separate things. You just need to make sure that it's always 32 bits.
>
> In other words if you have signed or unsigned data type in userspace is
> irrelevant for the kernel.
>
> You can refer to the kernel function amdgpu_ctx_priority_permit, if
> priority is greater
>
> than 0, and this process has not CAP_SYS_NICE capibility or DRM_MASTER
> permission,
>
> this process will be exited.
>
> Correct, that's intentional.
>
> Regards,
> Christian.
>
>
>
>
>
>
>
>
>
>
>
>
> *主 题：*Re: [PATCH libdrm 1/2] amdgpu: fix parameter of
> amdgpu_cs_ctx_create2
> *日 期：*2024-01-09 00:28
> *发件人：*Christian König
> *收件人：*李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;
>
> Am 08.01.24 um 10:40 schrieb Zhenneng Li:
> > In order to pass the correct priority parameter to the kernel,
> > we must change priority type from uint32_t to int32_t.
>
> Hui what? Why should it matter if the parameter is signed or not?
>
> That doesn't seem to make sense.
>
> Regards,
> Christian.
>
> >
> > Signed-off-by: Zhenneng Li
> > ---
> > amdgpu/amdgpu.h | 2 +-
> > amdgpu/amdgpu_cs.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> > index 9bdbf366..f46753f3 100644
> > --- a/amdgpu/amdgpu.h
> > +++ b/amdgpu/amdgpu.h
> > @@ -896,7 +896,7 @@ int amdgpu_bo_list_update(amdgpu_bo_list_handle
> handle,
> > *
> > */
> > int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,
> > - uint32_t priority,
> > + int32_t priority,
> > amdgpu_context_handle *context);
> > /**
> > * Create GPU execution Context
> > diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
> > index 49fc16c3..eb72c638 100644
> > --- a/amdgpu/amdgpu_cs.c
> > +++ b/amdgpu/amdgpu_cs.c
> > @@ -49,7 +49,7 @@ static int amdgpu_cs_reset_sem(amdgpu_semaphore_handle
> sem);
> > * \return 0 on success otherwise POSIX Error code
> > */
> > drm_public int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,
> > - uint32_t priority,
> > + int32_t priority,
> > amdgpu_context_handle *context)
> > {
> > struct amdgpu_context *gpu_context;
>
>
>
回复: Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2
Thanks!
What about the second patch?
The second patch: amdgpu: change proirity value to be consistent with kernel.
As I want to pass AMDGPU_CTX_PRIORITY_LOW to kernel module drm-scheduler, if these two patches are not applyed,
It can not pass LOW priority to drm-scheduler.
Do you have any other suggestion?
主 题：Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2 日 期：2024-01-09 15:15 发件人：Christian König 收件人：李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;
Am 09.01.24 um 02:50 schrieb 李真能:
When the priority value is passed to the kernel, the kernel compares it with the following values:
#define AMDGPU_CTX_PRIORITY_VERY_LOW -1023#define AMDGPU_CTX_PRIORITY_LOW -512#define AMDGPU_CTX_PRIORITY_NORMAL 0#define AMDGPU_CTX_PRIORITY_HIGH 512#define AMDGPU_CTX_PRIORITY_VERY_HIGH 1023
If priority is uint32_t, we can't set LOW and VERY_LOW value to kernel context priority,
Well that's nonsense.How the kernel handles the values and how userspace handles them are two separate things. You just need to make sure that it's always 32 bits.In other words if you have signed or unsigned data type in userspace is irrelevant for the kernel.
You can refer to the kernel function amdgpu_ctx_priority_permit, if priority is greater
than 0, and this process has not CAP_SYS_NICE capibility or DRM_MASTER permission,
this process will be exited.
Correct, that's intentional.Regards,Christian.
主 题：Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2 日 期：2024-01-09 00:28 发件人：Christian König 收件人：李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;
Am 08.01.24 um 10:40 schrieb Zhenneng Li:> In order to pass the correct priority parameter to the kernel,> we must change priority type from uint32_t to int32_t.Hui what? Why should it matter if the parameter is signed or not?That doesn't seem to make sense.Regards,Christian.>> Signed-off-by: Zhenneng Li > ---> amdgpu/amdgpu.h | 2 +-> amdgpu/amdgpu_cs.c | 2 +-> 2 files changed, 2 insertions(+), 2 deletions(-)>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h> index 9bdbf366..f46753f3 100644> --- a/amdgpu/amdgpu.h> +++ b/amdgpu/amdgpu.h> @@ -896,7 +896,7 @@ int amdgpu_bo_list_update(amdgpu_bo_list_handle handle,> *> */> int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,> - uint32_t priority,> + int32_t priority,> amdgpu_context_handle *context);> /**> * Create GPU execution Context> diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c> index 49fc16c3..eb72c638 100644> --- a/amdgpu/amdgpu_cs.c> +++ b/amdgpu/amdgpu_cs.c> @@ -49,7 +49,7 @@ static int amdgpu_cs_reset_sem(amdgpu_semaphore_handle sem);> * \return 0 on success otherwise POSIX Error code> */> drm_public int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,> - uint32_t priority,> + int32_t priority,> amdgpu_context_handle *context)> {> struct amdgpu_context *gpu_context;
Re: 回复: Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of amdgpu_cs_ctx_create2
Am 09.01.24 um 09:09 schrieb 李真能:
Thanks!
What about the second patch?
The second patch: amdgpu: change proirity value to be consistent
with kernel.
As I want to pass AMDGPU_CTX_PRIORITY_LOW to kernel module
drm-scheduler, if these two patches are not applyed,
It can not pass LOW priority to drm-scheduler.
Do you have any other suggestion?
Well what exactly is the problem? Just use AMD_PRIORITY=-512.
As far as I can see that is how it is supposed to be used.
Regards,
Christian.
*主 题：*Re: 回复: Re: [PATCH libdrm 1/2] amdgpu: fix parameter of
amdgpu_cs_ctx_create2
*日 期：*2024-01-09 15:15
*发件人：*Christian König
*收件人：*李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;
Am 09.01.24 um 02:50 schrieb 李真能:
When the priority value is passed to the kernel, the kernel compares
it with the following values:
#define AMDGPU_CTX_PRIORITY_VERY_LOW -1023
#define AMDGPU_CTX_PRIORITY_LOW -512
#define AMDGPU_CTX_PRIORITY_NORMAL 0
#define AMDGPU_CTX_PRIORITY_HIGH 512
#define AMDGPU_CTX_PRIORITY_VERY_HIGH 1023
If priority is uint32_t, we can't set LOW and VERY_LOW value to kernel
context priority,
Well that's nonsense.
How the kernel handles the values and how userspace handles them are
two separate things. You just need to make sure that it's always 32 bits.
In other words if you have signed or unsigned data type in userspace
is irrelevant for the kernel.
You can refer to the kernel function amdgpu_ctx_priority_permit, if
priority is greater
than 0, and this process has not CAP_SYS_NICE capibility or
DRM_MASTER permission,
this process will be exited.
Correct, that's intentional.
Regards,
Christian.
*主 题：*Re: [PATCH libdrm 1/2] amdgpu: fix parameter of
amdgpu_cs_ctx_create2
*日 期：*2024-01-09 00:28
*发件人：*Christian König
*收件人：*李真能;Marek Olsak;Pierre-Eric Pelloux-Prayer;dri-devel;amd-gfx;
Am 08.01.24 um 10:40 schrieb Zhenneng Li:
> In order to pass the correct priority parameter to the kernel,
> we must change priority type from uint32_t to int32_t.
Hui what? Why should it matter if the parameter is signed or not?
That doesn't seem to make sense.
Regards,
Christian.
>
> Signed-off-by: Zhenneng Li
> ---
> amdgpu/amdgpu.h | 2 +-
> amdgpu/amdgpu_cs.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> index 9bdbf366..f46753f3 100644
> --- a/amdgpu/amdgpu.h
> +++ b/amdgpu/amdgpu.h
> @@ -896,7 +896,7 @@ int amdgpu_bo_list_update(amdgpu_bo_list_handle
handle,
> *
> */
> int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,
> - uint32_t priority,
> + int32_t priority,
> amdgpu_context_handle *context);
> /**
> * Create GPU execution Context
> diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
> index 49fc16c3..eb72c638 100644
> --- a/amdgpu/amdgpu_cs.c
> +++ b/amdgpu/amdgpu_cs.c
> @@ -49,7 +49,7 @@ static int
amdgpu_cs_reset_sem(amdgpu_semaphore_handle sem);
> * \return 0 on success otherwise POSIX Error code
> */
> drm_public int amdgpu_cs_ctx_create2(amdgpu_device_handle dev,
> - uint32_t priority,
> + int32_t priority,
> amdgpu_context_handle *context)
> {
> struct amdgpu_context *gpu_context;
