From owner-freebsd-hackers  Mon Jan 18 17:28:48 1999
Return-Path: <owner-freebsd-hackers@FreeBSD.ORG>
Received: (from majordom@localhost)
          by hub.freebsd.org (8.8.8/8.8.8) id RAA17554
          for freebsd-hackers-outgoing; Mon, 18 Jan 1999 17:28:48 -0800 (PST)
          (envelope-from owner-freebsd-hackers@FreeBSD.ORG)
Received: from apollo.backplane.com (apollo.backplane.com [209.157.86.2])
          by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id RAA17525;
          Mon, 18 Jan 1999 17:28:43 -0800 (PST)
          (envelope-from dillon@apollo.backplane.com)
Received: (from dillon@localhost)
	by apollo.backplane.com (8.9.1/8.9.1) id RAA84794;
	Mon, 18 Jan 1999 17:28:35 -0800 (PST)
	(envelope-from dillon)
Date: Mon, 18 Jan 1999 17:28:35 -0800 (PST)
From: Matthew Dillon <dillon@apollo.backplane.com>
Message-Id: <199901190128.RAA84794@apollo.backplane.com>
To: "John S. Dyson" <root@dyson.iquest.net>
Cc: dg@root.com, jkh@FreeBSD.ORG, hackers@FreeBSD.ORG
Subject: Re: Found problem w/ Paging performance over NFS
References:  <199901190120.UAA27272@dyson.iquest.net>
Sender: owner-freebsd-hackers@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG


:> 
:>     in kern/vfs_bio.c, allocbuf()
:> 
:> 	    } else if (m->flags & PG_BUSY) {
:> 		    s = splvm();
:> 		    if (m->flags & PG_BUSY) {
:> 			    vm_page_flag_set(m, PG_WANTED);
:> 			    tsleep(m, PVM, "pgtblk", 0);
:> 		    }  
:> 		    splx(s);
:> 		    goto doretry;
:> 	    } else {
:> 		    ...
:> 		    if (tinc > (newbsize - toff))
:> 			    tinc = newbsize - toff;
:> 		    if (bp->b_flags & B_CACHE)
:> 			    vfs_buf_set_valid(bp, off, toff, tinc, m);
:> 		    vm_page_flag_clear(m, PG_ZERO);
:> 		    vm_page_wire(m);
:> 	    }
:> 
:>     Shouldn't those conditionals be 'm->busy || (m->flags & PG_BUSY)' instead
:>     of just testing against PG_BUSY?  A pageout operation will set m->busy
:>     without setting PG_BUSY.
:> 
:The existing code is okay.  Pageouts don't need to set PG_BUSY.  Also, *generally*
:during buffer cache read write operations, the PG_BUSY flag doesn't need to be set
:during the *entire* operation.  Think filesystems with block sizes < PAGE_SIZE
:as an example.
:
:PG_BUSY is quite severe, and not always needed.
:
:John

    Ok.  I just figured out the interaction -- in fact, we *can't* test m->busy
    in this section of code because of the following sequence:

	vm_fault -> vnode_pager_getpages -> ffs_getpages -> ffs_read -> 
	    cluster_read -> getblk -> allocbuf ( lockup )

    ffs_getpages converts the PG_BUSY into a vm_page_t->busy before getting into
    the cluster stuff, and thus allocbuf() deep down cannot sleep on
    vm_page_t->busy.

    Ick.  I'm going to add a comment explaining that case.

					-Matt

					Matthew Dillon 
					<dillon@backplane.com>

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message