From owner-svn-src-all@FreeBSD.ORG Wed May 20 16:01:07 2015 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id CDDF4451; Wed, 20 May 2015 16:01:07 +0000 (UTC) Received: from mail-qg0-x22d.google.com (mail-qg0-x22d.google.com [IPv6:2607:f8b0:400d:c04::22d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 8772C1C6C; Wed, 20 May 2015 16:01:07 +0000 (UTC) Received: by qgez61 with SMTP id z61so21897113qge.1; Wed, 20 May 2015 09:01:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type; bh=6P6fBblZujJpep52Lkjj9uiBNag3GR/XCBP4Y3wcVCY=; b=eAqBHe+h0v4R2h3ZmfJID0lqtOZRAu9ul8kUCC4Zo3kuQRmVJvG9ra+jkZGdX7I1KI tqFtJN8p8OubM38jL+0KZjBzGEx5kOEhamzoHDKyrQ67zsFoXrRatL0r3dmR1PVsN/Dz iycMc/4aHpIJa8PC/qnIcuImwiOoSBLv9rNUw8nzc4zJKAMPNx2HHD5c7fYFJQUgBpSj 7RNogWNzhQ9BeoK7h5iwu+TL1nojsxBIkIrKNCEdG8oUdXM0Lc3T9PTVR2A9eFmTmFb1 nZBO5cZuOWvth1h5yfgA1+xRSB0OD0W8c/vSs9TGm8YLmIVhNp9Mw9tolNVZWmQCBbm6 hK/w== X-Received: by 10.140.233.140 with SMTP id e134mr46616379qhc.63.1432137666574; Wed, 20 May 2015 09:01:06 -0700 (PDT) Received: from kan ([2601:6:6780:7e0:226:18ff:fe00:232e]) by mx.google.com with ESMTPSA id o4sm11472719qko.49.2015.05.20.09.01.05 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 May 2015 09:01:05 -0700 (PDT) Date: Wed, 20 May 2015 12:00:46 -0400 From: Alexander Kabaev To: John Baldwin Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r282971 - in head/sys: kern sys Message-ID: <20150520120046.268dde86@kan> In-Reply-To: <201505151350.t4FDocQT054144@svn.freebsd.org> References: <201505151350.t4FDocQT054144@svn.freebsd.org> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.27; amd64-portbld-freebsd11.0) MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; boundary="Sig_/alw=CGttxZ_+MlnkAqn6sCP"; protocol="application/pgp-signature" 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, 20 May 2015 16:01:08 -0000 --Sig_/alw=CGttxZ_+MlnkAqn6sCP Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Fri, 15 May 2015 13:50:38 +0000 (UTC) John Baldwin wrote: > Author: jhb > Date: Fri May 15 13:50:37 2015 > New Revision: 282971 > URL: https://svnweb.freebsd.org/changeset/base/282971 >=20 > Log: > Previously, cv_waiters was only updated by cv_signal or cv_wait. If > a thread awakened due to a time out, then cv_waiters was not > decremented. If INT_MAX threads timed out on a cv without an > intervening cv_broadcast, then cv_waiters could overflow. To fix > this, have each sleeping thread decrement cv_waiters when it resumes. > =20 > Note that previously cv_waiters was protected by the sleepq chain > lock. However, that lock is not held when threads resume from sleep. > In addition, the interlock is also not always reacquired after > resuming (cv_wait_unlock), nor is it always held by callers of > cv_signal() or cv_broadcast(). Instead, use atomic ops to update > cv_waiters. Since the sleepq chain lock is still held on every > increment, it should still be safe to compare cv_waiters against zero > while holding the lock in the wakeup routines as the only way the > race should be lost would result in extra calls to sleepq_signal() or > sleepq_broadcast().=20 > Differential Revision: https://reviews.freebsd.org/D2427 > Reviewed by: benno > Reported by: benno (wrap of cv_waiters in the field) > MFC after: 2 weeks >=20 > Modified: > head/sys/kern/kern_condvar.c > head/sys/sys/condvar.h >=20 This breaks ZFS range locking code, which expects to be able to wakeup everyone on the condition variable and then free the structure that contains it. Having woken up threads modify cv_waiters results in a race that leads to already freed memory to be accessed. It is debatable just how correct ZFS code in its expectations, but I think this commit should probably be reverted until either ZFS is changed not to expect cv modifiable by waking threads or until alternative solution is found to the cv_waiters overflow issue fixed by this commit. --=20 Alexander Kabaev --Sig_/alw=CGttxZ_+MlnkAqn6sCP Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlVcr68ACgkQQ6z1jMm+XZYUBQCgoDBcawgQ7bMmkMGelCj7UfDm y4QAniQ2Vyyzzhc5s+8uwWeAYyZrDwdm =92dI -----END PGP SIGNATURE----- --Sig_/alw=CGttxZ_+MlnkAqn6sCP--