svn commit: r233072 - projects/nand/sys/kern

[Mateusz Guzik]

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 <mjg semihalf.com>