On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe <jgg@xxxxxxxxxxxx> 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 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 <jgg@xxxxxxxxxxxx> Reviewed-by: Jérôme Glisse <jglisse@xxxxxxxxxx> --- 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); }/*
Since we are simplifying things, perhaps we should consider merging hmm_range_wait_until_valid() info hmm_range_register() and removing hmm_range_wait_until_valid() since the pattern is to always call the two together. In any case, this looks OK to me so you can add Reviewed-by: Ralph Campbell <rcampbell@xxxxxxxxxx>
