Date: Thu, 16 Sep 2004 15:53:07 +0200 From: Uwe Doering <gemini@geminix.org> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/71792: Wrong/missing 'goto' target label in contigmalloc1() Message-ID: <E1C7wh9-0007PJ-00@geminix.org> Resent-Message-ID: <200409161400.i8GE0mWI015842@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 71792 >Category: kern >Synopsis: Wrong/missing 'goto' target label in contigmalloc1() >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Sep 16 14:00:48 GMT 2004 >Closed-Date: >Last-Modified: >Originator: Uwe Doering >Release: FreeBSD 4.10-STABLE i386 >Organization: EscapeBox - Managed On-Demand UNIX Servers http://www.escapebox.net >Environment: System: FreeBSD geminix.org 4.10-STABLE FreeBSD 4.10-STABLE #1: Tue Sep 14 20:12:02 GMT 2004 root@localhost:/RELENG_4_Enhanced i386 >Description: There are two loops in contigmalloc1() (src/sys/vm/vm_page.c) that try to make room in the memory arena if there was no page available on first try that would fit the applying constraints. The first loop scans PQ_INACTIVE, the second one PQ_ACTIVE. Now, the second loop apparently has come into existence by means of a cut-and-paste operation. Unfortunately, the author of the code segment forgot to give that loop its own 'goto' target label, to be used in start-over situations. Instead, the 'goto' statements jump back into the PQ_INACTIVE loop, possibly over and over again. >How-To-Repeat: The problem becomes apparent when looking at the relevant source code. >Fix: Give the PQ_ACTIVE loop its own start-over target label. Please consider adopting the patch below. --- vm_page.c.diff begins here --- --- src/sys/vm/vm_page.c.orig Wed Jan 28 22:24:01 2004 +++ src/sys/vm/vm_page.c Sat May 8 20:48:22 2004 @@ -1892,6 +1894,7 @@ vm_page_cache(m); } +again2: for (m = TAILQ_FIRST(&vm_page_queues[PQ_ACTIVE].pl); m != NULL; m = next) { @@ -1901,18 +1904,18 @@ next = TAILQ_NEXT(m, pageq); if (vm_page_sleep_busy(m, TRUE, "vpctw1")) - goto again1; + goto again2; vm_page_test_dirty(m); if (m->dirty) { if (m->object->type == OBJT_VNODE) { vn_lock(m->object->handle, LK_EXCLUSIVE | LK_RETRY, curproc); vm_object_page_clean(m->object, 0, 0, OBJPC_SYNC); VOP_UNLOCK(m->object->handle, 0, curproc); - goto again1; + goto again2; } else if (m->object->type == OBJT_SWAP || m->object->type == OBJT_DEFAULT) { vm_pageout_flush(&m, 1, 0); - goto again1; + goto again2; } } if ((m->dirty == 0) && (m->busy == 0) && (m->hold_count == 0)) --- vm_page.c.diff ends here --- >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E1C7wh9-0007PJ-00>