* [PATCH] drm/amd/amdkfd: Fix large framesize for kfd_smi_ev_read()
@ 2020-05-20 13:53 Aurabindo Pillai
2020-05-20 18:21 ` Alex Deucher
2020-05-20 19:19 ` Felix Kuehling
0 siblings, 2 replies; 4+ messages in thread
From: Aurabindo Pillai @ 2020-05-20 13:53 UTC (permalink / raw)
To: Felix.Kuehling, alexander.deucher, christian.koenig
Cc: airlied, daniel, amd-gfx, dri-devel, linux-kernel, Amber Lin
The buffer allocated is of 1024 bytes. Allocate this from
heap instead of stack.
Also remove check for stack size since we're allocating from heap
Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
Tested-by: Amber Lin <Amber.Lin@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c | 26 +++++++++++++++------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
index f5fd18eacf0d..5aebe169f8c6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
@@ -77,9 +77,11 @@ static ssize_t kfd_smi_ev_read(struct file *filep, char __user *user,
int ret;
size_t to_copy;
struct kfd_smi_client *client = filep->private_data;
- unsigned char buf[MAX_KFIFO_SIZE];
+ unsigned char *buf;
- BUILD_BUG_ON(MAX_KFIFO_SIZE > 1024);
+ buf = kzalloc(MAX_KFIFO_SIZE * sizeof(*buf), GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
/* kfifo_to_user can sleep so we can't use spinlock protection around
* it. Instead, we kfifo out as spinlocked then copy them to the user.
@@ -88,19 +90,29 @@ static ssize_t kfd_smi_ev_read(struct file *filep, char __user *user,
to_copy = kfifo_len(&client->fifo);
if (!to_copy) {
spin_unlock(&client->lock);
- return -EAGAIN;
+ ret = -EAGAIN;
+ goto ret_err;
}
to_copy = min3(size, sizeof(buf), to_copy);
ret = kfifo_out(&client->fifo, buf, to_copy);
spin_unlock(&client->lock);
- if (ret <= 0)
- return -EAGAIN;
+ if (ret <= 0) {
+ ret = -EAGAIN;
+ goto ret_err;
+ }
ret = copy_to_user(user, buf, to_copy);
- if (ret)
- return -EFAULT;
+ if (ret) {
+ ret = -EFAULT;
+ goto ret_err;
+ }
+ kfree(buf);
return to_copy;
+
+ret_err:
+ kfree(buf);
+ return ret;
}
static ssize_t kfd_smi_ev_write(struct file *filep, const char __user *user,
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amd/amdkfd: Fix large framesize for kfd_smi_ev_read()
2020-05-20 13:53 [PATCH] drm/amd/amdkfd: Fix large framesize for kfd_smi_ev_read() Aurabindo Pillai
@ 2020-05-20 18:21 ` Alex Deucher
2020-05-20 19:19 ` Felix Kuehling
1 sibling, 0 replies; 4+ messages in thread
From: Alex Deucher @ 2020-05-20 18:21 UTC (permalink / raw)
To: Aurabindo Pillai
Cc: Kuehling, Felix, Deucher, Alexander, Christian Koenig,
Dave Airlie, LKML, Maling list - DRI developers, Amber Lin,
amd-gfx list, Daniel Vetter
On Wed, May 20, 2020 at 9:53 AM Aurabindo Pillai
<aurabindo.pillai@amd.com> wrote:
>
> The buffer allocated is of 1024 bytes. Allocate this from
> heap instead of stack.
>
> Also remove check for stack size since we're allocating from heap
>
> Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
> Tested-by: Amber Lin <Amber.Lin@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c | 26 +++++++++++++++------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> index f5fd18eacf0d..5aebe169f8c6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> @@ -77,9 +77,11 @@ static ssize_t kfd_smi_ev_read(struct file *filep, char __user *user,
> int ret;
> size_t to_copy;
> struct kfd_smi_client *client = filep->private_data;
> - unsigned char buf[MAX_KFIFO_SIZE];
> + unsigned char *buf;
>
> - BUILD_BUG_ON(MAX_KFIFO_SIZE > 1024);
> + buf = kzalloc(MAX_KFIFO_SIZE * sizeof(*buf), GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
>
> /* kfifo_to_user can sleep so we can't use spinlock protection around
> * it. Instead, we kfifo out as spinlocked then copy them to the user.
> @@ -88,19 +90,29 @@ static ssize_t kfd_smi_ev_read(struct file *filep, char __user *user,
> to_copy = kfifo_len(&client->fifo);
> if (!to_copy) {
> spin_unlock(&client->lock);
> - return -EAGAIN;
> + ret = -EAGAIN;
> + goto ret_err;
> }
> to_copy = min3(size, sizeof(buf), to_copy);
> ret = kfifo_out(&client->fifo, buf, to_copy);
> spin_unlock(&client->lock);
> - if (ret <= 0)
> - return -EAGAIN;
> + if (ret <= 0) {
> + ret = -EAGAIN;
> + goto ret_err;
> + }
>
> ret = copy_to_user(user, buf, to_copy);
> - if (ret)
> - return -EFAULT;
> + if (ret) {
> + ret = -EFAULT;
> + goto ret_err;
> + }
>
> + kfree(buf);
> return to_copy;
> +
> +ret_err:
> + kfree(buf);
> + return ret;
> }
>
> static ssize_t kfd_smi_ev_write(struct file *filep, const char __user *user,
> --
> 2.25.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amd/amdkfd: Fix large framesize for kfd_smi_ev_read()
2020-05-20 13:53 [PATCH] drm/amd/amdkfd: Fix large framesize for kfd_smi_ev_read() Aurabindo Pillai
2020-05-20 18:21 ` Alex Deucher
@ 2020-05-20 19:19 ` Felix Kuehling
2020-05-21 13:06 ` Aurabindo Pillai
1 sibling, 1 reply; 4+ messages in thread
From: Felix Kuehling @ 2020-05-20 19:19 UTC (permalink / raw)
To: Aurabindo Pillai, alexander.deucher, christian.koenig
Cc: airlied, daniel, amd-gfx, dri-devel, linux-kernel, Amber Lin
Am 2020-05-20 um 9:53 a.m. schrieb Aurabindo Pillai:
> The buffer allocated is of 1024 bytes. Allocate this from
> heap instead of stack.
>
> Also remove check for stack size since we're allocating from heap
>
> Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
> Tested-by: Amber Lin <Amber.Lin@amd.com>
See one comment inline. With that fixed, the patch is
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c | 26 +++++++++++++++------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> index f5fd18eacf0d..5aebe169f8c6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> @@ -77,9 +77,11 @@ static ssize_t kfd_smi_ev_read(struct file *filep, char __user *user,
> int ret;
> size_t to_copy;
> struct kfd_smi_client *client = filep->private_data;
> - unsigned char buf[MAX_KFIFO_SIZE];
> + unsigned char *buf;
>
> - BUILD_BUG_ON(MAX_KFIFO_SIZE > 1024);
> + buf = kzalloc(MAX_KFIFO_SIZE * sizeof(*buf), GFP_KERNEL);
kzalloc is not necessary here, you could use kmalloc. The part of that
allocation that matters will be overwritten by kfifo_out.
Regards,
Felix
> + if (!buf)
> + return -ENOMEM;
>
> /* kfifo_to_user can sleep so we can't use spinlock protection around
> * it. Instead, we kfifo out as spinlocked then copy them to the user.
> @@ -88,19 +90,29 @@ static ssize_t kfd_smi_ev_read(struct file *filep, char __user *user,
> to_copy = kfifo_len(&client->fifo);
> if (!to_copy) {
> spin_unlock(&client->lock);
> - return -EAGAIN;
> + ret = -EAGAIN;
> + goto ret_err;
> }
> to_copy = min3(size, sizeof(buf), to_copy);
> ret = kfifo_out(&client->fifo, buf, to_copy);
> spin_unlock(&client->lock);
> - if (ret <= 0)
> - return -EAGAIN;
> + if (ret <= 0) {
> + ret = -EAGAIN;
> + goto ret_err;
> + }
>
> ret = copy_to_user(user, buf, to_copy);
> - if (ret)
> - return -EFAULT;
> + if (ret) {
> + ret = -EFAULT;
> + goto ret_err;
> + }
>
> + kfree(buf);
> return to_copy;
> +
> +ret_err:
> + kfree(buf);
> + return ret;
> }
>
> static ssize_t kfd_smi_ev_write(struct file *filep, const char __user *user,
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amd/amdkfd: Fix large framesize for kfd_smi_ev_read()
2020-05-20 19:19 ` Felix Kuehling
@ 2020-05-21 13:06 ` Aurabindo Pillai
0 siblings, 0 replies; 4+ messages in thread
From: Aurabindo Pillai @ 2020-05-21 13:06 UTC (permalink / raw)
To: Felix Kuehling
Cc: alexander.deucher, christian.koenig, airlied, daniel, amd-gfx,
dri-devel, linux-kernel, Amber Lin
[-- Attachment #1: Type: text/plain, Size: 1575 bytes --]
On 05/20, Felix Kuehling wrote:
> Am 2020-05-20 um 9:53 a.m. schrieb Aurabindo Pillai:
> > The buffer allocated is of 1024 bytes. Allocate this from
> > heap instead of stack.
> >
> > Also remove check for stack size since we're allocating from heap
> >
> > Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
> > Tested-by: Amber Lin <Amber.Lin@amd.com>
>
> See one comment inline. With that fixed, the patch is
>
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
>
>
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c | 26 +++++++++++++++------
> > 1 file changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> > index f5fd18eacf0d..5aebe169f8c6 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> > @@ -77,9 +77,11 @@ static ssize_t kfd_smi_ev_read(struct file *filep, char __user *user,
> > int ret;
> > size_t to_copy;
> > struct kfd_smi_client *client = filep->private_data;
> > - unsigned char buf[MAX_KFIFO_SIZE];
> > + unsigned char *buf;
> >
> > - BUILD_BUG_ON(MAX_KFIFO_SIZE > 1024);
> > + buf = kzalloc(MAX_KFIFO_SIZE * sizeof(*buf), GFP_KERNEL);
>
> kzalloc is not necessary here, you could use kmalloc. The part of that
> allocation that matters will be overwritten by kfifo_out.
>
> Regards,
> Felix
>
>
Thank you Felix, Alex for the review. I shall make that change and submit it.
Thanks & Regards,
Aurabindo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-21 13:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20 13:53 [PATCH] drm/amd/amdkfd: Fix large framesize for kfd_smi_ev_read() Aurabindo Pillai
2020-05-20 18:21 ` Alex Deucher
2020-05-20 19:19 ` Felix Kuehling
2020-05-21 13:06 ` Aurabindo Pillai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).