[02/13] amdgpu: don't initialize range->list in amdgpu_hmm_init_range

Submitted by Christoph Hellwig on July 30, 2019, 5:51 a.m.

Details

Message ID 20190730055203.28467-3-hch@lst.de
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Nouveau

Not browsing as part of any series.

Commit Message

Christoph Hellwig July 30, 2019, 5:51 a.m.
The list is used to add the range to another list as an entry in the
core hmm code, so there is no need to initialize it in a driver.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 1 -
 1 file changed, 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index b698b423b25d..60b9fc9561d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -484,6 +484,5 @@  void amdgpu_hmm_init_range(struct hmm_range *range)
 		range->flags = hmm_range_flags;
 		range->values = hmm_range_values;
 		range->pfn_shift = PAGE_SHIFT;
-		INIT_LIST_HEAD(&range->list);
 	}
 }

Comments

On Tue, Jul 30, 2019 at 08:51:52AM +0300, Christoph Hellwig wrote:
> The list is used to add the range to another list as an entry in the
> core hmm code, so there is no need to initialize it in a driver.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>

Jason
On 2019-07-30 1:51 a.m., Christoph Hellwig wrote:
> The list is used to add the range to another list as an entry in the

> core hmm code, so there is no need to initialize it in a driver.


I've seen code that uses list_empty to check whether a list head has 
been added to a list or not. For that to work, the list head needs to be 
initialized, and it has to be removed with list_del_init. If HMM doesn't 
ever do that with range->list, then this patch is Reviewed-by: Felix 
Kuehling <Felix.Kuehling@amd.com>


>

> Signed-off-by: Christoph Hellwig <hch@lst.de>

> ---

>   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 1 -

>   1 file changed, 1 deletion(-)

>

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c

> index b698b423b25d..60b9fc9561d7 100644

> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c

> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c

> @@ -484,6 +484,5 @@ void amdgpu_hmm_init_range(struct hmm_range *range)

>   		range->flags = hmm_range_flags;

>   		range->values = hmm_range_values;

>   		range->pfn_shift = PAGE_SHIFT;

> -		INIT_LIST_HEAD(&range->list);

>   	}

>   }
On Wed, Jul 31, 2019 at 01:25:06PM +0000, Kuehling, Felix wrote:
> On 2019-07-30 1:51 a.m., Christoph Hellwig wrote:
> > The list is used to add the range to another list as an entry in the
> > core hmm code, so there is no need to initialize it in a driver.
> 
> I've seen code that uses list_empty to check whether a list head has 
> been added to a list or not. For that to work, the list head needs to be 
> initialized, and it has to be removed with list_del_init. 

I think the ida is that 'list' is a private member of range and
drivers shouldn't touch it at all.

> ever do that with range->list, then this patch is Reviewed-by: Felix 
> Kuehling <Felix.Kuehling@amd.com>

Please put tags on their own empty line so that patchworks will
collect them automatically..

Jason