[PATCH v2 hmm 08/11] mm/hmm: Remove racy protection against double-unregistration

[John Hubbard]

On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg at mellanox.com>
> 
> No other register/unregister kernel API attempts to provide this kind of
> protection as it is inherently racy, so just drop it.
> 
> Callers should provide their own protection, it appears nouveau already
> does, but just in case drop a debugging POISON.
> 
> Signed-off-by: Jason Gunthorpe <jgg at mellanox.com>
> Reviewed-by: Jérôme Glisse <jglisse at redhat.com>
> ---
>  mm/hmm.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/hmm.c b/mm/hmm.c
> index c702cd72651b53..6802de7080d172 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -284,18 +284,13 @@ EXPORT_SYMBOL(hmm_mirror_register);
>   */
>  void hmm_mirror_unregister(struct hmm_mirror *mirror)
>  {
> -	struct hmm *hmm = READ_ONCE(mirror->hmm);
> -
> -	if (hmm == NULL)
> -		return;
> +	struct hmm *hmm = mirror->hmm;
>  
>  	down_write(&hmm->mirrors_sem);
>  	list_del_init(&mirror->list);
> -	/* To protect us against double unregister ... */
> -	mirror->hmm = NULL;
>  	up_write(&hmm->mirrors_sem);
> -
>  	hmm_put(hmm);
> +	memset(&mirror->hmm, POISON_INUSE, sizeof(mirror->hmm));

I hadn't thought of POISON_* for these types of cases, it's a 
good technique to remember.

I noticed that this is now done outside of the lock, but that
follows directly from your commit description, so that all looks 
correct.

>  }
>  EXPORT_SYMBOL(hmm_mirror_unregister);
>  
> 


    Reviewed-by: John Hubbard <jhubbard at nvidia.com>

thanks,
-- 
John Hubbard
NVIDIA