From owner-freebsd-current@FreeBSD.ORG Wed Sep 28 22:24:57 2005 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 9B8F216A41F for ; Wed, 28 Sep 2005 22:24:57 +0000 (GMT) (envelope-from peadar.edwards@gmail.com) Received: from zproxy.gmail.com (zproxy.gmail.com [64.233.162.202]) by mx1.FreeBSD.org (Postfix) with ESMTP id DED8843D48 for ; Wed, 28 Sep 2005 22:24:52 +0000 (GMT) (envelope-from peadar.edwards@gmail.com) Received: by zproxy.gmail.com with SMTP id z31so120664nzd for ; Wed, 28 Sep 2005 15:24:51 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:reply-to:to:subject:mime-version:content-type:content-transfer-encoding:content-disposition; b=ibmnsm52kf9mUV23N13B667TL/QvzIN5FVX1JJCyfp2bkm1yz1k5XN7IduQYAH3s9siyeLSg41r/poIOJw6H+MIEr1QUff8pqBRtkdd+Ia8G/BZJemey1hf6qvoMH4YRajilTtJkUL7aqy4P8qz52zkz0TwK+omXcTrWFNfFaeQ= Received: by 10.36.13.11 with SMTP id 11mr4006517nzm; Wed, 28 Sep 2005 15:24:51 -0700 (PDT) Received: by 10.36.68.16 with HTTP; Wed, 28 Sep 2005 15:24:51 -0700 (PDT) Message-ID: <34cb7c8405092815247dc89bf6@mail.gmail.com> Date: Wed, 28 Sep 2005 23:24:51 +0100 From: Peter Edwards To: freebsd-current@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Subject: biodone panics X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Peter Edwards List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Sep 2005 22:24:57 -0000 There's a few people complaining about double-frees (and other crashes) in biodone(), and I just banged my head off it too: They generally look a little like this: > panic(c0870cfb,c1b0c4a4,c143f000,c0850ced,c0870cdf) at panic+0x127 (where c0870cfb =3D=3D "duplicate free of item %p from zone %p(%s)" > uma_dbg_free(c143f000,0,c1b0c4a4) at uma_dbg_free+0x110 > uma_zfree_arg(c143f000,c1b0c4a4,0) at uma_zfree_arg+0x66 > g_destroy_bio(c1b0c4a4) at g_destroy_bio+0x13 > g_vfs_done(c1b0c4a4) at g_vfs_done+0x5a > biodone(c1b0c4a4,ca0bccc4,0,c0850cb0,1e4) at biodone+0x57 > g_io_schedule_up(c1833300) at g_io_schedule_up+0xb5 > g_up_procbody(0,ca0bcd38,0,c05fed08,0) at g_up_procbody+0x5a > fork_exit(c05fed08,0,ca0bcd38) at fork_exit+0xa0 > fork_trampoline() at fork_trampoline+0x8 Am I just being dense, or is there a race condition in biodone(): > void > biodone(struct bio *bp) > { > > mtx_lock(&bdonelock); > bp->bio_flags |=3D BIO_DONE; > if (bp->bio_done =3D=3D NULL) > wakeup(bp); > mtx_unlock(&bdonelock); > if (bp->bio_done !=3D NULL) > bp->bio_done(bp); > } the call to wakeup may set in motion some events that cause the bio to be freed. By the time the mtx_unlock completes, "bp" could point to an invalid bio, and the "if (bp->bio_done !=3D NULL)" is bogus. True? Shouldn't it be > biodone(struct bio *bp) > { > void (*done)(struct bio *); > > mtx_lock(&bdonelock); > bp->bio_flags |=3D BIO_DONE; > done =3D bp->bio_done > if (done =3D=3D NULL) > wakeup(bp); > mtx_unlock(&bdonelock); > if (done !=3D NULL) > bp->bio_done(bp); > } Anyone agree?