Hi Joerg, On 5/27/19 5:23 PM, Joerg Roedel wrote: > On Mon, May 27, 2019 at 10:55:36AM +0200, Eric Auger wrote: >> - list_add_tail(&rmrr->resv->list, head); >> + length = rmrr->end_address - rmrr->base_address + 1; >> + resv = iommu_alloc_resv_region(rmrr->base_address, >> + length, prot, >> + IOMMU_RESV_DIRECT, >> + GFP_ATOMIC); >> + if (!resv) >> + break; >> + >> + list_add_tail(&resv->list, head); > > Okay, so this happens in a rcu_read_locked section and must be atomic, > but I don't like this extra parameter to iommu_alloc_resv_region(). > > How about replacing the rcu-lock with the dmar_global_lock, which > protects against changes to the global rmrr list? This will make this > loop preemptible and taking the global lock is okay because this > function is in no way performance relevant.
After studying in more details the for_each_active_dev_scope macro and rcu_dereference_check it looks OK to me. I respinned accordingly. Thanks Eric > > Regards, > > Joerg >
