Skip to content

Commit fa08280

Browse files
jgunthorpejoergroedel
authored andcommitted
iommu: Split iommu_group_add_device()
Move the list_add_tail() for the group_device into the critical region that immediately follows in __iommu_probe_device(). This avoids one case of unlocking and immediately re-locking the group->mutex. Consistently make the caller responsible for setting dev->iommu_group, prior patches moved this into iommu_init_device(), make the no-driver path do this in iommu_group_add_device(). This completes making __iommu_group_free_device() and iommu_group_alloc_device() into pair'd functions. Reviewed-by: Lu Baolu <[email protected]> Reviewed-by: Kevin Tian <[email protected]> Signed-off-by: Jason Gunthorpe <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Joerg Roedel <[email protected]>
1 parent cfb6ee6 commit fa08280

File tree

1 file changed

+43
-23
lines changed

1 file changed

+43
-23
lines changed

drivers/iommu/iommu.c

Lines changed: 43 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ static int iommu_create_device_direct_mappings(struct iommu_domain *domain,
129129
struct device *dev);
130130
static ssize_t iommu_group_store_type(struct iommu_group *group,
131131
const char *buf, size_t count);
132+
static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
133+
struct device *dev);
132134

133135
#define IOMMU_GROUP_ATTR(_name, _mode, _show, _store) \
134136
struct iommu_group_attribute iommu_group_attr_##_name = \
@@ -435,6 +437,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
435437
const struct iommu_ops *ops = dev->bus->iommu_ops;
436438
struct iommu_group *group;
437439
static DEFINE_MUTEX(iommu_probe_device_lock);
440+
struct group_device *gdev;
438441
int ret;
439442

440443
if (!ops)
@@ -459,16 +462,17 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
459462
goto out_unlock;
460463

461464
group = dev->iommu_group;
462-
ret = iommu_group_add_device(group, dev);
465+
gdev = iommu_group_alloc_device(group, dev);
463466
mutex_lock(&group->mutex);
464-
if (ret)
467+
if (IS_ERR(gdev)) {
468+
ret = PTR_ERR(gdev);
465469
goto err_put_group;
470+
}
466471

472+
list_add_tail(&gdev->list, &group->devices);
467473
if (group_list && !group->default_domain && list_empty(&group->entry))
468474
list_add_tail(&group->entry, group_list);
469475
mutex_unlock(&group->mutex);
470-
iommu_group_put(group);
471-
472476
mutex_unlock(&iommu_probe_device_lock);
473477

474478
return 0;
@@ -578,7 +582,10 @@ static void __iommu_group_remove_device(struct device *dev)
578582
}
579583
mutex_unlock(&group->mutex);
580584

581-
/* Pairs with the get in iommu_group_add_device() */
585+
/*
586+
* Pairs with the get in iommu_init_device() or
587+
* iommu_group_add_device()
588+
*/
582589
iommu_group_put(group);
583590
}
584591

@@ -1067,22 +1074,16 @@ static int iommu_create_device_direct_mappings(struct iommu_domain *domain,
10671074
return ret;
10681075
}
10691076

1070-
/**
1071-
* iommu_group_add_device - add a device to an iommu group
1072-
* @group: the group into which to add the device (reference should be held)
1073-
* @dev: the device
1074-
*
1075-
* This function is called by an iommu driver to add a device into a
1076-
* group. Adding a device increments the group reference count.
1077-
*/
1078-
int iommu_group_add_device(struct iommu_group *group, struct device *dev)
1077+
/* This is undone by __iommu_group_free_device() */
1078+
static struct group_device *iommu_group_alloc_device(struct iommu_group *group,
1079+
struct device *dev)
10791080
{
10801081
int ret, i = 0;
10811082
struct group_device *device;
10821083

10831084
device = kzalloc(sizeof(*device), GFP_KERNEL);
10841085
if (!device)
1085-
return -ENOMEM;
1086+
return ERR_PTR(-ENOMEM);
10861087

10871088
device->dev = dev;
10881089

@@ -1113,17 +1114,11 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
11131114
goto err_free_name;
11141115
}
11151116

1116-
iommu_group_ref_get(group);
1117-
dev->iommu_group = group;
1118-
1119-
mutex_lock(&group->mutex);
1120-
list_add_tail(&device->list, &group->devices);
1121-
mutex_unlock(&group->mutex);
11221117
trace_add_device_to_group(group->id, dev);
11231118

11241119
dev_info(dev, "Adding to iommu group %d\n", group->id);
11251120

1126-
return 0;
1121+
return device;
11271122

11281123
err_free_name:
11291124
kfree(device->name);
@@ -1132,7 +1127,32 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
11321127
err_free_device:
11331128
kfree(device);
11341129
dev_err(dev, "Failed to add to iommu group %d: %d\n", group->id, ret);
1135-
return ret;
1130+
return ERR_PTR(ret);
1131+
}
1132+
1133+
/**
1134+
* iommu_group_add_device - add a device to an iommu group
1135+
* @group: the group into which to add the device (reference should be held)
1136+
* @dev: the device
1137+
*
1138+
* This function is called by an iommu driver to add a device into a
1139+
* group. Adding a device increments the group reference count.
1140+
*/
1141+
int iommu_group_add_device(struct iommu_group *group, struct device *dev)
1142+
{
1143+
struct group_device *gdev;
1144+
1145+
gdev = iommu_group_alloc_device(group, dev);
1146+
if (IS_ERR(gdev))
1147+
return PTR_ERR(gdev);
1148+
1149+
iommu_group_ref_get(group);
1150+
dev->iommu_group = group;
1151+
1152+
mutex_lock(&group->mutex);
1153+
list_add_tail(&gdev->list, &group->devices);
1154+
mutex_unlock(&group->mutex);
1155+
return 0;
11361156
}
11371157
EXPORT_SYMBOL_GPL(iommu_group_add_device);
11381158

0 commit comments

Comments
 (0)