linux-kernel - Re:Re: [PATCH V3] amdgpu:optimization-- reduce no need mutex

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <AIsAfwDqCFqyDHqDpvcvoKov.3.1587523813368.Hmail.bernard@vivo.com>
Date:   Wed, 22 Apr 2020 10:50:13 +0800 (GMT+08:00)
From:   赵军奎 <bernard@...o.com>
To:     Felix Kuehling <felix.kuehling@....com>
Cc:     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>, amd-gfx@...ts.freedesktop.org,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        opensource.kernel@...o.com
Subject: Re:Re: [PATCH V3] amdgpu:optimization-- reduce no need mutex_lock area


Sure, this seems to be a lot more professional than my previous modification. 
My original intention is to make the code easier to read, and I learned a lot from
submitting these patches. Thank you very much for all your guidance!

Regards,
Bernard

发件人：Felix Kuehling <felix.kuehling@....com>
发送日期：2020-04-22 10:27:16
收件人：1587181464-114215-1-git-send-email-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>,amd-gfx@...ts.freedesktop.org,dri-devel@...ts.freedesktop.org,linux-kernel@...r.kernel.org
抄送人：opensource.kernel@...o.com,Bernard Zhao <bernard@...o.com>
主题：Re: [PATCH V3] amdgpu:optimization-- reduce no need mutex_lock area

Thanks again for the patch. I'm going
      to apply this with some minor fixes. The headline should start
      with "drm/amdgpu:".  I'll also change the wording of the headline
      and commit message:

drm/amdgpu: shrink critical section
        in amdgpu_amdkfd_gpuvm_free_memory_of_gpu

Reduce the mem->lock`s protected
        code area, no need to protect pr_debug.
      
This also simplifies error handling.

There is one more cosmetic change I'm
      going to make, see inline. I'll apply your patch with those
      updates if you're OK with that.

On 2020-04-21 2:48, Bernard Zhao wrote:

Maybe we could reduce the mutex_lock(&mem->lock)`s protected code area,
and no need to protect pr_debug.

Signed-off-by: Bernard Zhao <bernard@...o.com>

Changes since V1:
*commit message improve

Changes since V2:
*move comment along with the mutex_unlock

Link for V1:
*https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fpatchwork%2Fpatch%2F1226588%2F&amp;data=02%7C01%7CFelix.Kuehling%40amd.com%7Cc715f5d02aae40846b8f08d7e5c001fd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637230485152789682&amp;sdata=4UdZiWMAbW8eR1BS2%2F6qMvs5K6cHWy5c8I32ReQ4uz0%3D&amp;reserved=0
Link for V2:
*https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fpatchwork%2Fpatch%2F1227907%2F&amp;data=02%7C01%7CFelix.Kuehling%40amd.com%7Cc715f5d02aae40846b8f08d7e5c001fd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637230485152799673&amp;sdata=Wt%2Bk7k4MtSX4zIDgmLEOB6ZKzfuqAd6GJZ3Creqf1aY%3D&amp;reserved=0
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 327317c54f7c..f03d9843d723 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1285,21 +1285,21 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 	struct bo_vm_reservation_context ctx;
 	struct ttm_validate_buffer *bo_list_entry;
 	int ret;
+	unsigned int mapped_to_gpu_memory;
    
    
I'll move this before the "int ret;" line. It makes the code more
      readable if long declarations come before short ones.
    
Regards,

        Felix


 	mutex_lock(&mem->lock);
+	mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
+	mutex_unlock(&mem->lock);
+	/* lock is not needed after this, since mem is unused and will
+	 * be freed anyway
+	 */
 
-	if (mem->mapped_to_gpu_memory > 0) {
+	if (mapped_to_gpu_memory > 0) {
 		pr_debug("BO VA 0x%llx size 0x%lx is still mapped.\n",
 				mem->va, bo_size);
-		mutex_unlock(&mem->lock);
 		return -EBUSY;
 	}
 
-	mutex_unlock(&mem->lock);
-	/* lock is not needed after this, since mem is unused and will
-	 * be freed anyway
-	 */
-
 	/* No more MMU notifiers */
 	amdgpu_mn_unregister(mem->bo);