From: Jason Gunthorpe jgg@nvidia.com Sent: Tuesday, November 8, 2022 8:49 AM
+struct interval_tree_double_span_iter {
- struct rb_root_cached *itrees[2];
- struct interval_tree_span_iter spans[2];
- union {
unsigned long start_hole;unsigned long start_used;- };
- union {
unsigned long last_hole;unsigned long last_used;- };
- /* 0 = hole, 1 = used span[0], 2 = used span[1], -1 done iteration */
- int is_used;
+};
lack of a comment how this expects to be used as done for struct interval_tree_span_iter. e.g. there is no value representing used by both spans which implies this is used to find valid range in either side. Those should be spelled out.
+/*
- The IOVA to PFN map. The mapper automatically copies the PFNs into
multiple
what is the mapper?
- Be cautious of overflow, an IOVA can go all the way up to U64_MAX, so
- last_iova + 1 can overflow. An iopt_pages index will always be much less
than
- ULONG_MAX< so last_index + 1 cannot overflow.
remove trailing '<' after ULONG_MAX
- ret = iommu_unmap(domain, iova, size);
- /*
* It is a logic error in this code or a driver bug if the IOMMU unmaps* something other than exactly as requested. This implies that the* iommu driver may not fail unmap for reasons beyond badagruments.
* Particularly, the iommu driver may not do a memory allocation onthe
* unmap path.*/
didn't understand the last sentence.
+static void batch_skip_carry(struct pfn_batch *batch, unsigned int skip_pfns)
add a comment similar to batch_clear_carry()
+{
- if (!batch->total_pfns)
return;- skip_pfns = min(batch->total_pfns, skip_pfns);
- batch->pfns[0] += skip_pfns;
- batch->npfns[0] -= skip_pfns;
what about skip_pfns exceeds batch->npfns[0]? looks this works only if batch->total_pfns = batch->npfns[0]...
+/* true if the pfn could be added, false otherwise */ +static bool batch_add_pfn(struct pfn_batch *batch, unsigned long pfn) +{
- /* FIXME: U16 is too small */
performance or functional impact?
what would be the fix? and why cannot it be done now?
more comment is welcomed.
+static void batch_unpin(struct pfn_batch *batch, struct iopt_pages *pages,
unsigned int offset, size_t npages)+{
- unsigned int cur = 0;
- while (offset) {
if (batch->npfns[cur] > offset)break;offset -= batch->npfns[cur];cur++;- }
'offset' usually means byte-addressed. 'index' is a better fit in this context.
<to be continued>