From owner-svn-src-projects@FreeBSD.ORG Thu May 10 10:31:25 2012 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 3D91C106566C; Thu, 10 May 2012 10:31:25 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 9AF968FC08; Thu, 10 May 2012 10:31:24 +0000 (UTC) Received: from skuns.kiev.zoral.com.ua (localhost [127.0.0.1]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id q4AAV6jM017279; Thu, 10 May 2012 13:31:06 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5) with ESMTP id q4AAV6gs090596; Thu, 10 May 2012 13:31:06 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5/Submit) id q4AAV6rW090595; Thu, 10 May 2012 13:31:06 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 10 May 2012 13:31:05 +0300 From: Konstantin Belousov To: Grzegorz Bernacki Message-ID: <20120510103105.GG2358@deviant.kiev.zoral.com.ua> References: <201203170318.q2H3ITdI047893@svn.freebsd.org> <20120317085116.GC1340@garage.freebsd.pl> <20120317161050.GI75778@deviant.kiev.zoral.com.ua> <4FA8FFB9.7090002@freebsd.org> <20120508095631.GV2358@deviant.kiev.zoral.com.ua> <4FA94609.3060306@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="i0oNXV9PwusX79ef" Content-Disposition: inline In-Reply-To: <4FA94609.3060306@freebsd.org> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.0 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: svn-src-projects@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r233072 - projects/nand/sys/kern X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 May 2012 10:31:25 -0000 --i0oNXV9PwusX79ef Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, May 08, 2012 at 06:12:57PM +0200, Grzegorz Bernacki wrote: > On 05/08/12 11:56, Konstantin Belousov wrote: > >On Tue, May 08, 2012 at 01:12:57PM +0200, Grzegorz Bernacki wrote: > >>On 03/17/12 17:10, Konstantin Belousov wrote: > >>>On Sat, Mar 17, 2012 at 09:51:16AM +0100, Pawel Jakub Dawidek wrote: > >>>>On Sat, Mar 17, 2012 at 03:18:29AM +0000, Grzegorz Bernacki wrote: > >>>>>Author: gber > >>>>>Date: Sat Mar 17 03:18:28 2012 > >>>>>New Revision: 233072 > >>>>>URL: http://svn.freebsd.org/changeset/base/233072 > >>>>> > >>>>>Log: > >>>>> Add VFS changes necessary for NANDFS to work. > >>>>> > >>>>> Ignore B_MANAGED buffer by syncer and ignore signal when msleep a= s=20 > >>>>> it > >>>>> can cause file system inconsistency. > >>>> > >>>>I'd suggest running these changes through kib@. Especially > >>>>vn_start_write() > >>>>change below looks ugly, but maybe it is only temporary? > >>>It is not only ugly (and object against it). > >>> > >>>If the change makes any difference for the filesystem, then I just arg= ue > >>>that the filesystem is broken. The vn_start_write() is done on the > >>>VFS entry peripheral, long before filesystem code is hit. > >>> > >>>I did not looked at the managed changes, you would need to describe > >>>what is wrong with current code and what is the purpose of the changes. > >>>B_MANAGED came from xfs, it seems, or at least xfs is the only current > >>>consumer of B_MANAGED buffers. > >> > >>Hi Kostik, > >> > >>Without our change in getblk() whenewer we allocate new block we get=20 > >>panic: > >> > >>panic: bremfree: buffer 0xffffff807bf86080 not on a queue. > >> > >>It is because blocks with B_MANAGED flag are not queued on any queue in > >>brelse() function. Could you look at it and give us approval to merge > >>this change into HEAD? > > > >Right, but this is in fact the only function of the B_MANAGED flag. > >So the question is, what are you trying to accomplish. >=20 > Hi Kostik, >=20 > There are two separate issues. > First is that if we have B_MANAGED flag it should not cause the panic=20 > when used, so in my opinion fix should go into HEAD regardless of NANDFS. > Second thing is the reason of having B_MANAGED flag. We use it because=20 > we want to decide when and where to save buffers. Our FS is log=20 > filesystem and we write buffers every given amount of time or if number= =20 > of dirty buffer exceed given threshold. We write buffers in large groups= =20 > along with metadata related to this group, so we cannot afford writing=20 > single buffer. As a result we cannot allow buf deamon to write buffers= =20 > in an ad hoc manner. > I hope I answered your question. Please let me know if you have more=20 > concerns. If you have some ideas how we can avoid using B_MANAGED flags= =20 > please let us know. I looked at the whole B_MANAGED business over the two days. Finally I think that I agree to some extent with the idea of the patch, but I still do not like details. I put below the change that I think we are discussing. Index: vfs_bio.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- vfs_bio.c (.../head/sys/kern/vfs_bio.c) (revision 235215) +++ vfs_bio.c (.../projects/nand/sys/kern/vfs_bio.c) (revision 235215) @@ -2672,7 +2672,8 @@ loop: else if ((bp->b_flags & (B_VMIO | B_INVAL)) =3D=3D 0) bp->b_flags |=3D B_CACHE; BO_LOCK(bo); - bremfree(bp); + if (!(bp->b_flags & B_MANAGED)) + bremfree(bp); BO_UNLOCK(bo); =20 /* First note, you should not take BO_LOCK if you are not going to call bremfree().Second, I think you need to assert that b_qindex is already QUEUE_NONE if B_MANAGED is set. Note the comment right after the if (bp !=3D NULL) line in getblk(). I think it needs to be adjusted to say 'if not managed' or like. As a general note, explaining my limited disagreement with the whole idea of using managed buffers, is the question how do you react to low free buffer condition at all ? Do you get the notification on the condition at all ? Current scheme involves activation of the bufdaemon when getnewbuf() cannot find a suitable buffer or KVA. With a non-trivial count of buffers not presented on any queue, system has high risk of deadlocking. --i0oNXV9PwusX79ef Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (FreeBSD) iEYEARECAAYFAk+rmOkACgkQC3+MBN1Mb4gTuQCgwCvJ1Oi1ULSzBH6AOdVE8RLF 6OEAn2bQlcPDM2TrpS1B8u8ip1TMalZp =reWJ -----END PGP SIGNATURE----- --i0oNXV9PwusX79ef--