Hi Christoph,
On 5/29/19 8:17 AM, Christoph Hellwig wrote:
>> } else if ((start >= a) && (end <= b)) {
>> if (new->type == type)
>> - goto done;
>> + return 0;
>> else
>> pos = pos->next;
>
> Please remove the pointless else after the return statement.
Sorry I don't get your point here. For both cases, we only return if
both types match. Otherwise we continue parsing the list as there may be
a region of the same type as @new following the current interval that
may need to be merged with @new. Let's consider the insertion of those
resv regions:
add_resv(head, 0xa001000, 0xa001FFF, IOMMU_RESV_TYPE1);
head -> 0x000000000a001000 0x000000000a001fff type1
add_resv(head, 0xa003000, 0xa003FFF, IOMMU_RESV_TYPE1);
head ->
0x000000000a001000 0x000000000a001fff type1 ->
0x000000000a003000 0x000000000a003fff type1
add_resv(head, 0xa002000, 0xa004FFF, IOMMU_RESV_TYPE2);
head ->
0x000000000a001000 0x000000000a001fff type1 ->
0x000000000a003000 0x000000000a003fff type1 ->
0x000000000a002000 0x000000000a004fff type2 ->
add_resv(head, 0xa0010FF, 0xa003000, IOMMU_RESV_TYPE2);
head ->
0x000000000a001000 0x000000000a001fff type1 ->
0x000000000a003000 0x000000000a003fff type1 ->
0x000000000a0010ff 0x000000000a004fff type2 <- merge
Or maybe I simply missed your point. Please let me know.
Thanks
Eric
>
>> } else {
>> if (new->type == type) {
>> phys_addr_t new_start = min(a, start);
>> phys_addr_t new_end = max(b, end);
>> + int ret;
>>
>> list_del(&entry->list);
>> entry->start = new_start;
>> entry->length = new_end - new_start + 1;
>> - iommu_insert_resv_region(entry, regions);
>> + ret = iommu_insert_resv_region(entry, regions);
>> + kfree(entry);
>> + return ret;
>> } else {
>> pos = pos->next;
>> }
>
> Same here.
>
