[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 2/2] IOMMU/spinlock: Make the pcidevs_lock a recursive one
>>> On 10.03.16 at 12:27, <quan.xu@xxxxxxxxx> wrote: > On March 10, 2016 5:53pm, <March 10, 2016 5:53> wrote: >> >>> On 09.03.16 at 14:17, <quan.xu@xxxxxxxxx> wrote: >> > Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx> >> > Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx> >> >> The patch itself looks mostly fine now (see below for the minor left > issues), but >> the complete lack of a description (which should state why this change is > being >> done) makes this not ready to go in anyway. >> > > What about the following description: > -- > The pcidevs_lock may be recursively taken for hiding ATS device, > when VT-d Device-TLB flush timed out. s/may/is going to/ (since right now that isn't the case yet) >> > --- a/xen/drivers/passthrough/pci.c >> > +++ b/xen/drivers/passthrough/pci.c >> > @@ -48,7 +48,7 @@ struct pci_seg { >> > } bus2bridge[MAX_BUSES]; >> > }; >> > >> > -spinlock_t pcidevs_lock = SPIN_LOCK_UNLOCKED; >> > +static spinlock_t _pcidevs_lock = SPIN_LOCK_UNLOCKED; >> >> Why the renaming? >> > > The original name would cause compiler errors, as I also introduce a new > helper with same name -- void pcidevs_lock(void). Ah, of course. >> > @@ -103,6 +103,26 @@ static int pci_segments_iterate( >> > return rc; >> > } >> > >> > +void pcidevs_lock(void) >> > +{ >> > + spin_lock_recursive(&_pcidevs_lock); >> > +} >> > + >> > +void pcidevs_unlock(void) >> > +{ >> > + spin_unlock_recursive(&_pcidevs_lock); >> > +} >> > + >> > +int pcidevs_is_locked(void) >> >> bool_t >> >> > +{ >> > + return spin_is_locked(&_pcidevs_lock); } >> > + >> > +int pcidevs_trylock(void) >> >> bool_t >> >> (To avoid another round, please be aware that the underlying spin lock >> primitives still [wrongly] use "int", so to be fully correct you will need > to use !! in >> both return statements, unless you feel like (in another prereq patch) to > adjust >> those primitives too. >> > Thanks for reminding. > I'd prefer to use !! in both return statements. > btw, could you help me check the code style?: > > +bool_t pcidevs_is_locked(void) > +{ > + return !!(spin_is_locked(&_pcidevs_lock)); > +} > + > +bool_t pcidevs_trylock(void) > +{ > + return !!(spin_trylock_recursive(&_pcidevs_lock)); > +} > > Is it right? The outermost parentheses aren't needed, and harm readability. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |