From owner-svn-src-all@freebsd.org Wed Jul 15 17:36:36 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 65E669A29C1; Wed, 15 Jul 2015 17:36:36 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4974C1B28; Wed, 15 Jul 2015 17:36:36 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svnmir.geo.freebsd.org ([127.0.1.70]) by repo.freebsd.org (8.14.9/8.14.9) with ESMTP id t6FHaaid076835; Wed, 15 Jul 2015 17:36:36 GMT (envelope-from kib@FreeBSD.org) Received: (from kib@localhost) by svnmir.geo.freebsd.org (8.14.9/8.14.9/Submit) id t6FHaaBA076834; Wed, 15 Jul 2015 17:36:36 GMT (envelope-from kib@FreeBSD.org) Message-Id: <201507151736.t6FHaaBA076834@svnmir.geo.freebsd.org> X-Authentication-Warning: svnmir.geo.freebsd.org: kib set sender to kib@FreeBSD.org using -f From: Konstantin Belousov Date: Wed, 15 Jul 2015 17:36:36 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r285607 - head/sys/kern X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Jul 2015 17:36:36 -0000 Author: kib Date: Wed Jul 15 17:36:35 2015 New Revision: 285607 URL: https://svnweb.freebsd.org/changeset/base/285607 Log: Reset non-zero it_need indicator to zero atomically with fetching its current value. It is believed that the change is the real fix for the issue which was covered over by the r252683. With the current code, if the interrupt handler sets it_need between read and consequent reset, the update could be lost and ithread_execute_handlers() would not be called in response to the lost update. The r252683 could have hide the issue since at the moment of commit, atomic_load_acq_int() did locked cmpxchg on the variable, which puts the cache line into the exclusive owned state and clears store buffers. Then the immediate store of zero has very high chance of reusing the exclusive state of the cache line and make the load and store sequence operate as atomic swap. For now, add the acq+rel fence immediately after the swap, to not disturb current (but excessive) ordering. Acquire is needed for the ih_need reads after the load, while release does not serve a useful purpose [*]. Reviewed by: alc Noted by: alc [*] Discussed with: bde Tested by: pho Sponsored by: The FreeBSD Foundation MFC after: 2 weeks Modified: head/sys/kern/kern_intr.c Modified: head/sys/kern/kern_intr.c ============================================================================== --- head/sys/kern/kern_intr.c Wed Jul 15 17:14:05 2015 (r285606) +++ head/sys/kern/kern_intr.c Wed Jul 15 17:36:35 2015 (r285607) @@ -1327,14 +1327,13 @@ ithread_loop(void *arg) * we are running, it will set it_need to note that we * should make another pass. */ - while (atomic_load_acq_int(&ithd->it_need) != 0) { + while (atomic_swap_int(&ithd->it_need, 0) != 0) { /* - * This might need a full read and write barrier - * to make sure that this write posts before any - * of the memory or device accesses in the - * handlers. + * This needs a release barrier to make sure + * that this write posts before any of the + * memory or device accesses in the handlers. */ - atomic_store_rel_int(&ithd->it_need, 0); + atomic_thread_fence_acq_rel(); ithread_execute_handlers(p, ie); } WITNESS_WARN(WARN_PANIC, NULL, "suspending ithread"); @@ -1507,14 +1506,13 @@ ithread_loop(void *arg) * we are running, it will set it_need to note that we * should make another pass. */ - while (atomic_load_acq_int(&ithd->it_need) != 0) { + while (atomic_swap_int(&ithd->it_need, 0) != 0) { /* - * This might need a full read and write barrier - * to make sure that this write posts before any - * of the memory or device accesses in the - * handlers. + * This needs a release barrier to make sure + * that this write posts before any of the + * memory or device accesses in the handlers. */ - atomic_store_rel_int(&ithd->it_need, 0); + atomic_thread_fence_acq_rel(); if (priv) priv_ithread_execute_handler(p, ih); else