On 06/28/2012 04:51 AM, Hiroshi DOYU wrote:
> alloc_pdir() is called from smmu_iommu_domain_init() with spin_lock
> held. memory allocations in alloc_pdir() had to be
> atomic/unsleepable. Instead of converting into atomic allocation, this
> patch once releases a lock, do the allocation, hold the lock again and
> then see if it's raced or not in order to avoid introducing mutex.
> ---
You'd typically want to include a brief description of what changed from
v1->v2 here, as a hint to reviewers re: what to concentrate on.
> +static int alloc_pdir(struct smmu_as *as, unsigned long *flags)
> + spin_unlock_irqrestore(&as->lock, *flags);
> + cnt = devm_kzalloc(smmu->dev,
> + sizeof(cnt[0]) * SMMU_PDIR_COUNT, GFP_KERNEL);
> + page = alloc_page(GFP_KERNEL | __GFP_DMA);
> + spin_lock_irqsave(&as->lock, *flags);
> +
> + if (as->pdir_page) {
> + /* We raced, free the redundant */
> + err = -ENODEV;
> + goto err_out;
> }
Is that really an error; it just means that something else allocated the
same pdir already. Since the top of the function does:
if (as->pdir_page)
return 0;
I'd expect to s/err = -ENODEV/err = 0/ inside that if condition that I
quoted above, but still of cause "goto err_out" to free the now unneeded
allocations.
Aside from that, I think this looks reasonable.
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
