From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Gordon Subject: Re: [KERNEL] Regression bug in drm/i915, Wrong assumption in commit e11aa36 breaks suspend on at least lenovo x61 Date: Thu, 19 Feb 2015 15:42:08 +0000 Message-ID: <54E60450.9010605@intel.com> References: <20150211113919.GA5672@ikki.ethgen.ch> <20150216221142.GB14602@ikki.ethgen.ch> <20150217082937.GA27873@ikki.ethgen.ch> <87mw4bw7o7.fsf@intel.com> <1424276692.27236.1.camel@intel.com> <54E5BF59.4020208@intel.com> <1424344107.3496.10.camel@ideak-mobl> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1424344107.3496.10.camel@ideak-mobl> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Deak, Imre" Cc: dri-devel , Klaus Ethgen , "intel-gfx@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" List-Id: dri-devel@lists.freedesktop.org T24gMTkvMDIvMTUgMTE6MDgsIERlYWssIEltcmUgd3JvdGU6Cj4gT24gVGh1LCAyMDE1LTAyLTE5 IGF0IDEwOjQ3ICswMDAwLCBEYXZlIEdvcmRvbiB3cm90ZToKPj4gT24gMTgvMDIvMTUgMTY6MjQs IEltcmUgRGVhayB3cm90ZToKPj4+IE9uIGtlLCAyMDE1LTAyLTE4IGF0IDE3OjM5ICswMjAwLCBK YW5pIE5pa3VsYSB3cm90ZToKPj4+PiBPbiBUdWUsIDE3IEZlYiAyMDE1LCBLbGF1cyBFdGhnZW4g PEtsYXVzK2xrbWxAZXRoZ2VuLmRlPiB3cm90ZToKPj4+Pj4gQWZ0ZXIgc29sdmluZyAgdGhlIGNv bmZsaWN0cywgSSBhcHBsaWVkIHRoZSByZXZlcnQgKHNlZSBhdHRhY2htZW50KSB0bwo+Pj4+PiB2 My4xOC43LiBJIHRoaW5rIGl0IHNob3VsZCBhbHNvIGFwcGx5IHRvIHRoZSBjdXJyZW50IGhlYWQu IFdpdGggdGhhdAo+Pj4+PiBwYXRjaCwgc3VzcGVuZCBpcyB3b3JraW5nIGFnYWluIG9uIHRoYXQg dmVyc2lvbi4KPj4+Pj4KPj4+Pj4gSG93ZXZlciwgSSBoYXZlIG5vdCB0byBkZWVwIGtub3dsZWRn ZSBvZiB0aGF0IHN1YnN5c3RlbSwgc28gcGxlYXNlLAo+Pj4+PiBzb21lb25lIHdobyBoYXZlLCBo YXZlIGEgZGVlcGVyIGxvb2sgaW50byBpdC4gSSBlc3BlY2lhbGx5IGRvIG5vdCBrbm93Cj4+Pj4+ IGlmIHRoZSBsaW5lcyBpbiAuLi4vaW50ZWxfcG0uYyBhcmUgY29ycmVjdCBvciBiZXR0ZXIgbGVh dmluZyB0aGVtIGFzCj4+Pj4+IHRoZXkgYXJlIGluIHYzLjE4LjcuCj4+Pj4+Cj4+Pj4+IEkgd2Fu dCB0byBoYXZlIGl0IHdvcmtpbmcgb24gYSB2ZXJzaW9uIHRoYXQgSSBrbm93IGlzIHN0YWJsZSBi ZWZvcmUKPj4+Pj4gYXNraW5nIHRvIHB1bGwgaXQgdG8gaGVhZC4KPj4+Pgo+Pj4+IEhpIEtsYXVz LCB3ZSBmZWFyIHRoaXMgcGF0Y2ggbWF5IGhpZGUgdGhlIGFjdHVhbCBjYXVzZS4gV291bGQgYmUg dXNlZnVsCj4+Pj4gdG8gZ2V0IGEgYmV0dGVyIGRlc2NyaXB0aW9uIG9mIHdoYXQgaGFwcGVucywg YWxvbmcgd2l0aCBhIGRtZXNnIHdpdGgKPj4+PiBkcm0uZGVidWc9MTQgbW9kdWxlIHBhcmFtZXRl ciBzZXQuIFRoaXMgbWlnaHQgY2x1dHRlciB0aGUgbWFpbGluZyBsaXN0LAo+Pj4+IHdvdWxkIHlv dSBtaW5kIGZpbGluZyBhIGJ1ZyBhdCBbMV0gYW5kIGF0dGFjaCB0aGUgaW5mbyB0aGVyZT8KPj4+ Pgo+Pj4+IFRoYW5rcywKPj4+PiBKYW5pLgo+Pj4+Cj4+Pj4gWzFdIGh0dHBzOi8vYnVncy5mcmVl ZGVza3RvcC5vcmcvZW50ZXJfYnVnLmNnaT9wcm9kdWN0PURSSSZjb21wb25lbnQ9RFJNL0ludGVs Cj4+Pgo+Pj4gSW4gYWRkaXRpb24gdG8gdGhlIGFib3ZlIGNvdWxkIHlvdSBhbHNvIHRyeSB0aGUg Zm9sbG93aW5nIHBhdGNoIGFuZAo+Pj4gcHJvdmlkZSBhIGRtZXNnIGxvZyBvbiB0aGUgYnVnemls bGEgdGlja2V0IC0gYXQgdGhpcyBwb2ludCBvbmx5IHRvIHRyeQo+Pj4gdG8gbmFycm93IGRvd24g dGhlIGlzc3VlPzoKPj4+Cj4+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkx NV9pcnEuYyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfaXJxLmMKPj4+IGluZGV4IGQzNThj ZTguLjAyYzY1ZjQgMTAwNjQ0Cj4+PiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2ly cS5jCj4+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2lycS5jCj4+PiBAQCAtNDQ2 Niw2ICs0NDY2LDE0IEBAIHN0YXRpYyBpcnFyZXR1cm5fdCBpOTY1X2lycV9oYW5kbGVyKGludCBp cnEsIHZvaWQgKmFyZykKPj4+ICAJCUk5MTVfRElTUExBWV9QTEFORV9BX0ZMSVBfUEVORElOR19J TlRFUlJVUFQgfAo+Pj4gIAkJSTkxNV9ESVNQTEFZX1BMQU5FX0JfRkxJUF9QRU5ESU5HX0lOVEVS UlVQVDsKPj4+ICAKPj4+ICsJaWYgKCFpbnRlbF9pcnFzX2VuYWJsZWQoZGV2X3ByaXYpKSB7Cj4+ PiArCQlpZiAocHJpbnRrX3JhdGVsaW1pdCgpKQo+Pj4gKwkJCURSTV9FUlJPUigic3B1cmlvdXMv c2hhcmVkIGludGVycnVwdCBpaXIgJTA4eCBpbXIgJTA4eCBpZXIgJTA4eFxuIiwKPj4+ICsJCQkJ ICBJOTE1X1JFQUQoSUlSKSwgSTkxNV9SRUFEKElNUiksIEk5MTVfUkVBRChJRVIpKTsKPj4+ICsK Pj4+ICsJCXJldHVybiBJUlFfTk9ORTsKPj4+ICsJfQo+Pj4gKwo+Pj4gIAlpaXIgPSBJOTE1X1JF QUQoSUlSKTsKPj4+ICAKPj4+ICAJZm9yICg7Oykgewo+Pj4gQEAgLTQ3NjYsNyArNDc3NCwxMCBA QCB2b2lkIGludGVsX3J1bnRpbWVfcG1fZGlzYWJsZV9pbnRlcnJ1cHRzKHN0cnVjdCBkcm1fZGV2 aWNlICpkZXYpCj4+PiAgCXN0cnVjdCBkcm1faTkxNV9wcml2YXRlICpkZXZfcHJpdiA9IGRldi0+ ZGV2X3ByaXZhdGU7Cj4+PiAgCj4+PiAgCWRldi0+ZHJpdmVyLT5pcnFfdW5pbnN0YWxsKGRldik7 Cj4+PiArCj4+PiArCXNwaW5fbG9ja19pcnEoJmRldl9wcml2LT5pcnFfbG9jayk7Cj4+PiAgCWRl dl9wcml2LT5wbS5faXJxc19kaXNhYmxlZCA9IHRydWU7Cj4+PiArCXNwaW5fdW5sb2NrX2lycSgm ZGV2X3ByaXYtPmlycV9sb2NrKTsKPj4+ICB9Cj4+PiAgCj4+PiAgLyogUmVzdG9yZSBpbnRlcnJ1 cHRzIHNvIHdlIGNhbiByZWNvdmVyIGZyb20gcnVudGltZSBQTS4gKi8KPj4+IEBAIC00Nzc0LDcg KzQ3ODUsMTAgQEAgdm9pZCBpbnRlbF9ydW50aW1lX3BtX3Jlc3RvcmVfaW50ZXJydXB0cyhzdHJ1 Y3QgZHJtX2RldmljZSAqZGV2KQo+Pj4gIHsKPj4+ICAJc3RydWN0IGRybV9pOTE1X3ByaXZhdGUg KmRldl9wcml2ID0gZGV2LT5kZXZfcHJpdmF0ZTsKPj4+ICAKPj4+ICsJc3Bpbl9sb2NrX2lycSgm ZGV2X3ByaXYtPmlycV9sb2NrKTsKPj4+ICAJZGV2X3ByaXYtPnBtLl9pcnFzX2Rpc2FibGVkID0g ZmFsc2U7Cj4+PiArCXNwaW5fdW5sb2NrX2lycSgmZGV2X3ByaXYtPmlycV9sb2NrKTsKPj4+ICsK Pj4+ICAJZGV2LT5kcml2ZXItPmlycV9wcmVpbnN0YWxsKGRldik7Cj4+PiAgCWRldi0+ZHJpdmVy LT5pcnFfcG9zdGluc3RhbGwoZGV2KTsKPj4+ICB9Cj4+Cj4+IFN1cmVseSBzdXJyb3VuZGluZyAo d2hhdCBvdWdodCB0byBiZSkgYW4gYXRvbWljIGFzc2lnbm1lbnQgdG8gYSBzaW5nbGUKPj4gdmFy aWFibGUgY2Fubm90IG1ha2UgYSBkaWZmZXJlbmNlPyBVbmxlc3MgaXQncyB0aGUgbWVtb3J5IGJh cnJpZXIKPj4gc2VtYW50aWNzIHRoYXQgaGF2ZSBzb21lIGVmZmVjdD8gSXQgc2VlbXMgdW5saWtl bHkgdGhhdCB0aGUgY29tcGlsZXIgaGFzCj4+IGRlZmVycmVkIHRoZSB3cml0ZSB0byB0aGUgdmFy aWFibGUgcGFzdCB0aGUgcHJlL3Bvc3RpbnN0YWxsIGNhbGxzIHRoYXQKPj4gYWN0dWFsbHkgZW5h YmxlIHRoZSBoL3cgaW50ZXJydXB0cywgYnV0IG1heWJlIHdlIHNob3VsZCBhZGQgInZvbGF0aWxl Igo+PiBqdXN0IGluIGNhc2U/Cj4gCj4gc3BpbmxvY2tzIGFsc28gc2VydmUgYXMgYSBtZW1vcnkg YmFycmllci4gdm9sYXRpbGUgd291bGQgZ3VhcmFudGVlIG9ubHkKPiB0aGF0IHRoZSBjb21waWxl ciBkb2Vzbid0IHJlb3JkZXIgdGhlIHdyaXRlLCBzbyBpdCB3b3VsZG4ndCBiZSBlbm91Z2gKPiBh bG9uZS4gT3RvaCwgd2UgbWF5IGFsc28gbmVlZCB0byBhZGQgc3BpbmxvY2tpbmcgdG8gdGhlIGlu dGVycnVwdAo+IGhhbmRsZXIgaWYgdGhlIGlzc3VlIHR1cm5zIG91dCB0byBiZSB0aGF0IHdlIGNh bid0IGFjY2VzcyBzb21lIHJlZ2lzdGVyCj4gcGFzdC9iZWZvcmUgaW50ZWxfcnVudGltZV9wbV97 ZGlzYWJsZSxlbmFibGV9X2ludGVycnVwdHMuCj4gCj4gLS1JbXJlCgpJZiB3ZSB3ZXJlIGdldHRp bmcgaW50ZXJydXB0cyBkdXJpbmcgdGhlIGVuYWJsaW5nIHNlcXVlbmNlLCB0aGVyZSB3b3VsZApi ZSB0aHJlZSBwb3NzaWJpbGl0aWVzOgoxLgljb21waWxlciBoYXMgZGVsYXllZCB3cml0ZWJhY2su IFRoaXMgd291bGQgYmUgYSBjb21waWxlciBidWcKCShJTUhPKSBidXQgJ3ZvbGF0aWxlJyBtaWdo dCBmaXggaXQuCjIuCXRoZSAncmVzdG9yZScgdGhyZWFkIGhhcyB3cml0dGVuIHRoZSB2YWx1ZSwg YnV0IGl0IGlzbid0IHNlZW4KCWJ5IHRoZSB0aHJlYWQgaGFuZGxpbmcgdGhlIGludGVycnVwdCAo b24gYSBkaWZmZXJlbnQgY3B1PykuCglUaGlzIHdvdWxkIGJlIGEgY29oZXJlbmN5IGlzc3VlLCBh bmQgYSBtZW1vcnkgYmFycmllciBzaG91bGQKCWZpeCBpdC4gQnV0IEkgd291bGQgaGF2ZSBleHBl Y3RlZCB0aGUgdmFyaWFibGUgdG8gYmUgaW4KCWNvaGVyZW50IG1lbW9yeSBhbnl3YXkuCjMuCXRo ZSBHUFUgaC93IHNlbmRpbmcgaW50ZXJydXB0cyBiZWZvcmUgdGhleSdyZSBlbmFibGVkIDooCgpC dXQgSSBzdXNwZWN0IHRoaXMgbWlnaHQgYmUgZHVyaW5nICpkaXNhYmxpbmcqIHRoZSBpbnRlcnJ1 cHQuIFBvc3NpYmx5IGEKcmFjZSBjb25kaXRpb24gaW4gd2hpY2ggdGhlIGgvdyBoYXMgYWxyZWFk eSBnZW5lcmF0ZWQgYW4gaW50ZXJydXB0CmJlZm9yZSBpdCdzIGRpc2FibGVkLCBidXQgdGhhdCBp bnRlcnJ1cHQgaGFzIG5vdCB5ZXQgYmVlbiBmaWVsZGVkOyBzbwp0aGF0IGJ5IHRoZSB0aW1lIHRo ZSBpbnRlcnJ1cHQgaGFuZGxlciBydW5zIChvbiBhIGRpZmZlcmVudCBDUFUgZnJvbSB0aGUKJ3N1 c3BlbmQnIHRocmVhZCksIHRoZSB3cml0ZSB0byB0aGUgdmFyaWFibGUgY2FuIGhhdmUgaGFwcGVu ZWQgYW5kIHRoZW4KdGhlIG5ldyB2YWx1ZSBpcyBzZWVuIGJ5IHRoZSBpbnRlcnJ1cHQgaGFuZGxl ci4KCkluIHdoaWNoIGNhc2UgdGhlIHR3ZWFrIGFib3ZlIHdpbGwgcmVkdWNlIGJ1dCBub3QgbmVj ZXNzYXJpbHkgZWxpbWluYXRlCnRoZSB3aW5kb3c7IGl0IHdpbGwgZW5zdXJlIHRoYXQgaWYgdGhl IGhhbmRsZXIgaGFzIGdvdCBhdCBsZWFzdCBhcyBmYXIKYXMgdGFraW5nIHRoZSBzcGlubG9jayBi ZWZvcmUgdGhlIHN1c3BlbmQgdGhyZWFkLCB0aGVuIHRoZSB3cml0ZSB3aWxsIGJlCmRlbGF5ZWQg dW50aWwgdGhlIGhhbmRsZXIgaGFzIGZpbmlzaGVkLiBPVE9IIGlmIHRoZSBpbnRlcnJ1cHQgd2Vy ZQpkZWxheWVkIGEgbGl0dGxlIGJpdCBtb3JlLCB0aGlzIGNvZGUgbWlnaHQgc3RpbGwgZ2V0IHRv IHRha2UgdGhlCnNwaW5sb2NrIGJlZm9yZSB0aGUgaGFuZGxlciwgc28gdGhlIHVuZXhwZWN0ZWQg dmFsdWUgd291bGQgc3RpbGwgYmUgc2VlbiA6KAoKTWF5YmUgeW91IG5lZWQgYSB0d28tcGhhc2Ug dW5pbnN0YWxsPwoKLkRhdmUuCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fCkludGVsLWdmeCBtYWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNr dG9wLm9yZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50 ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752020AbbBSPmO (ORCPT ); Thu, 19 Feb 2015 10:42:14 -0500 Received: from mga11.intel.com ([192.55.52.93]:58632 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751402AbbBSPmN (ORCPT ); Thu, 19 Feb 2015 10:42:13 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,609,1418112000"; d="scan'208";a="456808984" Message-ID: <54E60450.9010605@intel.com> Date: Thu, 19 Feb 2015 15:42:08 +0000 From: Dave Gordon Organization: Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: "Deak, Imre" CC: Jani Nikula , Klaus Ethgen , "intel-gfx@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , dri-devel Subject: Re: [Intel-gfx] [KERNEL] Regression bug in drm/i915, Wrong assumption in commit e11aa36 breaks suspend on at least lenovo x61 References: <20150211113919.GA5672@ikki.ethgen.ch> <20150216221142.GB14602@ikki.ethgen.ch> <20150217082937.GA27873@ikki.ethgen.ch> <87mw4bw7o7.fsf@intel.com> <1424276692.27236.1.camel@intel.com> <54E5BF59.4020208@intel.com> <1424344107.3496.10.camel@ideak-mobl> In-Reply-To: <1424344107.3496.10.camel@ideak-mobl> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 19/02/15 11:08, Deak, Imre wrote: > On Thu, 2015-02-19 at 10:47 +0000, Dave Gordon wrote: >> On 18/02/15 16:24, Imre Deak wrote: >>> On ke, 2015-02-18 at 17:39 +0200, Jani Nikula wrote: >>>> On Tue, 17 Feb 2015, Klaus Ethgen wrote: >>>>> After solving the conflicts, I applied the revert (see attachment) to >>>>> v3.18.7. I think it should also apply to the current head. With that >>>>> patch, suspend is working again on that version. >>>>> >>>>> However, I have not to deep knowledge of that subsystem, so please, >>>>> someone who have, have a deeper look into it. I especially do not know >>>>> if the lines in .../intel_pm.c are correct or better leaving them as >>>>> they are in v3.18.7. >>>>> >>>>> I want to have it working on a version that I know is stable before >>>>> asking to pull it to head. >>>> >>>> Hi Klaus, we fear this patch may hide the actual cause. Would be useful >>>> to get a better description of what happens, along with a dmesg with >>>> drm.debug=14 module parameter set. This might clutter the mailing list, >>>> would you mind filing a bug at [1] and attach the info there? >>>> >>>> Thanks, >>>> Jani. >>>> >>>> [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel >>> >>> In addition to the above could you also try the following patch and >>> provide a dmesg log on the bugzilla ticket - at this point only to try >>> to narrow down the issue?: >>> >>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >>> index d358ce8..02c65f4 100644 >>> --- a/drivers/gpu/drm/i915/i915_irq.c >>> +++ b/drivers/gpu/drm/i915/i915_irq.c >>> @@ -4466,6 +4466,14 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) >>> I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT | >>> I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT; >>> >>> + if (!intel_irqs_enabled(dev_priv)) { >>> + if (printk_ratelimit()) >>> + DRM_ERROR("spurious/shared interrupt iir %08x imr %08x ier %08x\n", >>> + I915_READ(IIR), I915_READ(IMR), I915_READ(IER)); >>> + >>> + return IRQ_NONE; >>> + } >>> + >>> iir = I915_READ(IIR); >>> >>> for (;;) { >>> @@ -4766,7 +4774,10 @@ void intel_runtime_pm_disable_interrupts(struct drm_device *dev) >>> struct drm_i915_private *dev_priv = dev->dev_private; >>> >>> dev->driver->irq_uninstall(dev); >>> + >>> + spin_lock_irq(&dev_priv->irq_lock); >>> dev_priv->pm._irqs_disabled = true; >>> + spin_unlock_irq(&dev_priv->irq_lock); >>> } >>> >>> /* Restore interrupts so we can recover from runtime PM. */ >>> @@ -4774,7 +4785,10 @@ void intel_runtime_pm_restore_interrupts(struct drm_device *dev) >>> { >>> struct drm_i915_private *dev_priv = dev->dev_private; >>> >>> + spin_lock_irq(&dev_priv->irq_lock); >>> dev_priv->pm._irqs_disabled = false; >>> + spin_unlock_irq(&dev_priv->irq_lock); >>> + >>> dev->driver->irq_preinstall(dev); >>> dev->driver->irq_postinstall(dev); >>> } >> >> Surely surrounding (what ought to be) an atomic assignment to a single >> variable cannot make a difference? Unless it's the memory barrier >> semantics that have some effect? It seems unlikely that the compiler has >> deferred the write to the variable past the pre/postinstall calls that >> actually enable the h/w interrupts, but maybe we should add "volatile" >> just in case? > > spinlocks also serve as a memory barrier. volatile would guarantee only > that the compiler doesn't reorder the write, so it wouldn't be enough > alone. Otoh, we may also need to add spinlocking to the interrupt > handler if the issue turns out to be that we can't access some register > past/before intel_runtime_pm_{disable,enable}_interrupts. > > --Imre If we were getting interrupts during the enabling sequence, there would be three possibilities: 1. compiler has delayed writeback. This would be a compiler bug (IMHO) but 'volatile' might fix it. 2. the 'restore' thread has written the value, but it isn't seen by the thread handling the interrupt (on a different cpu?). This would be a coherency issue, and a memory barrier should fix it. But I would have expected the variable to be in coherent memory anyway. 3. the GPU h/w sending interrupts before they're enabled :( But I suspect this might be during *disabling* the interrupt. Possibly a race condition in which the h/w has already generated an interrupt before it's disabled, but that interrupt has not yet been fielded; so that by the time the interrupt handler runs (on a different CPU from the 'suspend' thread), the write to the variable can have happened and then the new value is seen by the interrupt handler. In which case the tweak above will reduce but not necessarily eliminate the window; it will ensure that if the handler has got at least as far as taking the spinlock before the suspend thread, then the write will be delayed until the handler has finished. OTOH if the interrupt were delayed a little bit more, this code might still get to take the spinlock before the handler, so the unexpected value would still be seen :( Maybe you need a two-phase uninstall? .Dave.