Date: Sat, 13 May 2006 01:07:20 GMT From: John Birrell <jb@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 97045 for review Message-ID: <200605130107.k4D17KY6021798@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
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));
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200605130107.k4D17KY6021798>