| Message ID | 20170609141639.8903-1-konrad.wilk@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 09.06.17 at 16:16, <konrad.wilk@oracle.com> wrote: > If we have a large amount of livepatches and want to print them > on the console using 'xl debug-keys x' we eventually hit > the preemption check: > > if ( i && !(i % 64) ) > { > spin_unlock(&payload_lock); > process_pending_softirqs(); > if ( spin_trylock(&payload_lock) ) > return > > <facepalm> The effect is that we have just effectively > taken the lock and returned without unlocking! > > CC: Ross Lagerwall <ross.lagerwall@citrix.com> > CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> > CC: Jan Beulich <jbeulich@suse.com> > CC: Andrew Cooper <andrew.cooper3@citrix.com> > CC: Julien Grall <julien.grall@arm.com> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
On 06/09/2017 03:16 PM, Konrad Rzeszutek Wilk wrote: > If we have a large amount of livepatches and want to print them > on the console using 'xl debug-keys x' we eventually hit > the preemption check: > > if ( i && !(i % 64) ) > { > spin_unlock(&payload_lock); > process_pending_softirqs(); > if ( spin_trylock(&payload_lock) ) > return > > <facepalm> The effect is that we have just effectively > taken the lock and returned without unlocking! > > CC: Ross Lagerwall <ross.lagerwall@citrix.com> > CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> > CC: Jan Beulich <jbeulich@suse.com> > CC: Andrew Cooper <andrew.cooper3@citrix.com> > CC: Julien Grall <julien.grall@arm.com> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
On 06/09/2017 10:16 AM, Konrad Rzeszutek Wilk wrote: > If we have a large amount of livepatches and want to print them > on the console using 'xl debug-keys x' we eventually hit > the preemption check: > > if ( i && !(i % 64) ) > { > spin_unlock(&payload_lock); > process_pending_softirqs(); > if ( spin_trylock(&payload_lock) ) > return > > <facepalm> The effect is that we have just effectively > taken the lock and returned without unlocking! > > CC: Ross Lagerwall <ross.lagerwall@citrix.com> > CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> > CC: Jan Beulich <jbeulich@suse.com> > CC: Andrew Cooper <andrew.cooper3@citrix.com> > CC: Julien Grall <julien.grall@arm.com> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-and-tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
On Fri, Jun 09, 2017 at 01:12:10PM -0400, Boris Ostrovsky wrote: > On 06/09/2017 10:16 AM, Konrad Rzeszutek Wilk wrote: > > If we have a large amount of livepatches and want to print them > > on the console using 'xl debug-keys x' we eventually hit > > the preemption check: > > > > if ( i && !(i % 64) ) > > { > > spin_unlock(&payload_lock); > > process_pending_softirqs(); > > if ( spin_trylock(&payload_lock) ) > > return > > > > <facepalm> The effect is that we have just effectively > > taken the lock and returned without unlocking! > > > > CC: Ross Lagerwall <ross.lagerwall@citrix.com> > > CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> > > CC: Jan Beulich <jbeulich@suse.com> > > CC: Andrew Cooper <andrew.cooper3@citrix.com> > > CC: Julien Grall <julien.grall@arm.com> Julien, could you Release-Ack it for 4.9 please? > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > > Reviewed-and-tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> > >
Hi Konrad, On 11/06/17 13:15, Konrad Rzeszutek Wilk wrote: > On Fri, Jun 09, 2017 at 01:12:10PM -0400, Boris Ostrovsky wrote: >> On 06/09/2017 10:16 AM, Konrad Rzeszutek Wilk wrote: >>> If we have a large amount of livepatches and want to print them >>> on the console using 'xl debug-keys x' we eventually hit >>> the preemption check: >>> >>> if ( i && !(i % 64) ) >>> { >>> spin_unlock(&payload_lock); >>> process_pending_softirqs(); >>> if ( spin_trylock(&payload_lock) ) >>> return >>> >>> <facepalm> The effect is that we have just effectively >>> taken the lock and returned without unlocking! >>> >>> CC: Ross Lagerwall <ross.lagerwall@citrix.com> >>> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> >>> CC: Jan Beulich <jbeulich@suse.com> >>> CC: Andrew Cooper <andrew.cooper3@citrix.com> >>> CC: Julien Grall <julien.grall@arm.com> > > Julien, could you Release-Ack it for 4.9 please? Sure: Release-acked-by: Julien Grall <julien.grall@arm.com> Cheers,
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index f14bcbc..df67a1a 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -1642,7 +1642,7 @@ static void livepatch_printall(unsigned char key) { spin_unlock(&payload_lock); process_pending_softirqs(); - if ( spin_trylock(&payload_lock) ) + if ( !spin_trylock(&payload_lock) ) { printk("Couldn't reacquire lock. Try again.\n"); return;
If we have a large amount of livepatches and want to print them on the console using 'xl debug-keys x' we eventually hit the preemption check: if ( i && !(i % 64) ) { spin_unlock(&payload_lock); process_pending_softirqs(); if ( spin_trylock(&payload_lock) ) return <facepalm> The effect is that we have just effectively taken the lock and returned without unlocking! CC: Ross Lagerwall <ross.lagerwall@citrix.com> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> CC: Jan Beulich <jbeulich@suse.com> CC: Andrew Cooper <andrew.cooper3@citrix.com> CC: Julien Grall <julien.grall@arm.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- xen/common/livepatch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)