On 6/15/2023 11:52 PM, Tian, Kevin wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.From: Brett Creeley <brett.creeley@xxxxxxx> Sent: Saturday, June 3, 2023 6:03 AM +void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes, + u32 req_nodes) +{ + struct interval_tree_node *prev, *curr, *comb_start, *comb_end; + unsigned long min_gap, curr_gap; + + /* Special shortcut when a single range is required */ + if (req_nodes == 1) { + unsigned long last; + + comb_start = interval_tree_iter_first(root, 0, ULONG_MAX); + curr = comb_start; + while (curr) { + last = curr->last; + prev = curr; + curr = interval_tree_iter_next(curr, 0, ULONG_MAX); + if (prev != comb_start) + interval_tree_remove(prev, root); + } + comb_start->last = last; + return; + } + + /* Combine ranges which have the smallest gap */ + while (cur_nodes > req_nodes) { + prev = NULL; + min_gap = ULONG_MAX; + curr = interval_tree_iter_first(root, 0, ULONG_MAX); + while (curr) { + if (prev) { + curr_gap = curr->start - prev->last; + if (curr_gap < min_gap) { + min_gap = curr_gap; + comb_start = prev; + comb_end = curr; + } + } + prev = curr; + curr = interval_tree_iter_next(curr, 0, ULONG_MAX); + } + comb_start->last = comb_end->last; + interval_tree_remove(comb_end, root); + cur_nodes--; + } +} +EXPORT_SYMBOL_GPL(vfio_combine_iova_ranges); +Being a public function please follow the kernel convention with comment explaining what this function actually does.
I've seen many cases that there's no documentation for public functions and I don't think any documentation is needed for this function as the name is self explanatory. VFIO drivers can use this to combine iova ranges, hence why I named it vfio_combine_iova_ranges().
btw while you rename it with 'vfio' and 'iova' keywords, the actual logic has nothing to do with either of them. Does it make more sense to move it to a more generic library?
I think it *could* go into a more generic library, but at this point in time I think it belongs here. As mentioned in the previous comment the function name describes its exact purpose. If/when it ever gets more users it can be moved and renamed.
