On Fri, Jan 03, 2020 at 01:55:51PM +0100, Jan Beulich wrote: > On 03.01.2020 13:34, Roger Pau Monné wrote: > > On Fri, Jan 03, 2020 at 01:08:20PM +0100, Jan Beulich wrote: > >> On 24.12.2019 13:44, Roger Pau Monne wrote: > >> Further a question on lock nesting: Since the commit message > >> doesn't say anything in this regard, did you check there are no > >> TLB flush invocations with the get_cpu_maps() lock held? > > > > The CPU maps lock is a recursive one, so it should be fine to attempt > > a TLB flush with the lock already held. > > When already held by the same CPU - sure. It being a recursive > one (which I paid attention to when writing my earlier reply) > doesn't make it (together with any other one) immune against > ABBA deadlocks, though.
There's no possibility of a deadlock here because get_cpu_maps does a trylock, so if another cpu is holding the lock the flush will just fallback to not using the shorthand. > >> Even if > >> you did and even if there are none, I think the function should > >> then get a comment attached to the effect of this lock order > >> inversion risk. (For example, it isn't obvious to me that no user > >> of stop_machine() would ever want to do any kind of TLB flushing.) > >> > >> Overall I wonder whether your goal couldn't be achieved without > >> the extra locking and without the special conditions. > > > > Hm, this so far has worked fine on all the boxes that I've tried. > > I'm happy to change it to a simpler approach, but I think the > > conditions and locking are required for this to work correctly. > > Which might then indicate said pre-existing use of cpu_online_map > to be a (latent?) problem. Hm, maybe it could be a problem when offlining a CPU. I assume this is not an issue so far because there are no global TLB flushes with a mask contaning a CPU that is being offlined. Regarding the patch itself, do you think the shorthand logic should be added to send_IPI_mask, or would you rather only use the shorthand for the TLB flushes? Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
