From owner-svn-src-projects@FreeBSD.ORG Fri May 18 12:43:09 2012 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 78105106566B; Fri, 18 May 2012 12:43:09 +0000 (UTC) (envelope-from mjg@semihalf.com) Received: from smtp.semihalf.com (smtp.semihalf.com [213.17.239.109]) by mx1.freebsd.org (Postfix) with ESMTP id E7D478FC0A; Fri, 18 May 2012 12:43:08 +0000 (UTC) Received: from localhost (unknown [213.17.239.109]) by smtp.semihalf.com (Postfix) with ESMTP id A3CC5D503C; Fri, 18 May 2012 14:42:52 +0200 (CEST) X-Virus-Scanned: by amavisd-new at semihalf.com Received: from smtp.semihalf.com ([213.17.239.109]) by localhost (smtp.semihalf.com [213.17.239.109]) (amavisd-new, port 10024) with ESMTP id inacw0MvqEop; Fri, 18 May 2012 14:42:51 +0200 (CEST) Received: from pcbsd-2342.semihalf.com (cardhu.semihalf.com [213.17.239.108]) by smtp.semihalf.com (Postfix) with ESMTPSA id BD1E9D5009; Fri, 18 May 2012 14:42:51 +0200 (CEST) Date: Fri, 18 May 2012 14:43:00 +0200 From: Mateusz Guzik To: Konstantin Belousov Message-ID: <20120518124259.GB23560@pcbsd-2342.semihalf.com> References: <4FA8FFB9.7090002@freebsd.org> <20120508095631.GV2358@deviant.kiev.zoral.com.ua> <4FA94609.3060306@freebsd.org> <20120510103105.GG2358@deviant.kiev.zoral.com.ua> <4FABC64F.3060502@freebsd.org> <20120510115857.GH2358@deviant.kiev.zoral.com.ua> <20120510164519.GA13258@pcbsd-2342.semihalf.com> <20120511104648.GM2358@deviant.kiev.zoral.com.ua> <20120511154810.GA99005@pcbsd-2342.semihalf.com> <20120517222946.GA2358@deviant.kiev.zoral.com.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20120517222946.GA2358@deviant.kiev.zoral.com.ua> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-projects@freebsd.org, Grzegorz Bernacki , 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: Fri, 18 May 2012 12:43:09 -0000 On Fri, May 18, 2012 at 01:29:46AM +0300, Konstantin Belousov wrote: > On Fri, May 11, 2012 at 05:48:10PM +0200, Mateusz Guzik wrote: > > On Fri, May 11, 2012 at 01:46:48PM +0300, Konstantin Belousov wrote: > > > On Thu, May 10, 2012 at 06:45:19PM +0200, Mateusz Guzik wrote: > > > > http://people.freebsd.org/~raj/patches/misc/vfs_highdirtybuf.diff > > > > > > > > callbacks are expected to increase flushed counter if they happend to > > > > flush some buffers. > > > I do not think this is right. You need to call a routine when getnewblk() > > > is unable to find a buffer to recycle. > > > > > > As I understand, in your situation with lot of managed buffers, the dirty > > > queue could be just empty. > > > > I don't think I follow. Is your concern that with a lot of managed buffers and > > empty dirty queue nothing can be recycled? Or that managed buffers > > affect calls to buf_do_flush? > > > > I presume you talk about the code below the following: > > /* > > * If we exhausted our list, sleep as appropriate. We may have to > > * wakeup various daemons and write out some dirty buffers. > > * > > * Generally we are sleeping due to insufficient buffer space. > > */ > > > > if (bp == NULL) { > > > > Conditions that cause bufdaemon weakups/buf_do_flush calls seem to be > > not dependent on buffers being managed or not. > My concern is that you use unappropriate terminology at first, and this > leads to mostly no-op code as a consequence. > > If you use managed buffers, not only you should flush dirty buffers when > bufdaemon is kicked, but you also need to actually return some buffers > to the clean or empty queues. Otherwise, the condition which caused > getnewbuf() allocation failure will persist. We clear B_MANAGED flag before calling bufwrite and this requeues our buffers. > > > > buf_do_flush with our change always calls registered callbacks and the > > callback that we register calls code that knowns what nandfs managed > > buffers are dirty and flushes those. Buffers from dirty queues (if any) > > are flushed separately. > > > > If I'm wrong or misunderstood you, please elaborate on problem you have > > in mind. > > > > > > > > > > Example proof-of-concept (will be cleaned up) change for nandfs: > > > > http://people.freebsd.org/~raj/patches/misc/nandfs_vfs_highdirtybuf.diff > > > > > > > > Does this look reasonable? > > The vfs_bio.c change is probably fine, but I have doubts that nandfs patch > will change anything for the reasons I described above. On the other hand, > I did not read nandfs code, so I may be wrong. To sum up, I believe that nandfs with this patch correctly reacts to demands from bufdaemon and getnewbuf by flushing and requeieng buffers. Do you have any futher comments? -- Mateusz Guzik