From owner-svn-src-head@freebsd.org Tue Sep 26 23:24:16 2017 Return-Path: Delivered-To: svn-src-head@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 BD341E254ED; Tue, 26 Sep 2017 23:24:16 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::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 8824B7F9FA; Tue, 26 Sep 2017 23:24:16 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v8QNOF03009599; Tue, 26 Sep 2017 23:24:15 GMT (envelope-from jhb@FreeBSD.org) Received: (from jhb@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v8QNOFCZ009598; Tue, 26 Sep 2017 23:24:15 GMT (envelope-from jhb@FreeBSD.org) Message-Id: <201709262324.v8QNOFCZ009598@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: jhb set sender to jhb@FreeBSD.org using -f From: John Baldwin Date: Tue, 26 Sep 2017 23:24:15 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r324039 - head/sys/ufs/ffs X-SVN-Group: head X-SVN-Commit-Author: jhb X-SVN-Commit-Paths: head/sys/ufs/ffs X-SVN-Commit-Revision: 324039 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Sep 2017 23:24:16 -0000 Author: jhb Date: Tue Sep 26 23:24:15 2017 New Revision: 324039 URL: https://svnweb.freebsd.org/changeset/base/324039 Log: Don't defer wakeup()s for completed journal workitems. Normally wakeups() are performed for completed softupdates work items in workitem_free() before the underlying memory is free()'d. complete_jseg() was clearing the "wakeup needed" flag in work items to defer the wakeup until the end of each loop iteration. However, this resulted in the item being free'd before it's address was used with wakeup(). As a result, another part of the kernel could allocate this memory from malloc() and use it as a wait channel for a different "event" with a different lock. This triggered an assertion failure when the lock passed to sleepq_add() did not match the existing lock associated with the sleep queue. Fix this by removing the code to defer the wakeup in complete_jseg() allowing the wakeup to occur slightly earlier in workitem_free() before free() is called. The main reason I can think of for deferring a wakeup() would be to avoid waking up a waiter while holding a lock that the waiter would need. However, no locks are dropped in between the wakeup() in workitem_free() and the end of the loop in complete_jseg() as far as I can tell. In general I think it is not safe to do a wakeup() after free() as one cannot control how other parts of the kernel that might reuse the address for a different wait channel will handle spurious wakeups. Reported by: pho Reviewed by: kib MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D12494 Modified: head/sys/ufs/ffs/ffs_softdep.c Modified: head/sys/ufs/ffs/ffs_softdep.c ============================================================================== --- head/sys/ufs/ffs/ffs_softdep.c Tue Sep 26 23:23:58 2017 (r324038) +++ head/sys/ufs/ffs/ffs_softdep.c Tue Sep 26 23:24:15 2017 (r324039) @@ -3596,15 +3596,13 @@ complete_jseg(jseg) { struct worklist *wk; struct jmvref *jmvref; - int waiting; #ifdef INVARIANTS int i = 0; #endif while ((wk = LIST_FIRST(&jseg->js_entries)) != NULL) { WORKLIST_REMOVE(wk); - waiting = wk->wk_state & IOWAITING; - wk->wk_state &= ~(INPROGRESS | IOWAITING); + wk->wk_state &= ~INPROGRESS; wk->wk_state |= COMPLETE; KASSERT(i++ < jseg->js_cnt, ("handle_written_jseg: overflow %d >= %d", @@ -3645,8 +3643,6 @@ complete_jseg(jseg) TYPENAME(wk->wk_type)); /* NOTREACHED */ } - if (waiting) - wakeup(wk); } /* Release the self reference so the structure may be freed. */ rele_jseg(jseg);