On Mon, Jul 06, 2020 at 09:29:04PM -0700, Sean Christopherson wrote: > > > > + idx_start = PFN_DOWN(start); > > > > + idx_end = PFN_DOWN(end - 1); > > > > + > > > > + for (idx = idx_start; idx <= idx_end; ++idx) { > > > > + mutex_lock(&encl->lock); > > > > + page = radix_tree_lookup(&encl->page_tree, idx); > > > > + mutex_unlock(&encl->lock); > > > > + > > > > + if (!page || (~page->vm_max_prot_bits & vm_prot_bits)) > > > > + return -EACCES; > > > > > > You should really use an iterator here instead of repeated lookups. > > > xas_for_each() will probably be what you want. > > > > Thank you for your remarks. I'll look into using xarray for this. > > Question for Matthew: > > To enforce the "page must be populated" rule, is there a clean way to retrieve > the index of the current entry? Our entries/pages don't have information > about their index. Or should we just count the number of entries and check > 'em at the end? E.g. > > xas_for_each(...) { > if (~page->vm_max_prot_bits & vm_prot_bits) > return -EACCES; > nr_entries++; > } > > if (nr_entries != (end_index - start_index)) > return -EACCES; Probably best just to steal the implementation from here: pgoff_t page_cache_next_miss(struct address_space *mapping, pgoff_t index, unsigned long max_scan) { XA_STATE(xas, &mapping->i_pages, index); while (max_scan--) { void *entry = xas_next(&xas); if (!entry || xa_is_value(entry)) break; if (xas.xa_index == 0) break; } return xas.xa_index; } although I think you have a simpler task. XA_STATE(xas, ..., start_index); for (;;) { struct page *page = xas_next(&xas); if (!page || (~page->vm_max_prot_bits & vm_prot_bits)) return -EACCES; } return 0; should do the trick, I think.