Messages in this thread | | | Date | Fri, 10 Jun 2011 10:36:04 -0700 | From | Chris Wright <> | Subject | Re: [PATCH 2/9] x86/amd-iommu: Introduce global dev_data_list |
| |
* Joerg Roedel (joerg.roedel@amd.com) 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.
| |