On Tue, Mar 24, 2020 at 08:33:39AM +0100, Christoph Hellwig wrote:
+/*
- If the valid flag is masked off, and default_flags doesn't set valid, then
- hmm_pte_need_fault() always returns 0.
- */
+static bool hmm_can_fault(struct hmm_range *range) +{
- return ((range->flags[HMM_PFN_VALID] & range->pfn_flags_mask) |
range->default_flags) &range->flags[HMM_PFN_VALID];+}
So my idea behind the helper was to turn this into something readable :)
Well, it does help to give the expression a name :)
E.g.
/*
- We only need to fault if either the default mask requires to fault all
- pages, or at least the mask allows for individual pages to be faulted.
*/ static bool hmm_can_fault(struct hmm_range *range) { return ((range->default_flags | range->pfn_flags_mask) & range->flags[HMM_PFN_VALID]); }
Okay, I find this as understandable and it is less cluttered. I think the comment is good enough now.
Can we concur on this then:
static unsigned int hmm_range_need_fault(const struct hmm_vma_walk *hmm_vma_walk, const uint64_t *pfns, unsigned long npages, uint64_t cpu_flags) { + struct hmm_range *range = hmm_vma_walk->range; unsigned int required_fault = 0; unsigned long i;
- if (hmm_vma_walk->flags & HMM_FAULT_SNAPSHOT) + /* + * If the default flags do not request to fault pages, and the mask does + * not allow for individual pages to be faulted, then + * hmm_pte_need_fault() will always return 0. + */ + if (!((range->default_flags | range->pfn_flags_mask) & + range->flags[HMM_PFN_VALID])) return 0;
I think everything else is sorted now, so if yes I'll send this as v3.
Thanks, Jason