From: Felix Kuehling <felix.kuehling@amd.com> To: 1587181464-114215-1-git-send-email-bernard@vivo.com, "Alex Deucher" <alexander.deucher@amd.com>, "Christian König" <christian.koenig@amd.com>, "David (ChunMing) Zhou" <David1.Zhou@amd.com>, "David Airlie" <airlied@linux.ie>, "Daniel Vetter" <daniel@ffwll.ch>, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Cc: opensource.kernel@vivo.com, Bernard Zhao <bernard@vivo.com> Subject: Re: [PATCH V3] amdgpu:optimization-- reduce no need mutex_lock area Date: Tue, 21 Apr 2020 22:27:16 -0400 [thread overview] Message-ID: <08ef8975-43d2-049b-1c80-de33af7ebd4a@amd.com> (raw) In-Reply-To: <20200421064818.129158-1-bernard@vivo.com> [-- Attachment #1.1: Type: text/plain, Size: 2965 bytes --] 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@vivo.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&data=02%7C01%7CFelix.Kuehling%40amd.com%7Cc715f5d02aae40846b8f08d7e5c001fd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637230485152789682&sdata=4UdZiWMAbW8eR1BS2%2F6qMvs5K6cHWy5c8I32ReQ4uz0%3D&reserved=0 > Link for V2: > *https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fpatchwork%2Fpatch%2F1227907%2F&data=02%7C01%7CFelix.Kuehling%40amd.com%7Cc715f5d02aae40846b8f08d7e5c001fd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637230485152799673&sdata=Wt%2Bk7k4MtSX4zIDgmLEOB6ZKzfuqAd6GJZ3Creqf1aY%3D&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); > [-- Attachment #1.2: Type: text/html, Size: 4972 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Felix Kuehling <felix.kuehling@amd.com> To: 1587181464-114215-1-git-send-email-bernard@vivo.com, "Alex Deucher" <alexander.deucher@amd.com>, "Christian König" <christian.koenig@amd.com>, "David (ChunMing) Zhou" <David1.Zhou@amd.com>, "David Airlie" <airlied@linux.ie>, "Daniel Vetter" <daniel@ffwll.ch>, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Cc: opensource.kernel@vivo.com, Bernard Zhao <bernard@vivo.com> Subject: Re: [PATCH V3] amdgpu:optimization-- reduce no need mutex_lock area Date: Tue, 21 Apr 2020 22:27:16 -0400 [thread overview] Message-ID: <08ef8975-43d2-049b-1c80-de33af7ebd4a@amd.com> (raw) In-Reply-To: <20200421064818.129158-1-bernard@vivo.com> [-- Attachment #1.1: Type: text/plain, Size: 2965 bytes --] 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@vivo.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&data=02%7C01%7CFelix.Kuehling%40amd.com%7Cc715f5d02aae40846b8f08d7e5c001fd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637230485152789682&sdata=4UdZiWMAbW8eR1BS2%2F6qMvs5K6cHWy5c8I32ReQ4uz0%3D&reserved=0 > Link for V2: > *https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fpatchwork%2Fpatch%2F1227907%2F&data=02%7C01%7CFelix.Kuehling%40amd.com%7Cc715f5d02aae40846b8f08d7e5c001fd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637230485152799673&sdata=Wt%2Bk7k4MtSX4zIDgmLEOB6ZKzfuqAd6GJZ3Creqf1aY%3D&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); > [-- Attachment #1.2: Type: text/html, Size: 4972 bytes --] [-- Attachment #2: Type: text/plain, Size: 154 bytes --] _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2020-04-22 2:27 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-21 6:48 [PATCH V3] amdgpu:optimization-- reduce no need mutex_lock area Bernard Zhao 2020-04-21 6:48 ` Bernard Zhao 2020-04-21 6:48 ` Bernard Zhao 2020-04-22 2:27 ` Felix Kuehling [this message] 2020-04-22 2:27 ` Felix Kuehling 2020-04-22 2:50 ` 赵军奎 2020-04-22 2:50 ` 赵军奎 2020-04-22 2:50 ` 赵军奎
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=08ef8975-43d2-049b-1c80-de33af7ebd4a@amd.com \ --to=felix.kuehling@amd.com \ --cc=1587181464-114215-1-git-send-email-bernard@vivo.com \ --cc=David1.Zhou@amd.com \ --cc=airlied@linux.ie \ --cc=alexander.deucher@amd.com \ --cc=amd-gfx@lists.freedesktop.org \ --cc=bernard@vivo.com \ --cc=christian.koenig@amd.com \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-kernel@vger.kernel.org \ --cc=opensource.kernel@vivo.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.