On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <j...@mellanox.com>
> 
> The wait_event_timeout macro already tests the condition as its first
> action, so there is no reason to open code another version of this, all
> that does is skip the might_sleep() debugging in common cases, which is
> not helpful.
> 
> Further, based on prior patches, we can no simplify the required condition

                                          "now simplify"

> test:
>  - If range is valid memory then so is range->hmm
>  - If hmm_release() has run then range->valid is set to false
>    at the same time as dead, so no reason to check both.
>  - A valid hmm has a valid hmm->mm.
> 
> Also, add the READ_ONCE for range->valid as there is no lock held here.
> 
> Signed-off-by: Jason Gunthorpe <j...@mellanox.com>
> Reviewed-by: Jérôme Glisse <jgli...@redhat.com>
> ---
>  include/linux/hmm.h | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 4ee3acabe5ed22..2ab35b40992b24 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -218,17 +218,9 @@ static inline unsigned long hmm_range_page_size(const 
> struct hmm_range *range)
>  static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
>                                             unsigned long timeout)
>  {
> -     /* Check if mm is dead ? */
> -     if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) {
> -             range->valid = false;
> -             return false;
> -     }
> -     if (range->valid)
> -             return true;
> -     wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead,
> +     wait_event_timeout(range->hmm->wq, range->valid,
>                          msecs_to_jiffies(timeout));
> -     /* Return current valid status just in case we get lucky */
> -     return range->valid;
> +     return READ_ONCE(range->valid);

Just to ensure that I actually understand the model: I'm assuming that the 
READ_ONCE is there solely to ensure that range->valid is read *after* the
wait_event_timeout() returns. Is that correct?


>  }
>  
>  /*
> 

In any case, it looks good, so:

    Reviewed-by: John Hubbard <jhubb...@nvidia.com>

thanks,
-- 
John Hubbard
NVIDIA
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to