linux-kernel - RE: [PATCH v2] drm/amd/amdgpu: cleanup coding style a bit

Open Source and information security mailing list archives

Message-ID: <14063C7AD467DE4B82DEDB5C278E8663010E20C5BD@FMSMSX108.amr.corp.intel.com>
Date:   Fri, 8 May 2020 15:45:07 +0000
From:   "Ruhl, Michael J" <michael.j.ruhl@...el.com>
To:     Bernard Zhao <bernard@...o.com>,
        Alex Deucher <alexander.deucher@....com>,
        Christian König <christian.koenig@....com>,
        "David (ChunMing) Zhou" <David1.Zhou@....com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Tom St Denis <tom.stdenis@....com>,
        "Sam Ravnborg" <sam@...nborg.org>,
        Ori Messinger <Ori.Messinger@....com>,
        "amd-gfx@...ts.freedesktop.org" <amd-gfx@...ts.freedesktop.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "opensource.kernel@...o.com" <opensource.kernel@...o.com>
Subject: RE: [PATCH v2] drm/amd/amdgpu: cleanup coding style a bit

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@...ts.freedesktop.org> On Behalf Of
>Bernard Zhao
>Sent: Thursday, May 7, 2020 5:13 AM
>To: Alex Deucher <alexander.deucher@....com>; Christian König
><christian.koenig@....com>; David (ChunMing) Zhou
><David1.Zhou@....com>; David Airlie <airlied@...ux.ie>; Daniel Vetter
><daniel@...ll.ch>; Tom St Denis <tom.stdenis@....com>; Sam Ravnborg
><sam@...nborg.org>; Ori Messinger <Ori.Messinger@....com>; Bernard
>Zhao <bernard@...o.com>; amd-gfx@...ts.freedesktop.org; dri-
>devel@...ts.freedesktop.org; linux-kernel@...r.kernel.org
>Cc: opensource.kernel@...o.com
>Subject: [PATCH v2] drm/amd/amdgpu: cleanup coding style a bit
>
>There is DEVICE_ATTR mechanism in separate attribute define.
>So this change is to use attr array, also use
>sysfs_create_files in init function & sysfs_remove_files in
>fini function.
>This maybe make the code a bit readable.
>
>Signed-off-by: Bernard Zhao <bernard@...o.com>
>
>Changes since V1:
>*Use DEVICE_ATTR mechanism
>
>Link for V1:
>*https://lore.kernel.org/patchwork/patch/1228076/
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 43 ++++++-------------
>-
> 1 file changed, 13 insertions(+), 30 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>index 82a3299e53c0..57bbc70662ff 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>@@ -148,6 +148,15 @@ static DEVICE_ATTR(mem_info_vis_vram_used,
>S_IRUGO,
> static DEVICE_ATTR(mem_info_vram_vendor, S_IRUGO,
> 		   amdgpu_mem_info_vram_vendor, NULL);
>
>+static struct attribute *amdgpu_vram_mgr_attributes[] = {
>+	&dev_attr_mem_info_vram_total.attr,
>+	&dev_attr_mem_info_vis_vram_total.attr,
>+	&dev_attr_mem_info_vram_used.attr,
>+	&dev_attr_mem_info_vis_vram_used.attr,
>+	&dev_attr_mem_info_vram_vendor.attr,
>+	NULL
>+};
>+
> /**
>  * amdgpu_vram_mgr_init - init VRAM manager and DRM MM
>  *
>@@ -172,31 +181,9 @@ static int amdgpu_vram_mgr_init(struct
>ttm_mem_type_manager *man,
> 	man->priv = mgr;
>
> 	/* Add the two VRAM-related sysfs files */
>-	ret = device_create_file(adev->dev,
>&dev_attr_mem_info_vram_total);
>-	if (ret) {
>-		DRM_ERROR("Failed to create device file
>mem_info_vram_total\n");
>-		return ret;
>-	}
>-	ret = device_create_file(adev->dev,
>&dev_attr_mem_info_vis_vram_total);
>-	if (ret) {
>-		DRM_ERROR("Failed to create device file
>mem_info_vis_vram_total\n");
>-		return ret;
>-	}
>-	ret = device_create_file(adev->dev,
>&dev_attr_mem_info_vram_used);
>-	if (ret) {
>-		DRM_ERROR("Failed to create device file
>mem_info_vram_used\n");
>-		return ret;
>-	}
>-	ret = device_create_file(adev->dev,
>&dev_attr_mem_info_vis_vram_used);
>-	if (ret) {
>-		DRM_ERROR("Failed to create device file
>mem_info_vis_vram_used\n");
>-		return ret;
>-	}
>-	ret = device_create_file(adev->dev,
>&dev_attr_mem_info_vram_vendor);
>-	if (ret) {
>-		DRM_ERROR("Failed to create device file
>mem_info_vram_vendor\n");
>-		return ret;
>-	}
>+	ret = sysfs_create_files(&adev->dev->kobj,
>amdgpu_vram_mgr_attributes);
>+	if (ret)
>+		DRM_ERROR("Failed to register sysfs\n");

This looks good to me.

I think that there is a new error macro (drm_err?) that you might
want to use instead of DRM_ERROR().

Otherwise:

Acked-by: Michael J. Ruhl <michael.j.ruhl@...el.com>

m

>
> 	return 0;
> }
>@@ -219,11 +206,7 @@ static int amdgpu_vram_mgr_fini(struct
>ttm_mem_type_manager *man)
> 	spin_unlock(&mgr->lock);
> 	kfree(mgr);
> 	man->priv = NULL;
>-	device_remove_file(adev->dev, &dev_attr_mem_info_vram_total);
>-	device_remove_file(adev->dev,
>&dev_attr_mem_info_vis_vram_total);
>-	device_remove_file(adev->dev, &dev_attr_mem_info_vram_used);
>-	device_remove_file(adev->dev,
>&dev_attr_mem_info_vis_vram_used);
>-	device_remove_file(adev->dev,
>&dev_attr_mem_info_vram_vendor);
>+	sysfs_remove_files(&adev->dev->kobj,
>amdgpu_vram_mgr_attributes);
> 	return 0;
> }
>
>--
>2.26.2
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@...ts.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives