Re: [PATCH 2/9] x86/amd-iommu: Introduce global dev_data_list

From: Chris Wright
Date: Fri Jun 10 2011 - 13:36:19 EST


* Joerg Roedel (joerg.roedel@xxxxxxx) wrote:
> +static struct iommu_dev_data *alloc_dev_data(void)
> +{
> + struct iommu_dev_data *dev_data;
> + unsigned long flags;
> +
> + dev_data = kzalloc(sizeof(*dev_data), GFP_KERNEL);
> + if (!dev_data)
> + return NULL;
> +
> + atomic_set(&dev_data->bind, 0);
> +
> + spin_lock_irqsave(&dev_data_list_lock, flags);
> + list_add_tail(&dev_data->dev_data_list, &dev_data_list);
> + spin_unlock_irqrestore(&dev_data_list_lock, flags);

Globally visible but only paritially initiailized. I didn't see any, but
would this ever cause an issue?

> +static void free_dev_data(struct iommu_dev_data *dev_data)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev_data_list_lock, flags);
> + list_del(&dev_data->dev_data_list);
> + spin_unlock_irqrestore(&dev_data_list_lock, flags);
> +
> + kfree(dev_data);
<snip>
> + /* Free all of our dev_data structures */
> + list_for_each_entry_safe(dev_data, n, &dev_data_list, dev_data_list)
> + free_dev_data(dev_data);

Given that it's not actually contended in early init, should be fine...but
typically full list traversal would be protected by lock rather than
repeatedly acquiring and releasing the lock.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/