More specific libdrm error message

Submitted by Xie, AlexBin on Jan. 11, 2017, 9:49 p.m.

Details

Message ID CY4PR12MB16405027D18941DAF1D35A22F2660@CY4PR12MB1640.namprd12.prod.outlook.com
State New
Headers show
Series "More specific libdrm error message" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Xie, AlexBin Jan. 11, 2017, 9:49 p.m.
Hi,


v2: Use strerror instead of %m. %m is a GNU C Library extension.

Thanks,

Alex Bin Xie

Patch hide | download patch | download mbox

From 02fe2acc7a1aec34a9f1ae687fb6210a848b308e Mon Sep 17 00:00:00 2001
From: Alex Xie <AlexBin.Xie@amd.com>
Date: Wed, 11 Jan 2017 16:43:21 -0500
Subject: [PATCH libdrm] amdgpu: Provide more specific error message if
 non-privileged user runs amdgpu_test

Before this change, the error message is:" WARNING - Suite initialization failed..." People might think this is a driver problem.

v2: Use strerror instead of %m. %m is a GNU C Library extension.

Change-Id: Ib891c40ec812053f49ce5a99909455ac3137e32c
Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
---
 tests/amdgpu/basic_tests.c | 5 ++++-
 tests/amdgpu/bo_tests.c    | 6 +++++-
 tests/amdgpu/cs_tests.c    | 6 +++++-
 tests/amdgpu/vce_tests.c   | 6 +++++-
 4 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c
index 11f6a63..7a47274 100644
--- a/tests/amdgpu/basic_tests.c
+++ b/tests/amdgpu/basic_tests.c
@@ -206,8 +206,11 @@  int suite_basic_tests_init(void)
 
 	if (r == 0)
 		return CUE_SUCCESS;
-	else
+	else {
+		if ((r == -EACCES) && (errno == EACCES))
+			printf ("\n\nError:%s. Hint: Try to run this test program as root.", strerror(errno));
 		return CUE_SINIT_FAILED;
+	}
 }
 
 int suite_basic_tests_clean(void)
diff --git a/tests/amdgpu/bo_tests.c b/tests/amdgpu/bo_tests.c
index 993895d..37ee6d1 100644
--- a/tests/amdgpu/bo_tests.c
+++ b/tests/amdgpu/bo_tests.c
@@ -65,8 +65,12 @@  int suite_bo_tests_init(void)
 
 	r = amdgpu_device_initialize(drm_amdgpu[0], &major_version,
 				  &minor_version, &device_handle);
-	if (r)
+	if (r) {
+		if ((r == -EACCES) && (errno == EACCES))
+			printf ("\n\nError:%s. Hint: Try to run this test program as root.", strerror(errno));
+
 		return CUE_SINIT_FAILED;
+	}
 
 	req.alloc_size = BUFFER_SIZE;
 	req.phys_alignment = BUFFER_ALIGN;
diff --git a/tests/amdgpu/cs_tests.c b/tests/amdgpu/cs_tests.c
index a01ee48..ce723a9 100644
--- a/tests/amdgpu/cs_tests.c
+++ b/tests/amdgpu/cs_tests.c
@@ -76,8 +76,12 @@  int suite_cs_tests_init(void)
 
 	r = amdgpu_device_initialize(drm_amdgpu[0], &major_version,
 				     &minor_version, &device_handle);
-	if (r)
+	if (r) {
+		if ((r == -EACCES) && (errno == EACCES))
+			printf ("\n\nError:%s. Hint: Try to run this test program as root.", strerror(errno));
+
 		return CUE_SINIT_FAILED;
+	}
 
 	family_id = device_handle->info.family_id;
 	/* VI asic POLARIS10/11 have specific external_rev_id */
diff --git a/tests/amdgpu/vce_tests.c b/tests/amdgpu/vce_tests.c
index 4915170..2936ef5 100644
--- a/tests/amdgpu/vce_tests.c
+++ b/tests/amdgpu/vce_tests.c
@@ -94,8 +94,12 @@  int suite_vce_tests_init(void)
 
 	r = amdgpu_device_initialize(drm_amdgpu[0], &major_version,
 				     &minor_version, &device_handle);
-	if (r)
+	if (r) {
+		if ((r == -EACCES) && (errno == EACCES))
+			printf ("\n\nError:%s. Hint: Try to run this test program as root.", strerror(errno));
+
 		return CUE_SINIT_FAILED;
+	}
 
 	family_id = device_handle->info.family_id;
 	vce_harvest_config = device_handle->info.vce_harvest_config;
-- 
2.7.4


Comments

Hi Xie,

Perhaps you want to use `fprintf(stderr, "...")` over `printf("..")` and
lose the space before the start parenthesis. Also, line wrap
your commit message.

Side note, use git send-email so that the patch is inline and not a HTML
email for easy review and application of the patch.

Kind Regards,
Edward.

On 01/12/2017 08:49 AM, Xie, AlexBin wrote:
> Hi,
> 
> 
> v2: Use strerror instead of %m. %m is a GNU C Library extension.
> 
> Thanks,
> 
> Alex Bin Xie
> 
> 
> ------------------------------------------------------------------------
> *From:* Xie, AlexBin
> *Sent:* Wednesday, January 11, 2017 4:14 PM
> *To:* amd-gfx@lists.freedesktop.org
> *Subject:* More specific libdrm error message
>  
> Hi,
> 
> Provide more specific error message if non-privileged user runs amdgpu_test
> 
> Before this change, the error message is:" WARNING - Suite
> initialization failed..." People might think this is a driver problem.
> 
> 
> Tested with non-privileged user. Now the error message is like.
> 
> ...
> Error:Permission denied. Hint: Try to run this test program as root.
> WARNING - Suite initialization failed for 'Basic Tests'.
> 
> ...
> 
> 
> 
> Tested as root with no regression.
> 
> 
> 
> Please review.
> 
> 
> Thanks,
> 
> Alex Bin Xie
> 
> 
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
On 12 January 2017 at 01:29, Edward O'Callaghan
<funfunctor@folklore1984.net> wrote:
> Hi Xie,
>
> Perhaps you want to use `fprintf(stderr, "...")` over `printf("..")` and
> lose the space before the start parenthesis. Also, line wrap
> your commit message.
>
> Side note, use git send-email so that the patch is inline and not a HTML
> email for easy review and application of the patch.
>
Thanks Edward.

Xie, suggesting to run almost anything as root is a bad idea ;-)
Instead one could auth, in order to have the correct permissions.
Alternatively can use the renderD node all together.

Idea for future work:
Would be even better to use drmGetDevices2 to fetch all the devices
and use the correct node since card0/renderD128 is not guaranteed to
be a amdgpu one. Or maybe even run all the amdgpu devices through the
tests ?

Please correctly wrap commit messages and code.

Thanks
Emil
Hi Emil,


Thanks for the advice. I noticed that you have changed some tests using drmGetDevices2.

I mark this to my to do list.


Thanks,

Alex Bin Xie