* Re: [PATCH] drm/amdkfd: Put ACPI table after using it
@ 2020-07-23 5:19 ` Markus Elfring
0 siblings, 0 replies; 7+ messages in thread
From: Markus Elfring @ 2020-07-23 5:19 UTC (permalink / raw)
To: Hanjun Guo, amd-gfx, dri-devel
Cc: kernel-janitors, linux-kernel, Alex Deucher,
Christian König, Felix Kühling, Daniel Vetter,
David Airlie
…
> and in kfd_create_vcrat_image_cpu(), the acpi_table is only used to
> get the OEM info, so those table mappings need to be release after
…
1. Please avoid a typo for this change description.
2. An imperative wording can be preferred here, can't it?
3. Will the tag “Fixes” become helpful for the commit message?
Regards,
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdkfd: Put ACPI table after using it
@ 2020-07-23 5:19 ` Markus Elfring
0 siblings, 0 replies; 7+ messages in thread
From: Markus Elfring @ 2020-07-23 5:19 UTC (permalink / raw)
To: Hanjun Guo, amd-gfx, dri-devel
Cc: David Airlie, Felix Kühling, kernel-janitors, linux-kernel,
Daniel Vetter, Alex Deucher, Christian König
…
> and in kfd_create_vcrat_image_cpu(), the acpi_table is only used to
> get the OEM info, so those table mappings need to be release after
…
1. Please avoid a typo for this change description.
2. An imperative wording can be preferred here, can't it?
3. Will the tag “Fixes” become helpful for the commit message?
Regards,
Markus
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdkfd: Put ACPI table after using it
@ 2020-07-23 5:19 ` Markus Elfring
0 siblings, 0 replies; 7+ messages in thread
From: Markus Elfring @ 2020-07-23 5:19 UTC (permalink / raw)
To: Hanjun Guo, amd-gfx, dri-devel
Cc: David Airlie, Felix Kühling, kernel-janitors, linux-kernel,
Alex Deucher, Christian König
…
> and in kfd_create_vcrat_image_cpu(), the acpi_table is only used to
> get the OEM info, so those table mappings need to be release after
…
1. Please avoid a typo for this change description.
2. An imperative wording can be preferred here, can't it?
3. Will the tag “Fixes” become helpful for the commit message?
Regards,
Markus
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdkfd: Put ACPI table after using it
@ 2020-07-23 5:19 ` Markus Elfring
0 siblings, 0 replies; 7+ messages in thread
From: Markus Elfring @ 2020-07-23 5:19 UTC (permalink / raw)
To: Hanjun Guo, amd-gfx, dri-devel
Cc: David Airlie, Felix Kühling, kernel-janitors, linux-kernel,
Daniel Vetter, Alex Deucher, Christian König
…
> and in kfd_create_vcrat_image_cpu(), the acpi_table is only used to
> get the OEM info, so those table mappings need to be release after
…
1. Please avoid a typo for this change description.
2. An imperative wording can be preferred here, can't it?
3. Will the tag “Fixes” become helpful for the commit message?
Regards,
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] drm/amdkfd: Put ACPI table after using it
@ 2020-07-22 9:48 Hanjun Guo
2020-07-31 2:41 ` Felix Kuehling
0 siblings, 1 reply; 7+ messages in thread
From: Hanjun Guo @ 2020-07-22 9:48 UTC (permalink / raw)
To: Felix Kuehling, Alex Deucher, Christian König, David Airlie,
Daniel Vetter
Cc: linux-kernel, Hanjun Guo
The acpi_get_table() should be coupled with acpi_put_table() if
the mapped table is not used at runtime to release the table
mapping.
In kfd_create_crat_image_acpi(), crat_table is copied to pcrat_image,
and in kfd_create_vcrat_image_cpu(), the acpi_table is only used to
get the OEM info, so those table mappings need to be release after
using it.
Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index 1009a3b..d378e61 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -756,6 +756,7 @@ int kfd_create_crat_image_acpi(void **crat_image, size_t *size)
struct acpi_table_header *crat_table;
acpi_status status;
void *pcrat_image;
+ int rc = 0;
if (!crat_image)
return -EINVAL;
@@ -776,17 +777,21 @@ int kfd_create_crat_image_acpi(void **crat_image, size_t *size)
if (ignore_crat) {
pr_info("CRAT table disabled by module option\n");
- return -ENODATA;
+ rc = -ENODATA;
+ goto out;
}
pcrat_image = kmemdup(crat_table, crat_table->length, GFP_KERNEL);
- if (!pcrat_image)
- return -ENOMEM;
+ if (!pcrat_image) {
+ rc = -ENOMEM;
+ goto out;
+ }
*crat_image = pcrat_image;
*size = crat_table->length;
-
- return 0;
+out:
+ acpi_put_table(crat_table);
+ return rc;
}
/* Memory required to create Virtual CRAT.
@@ -970,6 +975,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, size_t *size)
CRAT_OEMID_LENGTH);
memcpy(crat_table->oem_table_id, acpi_table->oem_table_id,
CRAT_OEMTABLEID_LENGTH);
+ acpi_put_table(acpi_table);
}
crat_table->total_entries = 0;
crat_table->num_domains = 0;
--
1.7.12.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdkfd: Put ACPI table after using it
2020-07-22 9:48 Hanjun Guo
@ 2020-07-31 2:41 ` Felix Kuehling
2020-07-31 6:15 ` Hanjun Guo
0 siblings, 1 reply; 7+ messages in thread
From: Felix Kuehling @ 2020-07-31 2:41 UTC (permalink / raw)
To: Hanjun Guo, Alex Deucher, Christian König, David Airlie,
Daniel Vetter
Cc: linux-kernel
Hi Hanjun,
Sorry for the late reply.
Thank you for the patch and the explanation. This seems to have been
broken since the first version of KFD in 2014. See one suggestion inline.
Am 2020-07-22 um 5:48 a.m. schrieb Hanjun Guo:
> The acpi_get_table() should be coupled with acpi_put_table() if
> the mapped table is not used at runtime to release the table
> mapping.
>
> In kfd_create_crat_image_acpi(), crat_table is copied to pcrat_image,
> and in kfd_create_vcrat_image_cpu(), the acpi_table is only used to
> get the OEM info, so those table mappings need to be release after
> using it.
>
> Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> index 1009a3b..d378e61 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> @@ -756,6 +756,7 @@ int kfd_create_crat_image_acpi(void **crat_image, size_t *size)
> struct acpi_table_header *crat_table;
> acpi_status status;
> void *pcrat_image;
> + int rc = 0;
>
> if (!crat_image)
> return -EINVAL;
> @@ -776,17 +777,21 @@ int kfd_create_crat_image_acpi(void **crat_image, size_t *size)
>
> if (ignore_crat) {
> pr_info("CRAT table disabled by module option\n");
We should probably move this check to before we get the CRAT table.
There is not point getting and putting it if we're going to ignore it
anyway.
Do you want to send an updated patch with that change? Or maybe do it as
a 2-patch series?
Regards,
Felix
> - return -ENODATA;
> + rc = -ENODATA;
> + goto out;
> }
>
> pcrat_image = kmemdup(crat_table, crat_table->length, GFP_KERNEL);
> - if (!pcrat_image)
> - return -ENOMEM;
> + if (!pcrat_image) {
> + rc = -ENOMEM;
> + goto out;
> + }
>
> *crat_image = pcrat_image;
> *size = crat_table->length;
> -
> - return 0;
> +out:
> + acpi_put_table(crat_table);
> + return rc;
> }
>
> /* Memory required to create Virtual CRAT.
> @@ -970,6 +975,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, size_t *size)
> CRAT_OEMID_LENGTH);
> memcpy(crat_table->oem_table_id, acpi_table->oem_table_id,
> CRAT_OEMTABLEID_LENGTH);
> + acpi_put_table(acpi_table);
> }
> crat_table->total_entries = 0;
> crat_table->num_domains = 0;
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdkfd: Put ACPI table after using it
2020-07-31 2:41 ` Felix Kuehling
@ 2020-07-31 6:15 ` Hanjun Guo
0 siblings, 0 replies; 7+ messages in thread
From: Hanjun Guo @ 2020-07-31 6:15 UTC (permalink / raw)
To: Felix Kuehling, Alex Deucher, Christian König, David Airlie,
Daniel Vetter
Cc: linux-kernel
On 2020/7/31 10:41, Felix Kuehling wrote:
> Hi Hanjun,
>
> Sorry for the late reply.
>
> Thank you for the patch and the explanation. This seems to have been
> broken since the first version of KFD in 2014. See one suggestion inline.
>
> Am 2020-07-22 um 5:48 a.m. schrieb Hanjun Guo:
>> The acpi_get_table() should be coupled with acpi_put_table() if
>> the mapped table is not used at runtime to release the table
>> mapping.
>>
>> In kfd_create_crat_image_acpi(), crat_table is copied to pcrat_image,
>> and in kfd_create_vcrat_image_cpu(), the acpi_table is only used to
>> get the OEM info, so those table mappings need to be release after
>> using it.
>>
>> Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
>> ---
>> drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 16 +++++++++++-----
>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>> index 1009a3b..d378e61 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>> @@ -756,6 +756,7 @@ int kfd_create_crat_image_acpi(void **crat_image, size_t *size)
>> struct acpi_table_header *crat_table;
>> acpi_status status;
>> void *pcrat_image;
>> + int rc = 0;
>>
>> if (!crat_image)
>> return -EINVAL;
>> @@ -776,17 +777,21 @@ int kfd_create_crat_image_acpi(void **crat_image, size_t *size)
>>
>> if (ignore_crat) {
>> pr_info("CRAT table disabled by module option\n");
>
> We should probably move this check to before we get the CRAT table.
> There is not point getting and putting it if we're going to ignore it
> anyway.
>
> Do you want to send an updated patch with that change? Or maybe do it as
> a 2-patch series?
I will do it as 2-patch series and send a updated patch set.
Thanks
Hanjun
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-31 6:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23 5:19 [PATCH] drm/amdkfd: Put ACPI table after using it Markus Elfring
2020-07-23 5:19 ` Markus Elfring
2020-07-23 5:19 ` Markus Elfring
2020-07-23 5:19 ` Markus Elfring
-- strict thread matches above, loose matches on Subject: below --
2020-07-22 9:48 Hanjun Guo
2020-07-31 2:41 ` Felix Kuehling
2020-07-31 6:15 ` Hanjun Guo
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.