| Message ID | 20190607133107.GF14802@ziepe.ca (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Fri, Jun 07, 2019 at 10:31:07AM -0300, Jason Gunthorpe wrote: > 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 now 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. > > Allowing the return value of wait_event_timeout() (along with its internal > barriers) to compute the result of the function. > > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> Reviewed-by: Ira Weiny <ira.weiny@intel.com> > --- > v3 > - Simplify the wait_event_timeout to not check valid > --- > include/linux/hmm.h | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/include/linux/hmm.h b/include/linux/hmm.h > index 1d97b6d62c5bcf..26e7c477490c4e 100644 > --- a/include/linux/hmm.h > +++ b/include/linux/hmm.h > @@ -209,17 +209,8 @@ 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, > - msecs_to_jiffies(timeout)); > - /* Return current valid status just in case we get lucky */ > - return range->valid; > + return wait_event_timeout(range->hmm->wq, range->valid, > + msecs_to_jiffies(timeout)) != 0; > } > > /* > -- > 2.21.0 >
On 6/7/19 6:31 AM, Jason Gunthorpe wrote: > 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 now 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. > > Allowing the return value of wait_event_timeout() (along with its internal > barriers) to compute the result of the function. > > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> > --- Reviewed-by: John Hubbard <jhubbard@nvidia.com> thanks,
diff --git a/include/linux/hmm.h b/include/linux/hmm.h index 1d97b6d62c5bcf..26e7c477490c4e 100644 --- a/include/linux/hmm.h +++ b/include/linux/hmm.h @@ -209,17 +209,8 @@ 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, - msecs_to_jiffies(timeout)); - /* Return current valid status just in case we get lucky */ - return range->valid; + return wait_event_timeout(range->hmm->wq, range->valid, + msecs_to_jiffies(timeout)) != 0; } /*
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 now 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. Allowing the return value of wait_event_timeout() (along with its internal barriers) to compute the result of the function. Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> --- v3 - Simplify the wait_event_timeout to not check valid --- include/linux/hmm.h | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)