From owner-p4-projects@FreeBSD.ORG Sat May 13 01:07:22 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 3E72516A42F; Sat, 13 May 2006 01:07:22 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 01FB916A407 for ; Sat, 13 May 2006 01:07:22 +0000 (UTC) (envelope-from jb@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 8D9D443D48 for ; Sat, 13 May 2006 01:07:21 +0000 (GMT) (envelope-from jb@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.6/8.13.6) with ESMTP id k4D17KFR021803 for ; Sat, 13 May 2006 01:07:20 GMT (envelope-from jb@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.6/8.13.4/Submit) id k4D17KY6021798 for perforce@freebsd.org; Sat, 13 May 2006 01:07:20 GMT (envelope-from jb@freebsd.org) Date: Sat, 13 May 2006 01:07:20 GMT Message-Id: <200605130107.k4D17KY6021798@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to jb@freebsd.org using -f From: John Birrell To: Perforce Change Reviews Cc: Subject: PERFORCE change 97045 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 May 2006 01:07:22 -0000 http://perforce.freebsd.org/chv.cgi?CH=97045 Change 97045 by jb@jb_freebsd2 on 2006/05/13 01:06:55 I must be making progress because Sun's DTrace tests are now finding my hacks. Blush. It's time to handle the memory allocation for the buffers properly. Respect the request to allocate buffers for all CPUs or just one. If allocating buffers for all CPUs, then respect the fact that the CPU address space is sparse. This allows us to pass all but two of Sun's DTrace buffer tests. One that fails requires the builtin variable 'curthread', so that's not a buffer-specific problem. The second one causes a deadlock in the kernel witout a panic, so that's a little bit of a, umm, err, problem. Sigh. Affected files ... .. //depot/projects/dtrace/src/cddl/test/dtrace/Makefile#3 edit .. //depot/projects/dtrace/src/sys/cddl/dev/dtrace/dtrace_buffer.c#6 edit .. //depot/projects/dtrace/src/sys/cddl/dev/dtrace/dtrace_dynvar.c#4 edit .. //depot/projects/dtrace/src/sys/cddl/dev/dtrace/dtrace_state.c#8 edit Differences ... ==== //depot/projects/dtrace/src/cddl/test/dtrace/Makefile#3 (text+ko) ==== @@ -156,6 +156,22 @@ ${.CURDIR}/tst/common/bitfields/tst.BitFieldPromotion.d \ ${.CURDIR}/tst/common/bitfields/tst.SizeofBitField.d +TESTBUFFERING= \ + ${.CURDIR}/tst/common/buffering/err.end.d \ + ${.CURDIR}/tst/common/buffering/err.resize1.d \ + ${.CURDIR}/tst/common/buffering/err.resize2.d \ + ${.CURDIR}/tst/common/buffering/err.resize3.d \ + ${.CURDIR}/tst/common/buffering/err.zerobuf.d \ + ${.CURDIR}/tst/common/buffering/tst.dynvarsize.d \ + ${.CURDIR}/tst/common/buffering/tst.fill1.d \ + ${.CURDIR}/tst/common/buffering/tst.resize1.d \ + ${.CURDIR}/tst/common/buffering/tst.resize2.d \ + ${.CURDIR}/tst/common/buffering/tst.resize3.d \ + ${.CURDIR}/tst/common/buffering/tst.ring1.d \ + ${.CURDIR}/tst/common/buffering/tst.ring2.d \ + ${.CURDIR}/tst/common/buffering/tst.smallring.d \ + ${.CURDIR}/tst/common/buffering/tst.switch1.d + TESTBUILTINVAR= \ ${.CURDIR}/tst/common/builtinvar/tst.arg0.d \ ${.CURDIR}/tst/common/builtinvar/tst.arg0clause.d \ @@ -1047,24 +1063,9 @@ # -------------------------------------------------------------------------------- # Tests that either cause a system lock or a panic: -TESTBUFFERING= \ - ${.CURDIR}/tst/common/buffering/err.end.d \ - ${.CURDIR}/tst/common/buffering/err.resize1.d \ - ${.CURDIR}/tst/common/buffering/err.resize2.d \ - ${.CURDIR}/tst/common/buffering/err.resize3.d \ - ${.CURDIR}/tst/common/buffering/err.zerobuf.d \ +KABOOM= \ ${.CURDIR}/tst/common/buffering/tst.alignring.d \ ${.CURDIR}/tst/common/buffering/tst.cputime.ksh \ - ${.CURDIR}/tst/common/buffering/tst.dynvarsize.d \ - ${.CURDIR}/tst/common/buffering/tst.fill1.d \ - ${.CURDIR}/tst/common/buffering/tst.resize1.d \ - ${.CURDIR}/tst/common/buffering/tst.resize2.d \ - ${.CURDIR}/tst/common/buffering/tst.resize3.d \ - ${.CURDIR}/tst/common/buffering/tst.ring1.d \ - ${.CURDIR}/tst/common/buffering/tst.ring2.d \ - ${.CURDIR}/tst/common/buffering/tst.ring3.d \ - ${.CURDIR}/tst/common/buffering/tst.smallring.d \ - ${.CURDIR}/tst/common/buffering/tst.switch1.d # -------------------------------------------------------------------------------- # The normal test targets: @@ -1076,6 +1077,7 @@ ${TESTASSOCS} \ ${TESTBEGIN} \ ${TESTBITFIELDS} \ + ${TESTBUFFERING} \ ${TESTBUILTINVAR} \ ${TESTCLAUSES} \ ${TESTDECLS} \ @@ -1292,6 +1294,7 @@ REQUIRES_CURTHREAD= \ ${.CURDIR}/tst/common/assocs/tst.orthogonality.d \ + ${.CURDIR}/tst/common/buffering/tst.ring3.d \ ${.CURDIR}/tst/common/scalars/tst.selfarray2.d \ ${.CURDIR}/tst/common/types/tst.complex.d @@ -1328,6 +1331,9 @@ REQUIRES_ROOTFS= \ ${.CURDIR}/tst/common/printf/tst.str.d +REQUIRES_SCHED= \ + ${.CURDIR}/tst/common/buffering/tst.cputime.ksh + REQUIRES_TCP_T= \ ${.CURDIR}/tst/common/offsetof/err.D_OFFSETOF_BITFIELD.bitfield.d ==== //depot/projects/dtrace/src/sys/cddl/dev/dtrace/dtrace_buffer.c#6 (text+ko) ==== @@ -69,9 +69,9 @@ dtrace_buffer_alloc(dtrace_buffer_t *bufs, size_t size, int flags, processorid_t cpu) { -#if defined(sun) cpu_t *cp; dtrace_buffer_t *buf; +#if defined(sun) ASSERT(MUTEX_HELD(&cpu_lock)); ASSERT(MUTEX_HELD(&dtrace_lock)); @@ -144,16 +144,18 @@ return (ENOMEM); #else - dtrace_buffer_t *buf; int i; ASSERT(MUTEX_HELD(&dtrace_lock)); for (i = 0; i <= mp_maxid; i++) { - buf = &bufs[i]; + if ((cp = pcpu_find(i)) == NULL) + continue; - if (CPU_ABSENT(i)) + if (cpu != DTRACE_CPUALL && cpu != i) continue; + buf = &bufs[i]; + /* * If there is already a buffer allocated for this CPU, it * is only possible that this is a DR event. In this case, @@ -166,27 +168,55 @@ ASSERT(buf->dtb_xamot == NULL); - /* - * XXX Hack. - * This should be KM_NOSLEEP with error handling if low on - * memory. - */ - buf->dtb_tomax = kmem_zalloc(size, KM_SLEEP); + if ((buf->dtb_tomax = kmem_zalloc(size, KM_NOSLEEP)) == NULL) + goto err; buf->dtb_size = size; buf->dtb_flags = flags; buf->dtb_offset = 0; buf->dtb_drops = 0; - /* - * XXX Hack. - * This should be KM_NOSLEEP with error handling if low on - * memory. - */ - buf->dtb_xamot = kmem_zalloc(size, KM_SLEEP); + if (flags & DTRACEBUF_NOSWITCH) + continue; + + if ((buf->dtb_xamot = kmem_zalloc(size, KM_NOSLEEP)) == NULL) + goto err; } return (0); + +err: + /* + * Error allocating memory, so free the buffers that were + * allocated before the failed allocation. + */ + for (i = 0; i <= mp_maxid; i++) { + if ((cp = pcpu_find(i)) == NULL) + continue; + + if (cpu != DTRACE_CPUALL && cpu != i) + continue; + + buf = &bufs[i]; + + if (buf->dtb_xamot != NULL) { + ASSERT(buf->dtb_tomax != NULL); + ASSERT(buf->dtb_size == size); + kmem_free(buf->dtb_xamot, size); + } + + if (buf->dtb_tomax != NULL) { + ASSERT(buf->dtb_size == size); + kmem_free(buf->dtb_tomax, size); + } + + buf->dtb_tomax = NULL; + buf->dtb_xamot = NULL; + buf->dtb_size = 0; + + } + + return (ENOMEM); #endif } @@ -494,6 +524,11 @@ int i; for (i = 0; i < NCPU; i++) { +#if !defined(sun) + if (pcpu_find(i) == NULL) + continue; +#endif + dtrace_buffer_t *buf = &bufs[i]; if (buf->dtb_tomax == NULL) { ==== //depot/projects/dtrace/src/sys/cddl/dev/dtrace/dtrace_dynvar.c#4 (text+ko) ==== @@ -13,6 +13,10 @@ int i, work = 0; for (i = 0; i < NCPU; i++) { +#if !defined(sun) + if (CPU_ABSENT(i)) + continue; +#endif dcpu = &dstate->dtds_percpu[i]; ASSERT(dcpu->dtdsc_rinsing == NULL); @@ -63,6 +67,10 @@ dtrace_sync(); for (i = 0; i < NCPU; i++) { +#if !defined(sun) + if (CPU_ABSENT(i)) + continue; +#endif dcpu = &dstate->dtds_percpu[i]; if (dcpu->dtdsc_rinsing == NULL) ==== //depot/projects/dtrace/src/sys/cddl/dev/dtrace/dtrace_state.c#8 (text+ko) ==== @@ -63,6 +63,10 @@ maxper = (maxper / dstate->dtds_chunksize) * dstate->dtds_chunksize; for (i = 0; i < NCPU; i++) { +#if !defined(sun) + if (CPU_ABSENT(i)) + continue; +#endif dstate->dtds_percpu[i].dtdsc_free = dvar = start; /* @@ -408,7 +412,7 @@ dtrace_state_buffer(dtrace_state_t *state, dtrace_buffer_t *buf, int which) { dtrace_optval_t *opt = state->dts_options, size; - processorid_t cpu; + processorid_t cpu = 0; int flags = 0, rval; ASSERT(MUTEX_HELD(&dtrace_lock));