Re: svn commit: r253636 - head/sys/vm
On Thu, Jul 25, 2013 at 4:43 AM, David Chisnall thera...@freebsd.orgwrote: However(), memset is to be preferred in this idiom because the compiler provides better diagnostics in the case of error: bzero.c:9:22: warning: 'memset' call operates on objects of type 'struct foo' while the size is based on a different type 'struct foo *' [-Wsizeof-pointer-memaccess] memset(f, 0, sizeof(f)); ~^ bzero.c:9:22: note: did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)? memset(f, 0, sizeof(f)); ^ The same line with bzero(f, sizeof(f)) generates no error. Isn't that a compiler bug? memset(p, 0, n) is the same as bzero(p, n). Why would the compiler warn on one and not the other? Does clang have a similar bias for memcpy versus bcopy? Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r252376 - head/lib/libutil
On Sat, Jun 29, 2013 at 9:36 AM, Tim Kientzle kient...@freebsd.org wrote: On Jun 29, 2013, at 9:19 AM, Konstantin Belousov wrote: On Sat, Jun 29, 2013 at 03:52:49PM +, Tim Kientzle wrote: Author: kientzle Date: Sat Jun 29 15:52:48 2013 New Revision: 252376 URL: http://svnweb.freebsd.org/changeset/base/252376 Log: Fix -Wunsequenced warning What is this ? From the name of the warning, it sounds as if the problem is in the lack of sequence point between two modifications of the same variable in the expression ? But, there function' argument evaluation and function call are separated by seq point, AFAIR. Could you, please, clarify ? I think you're right about that, though I'd have to look at the spec to be sure. Not sure why clang would report this as a -Wunsequenced warning. The implied store here is certainly redundant, though. It may be like other warnings (-Wmissing-field-initializers, I'm looking at you) that warn about currently correct, but potentially problematic behavior. In particular, if any of the functions is re-implemented as a macro, the sequence point goes away, and this code is broken without the code's author having made any changes. So it seems like a reasonable warning. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r252032 - head/sys/amd64/include
[snipping everything about counter64, atomic ops, cycles, etc.] I wonder if the idea explained in this paper: http://static.usenix.org/event/usenix03/tech/freenix03/full_papers/mcgarry/mcgarry_html/ Which seems to be used in FreeBSD for some ARM atomics: http://svnweb.freebsd.org/base/head/sys/arm/include/atomic.h?view=annotate , look for ARM_RAS_START would be more efficient. To summarize: one marks a section of code such that if a thread is interrupted during the code it restarts at the beginning instead of where it was interrupted. This has been used to implement atomic increment on some hardware without the necessary instructions. Here it could be used to implement atomic increment on the per-cpu counter without the overhead of an atomic instruction. It's multiple stores to mark the section of code doing the increment, but they're all per-cpu or per thread. That may be cheaper than an atomic increment, at least on 32-bit platforms that are doing an atomic 64-bit increment. I haven't benchmarked this (ENOTIME, plus I'm on vacation right now), but using restartable sections requires three stores (add an item to a linked list, 64-bit increment for the counter, remove an item from a linked list). Some of the penalty is payed in the context switch code, which has to check if the instruction pointer is in one of these critical sections. I haven't checked to see if this code is enabled on all architectures or just ARM. But if context switches are less frequent than counter increments in the networking code, it's still a win for most uses. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r250986 - head/sys/dev/usb
On Sat, May 25, 2013 at 10:09 AM, Hans Petter Selasky
hsela...@freebsd.orgwrote:
Author: hselasky
Date: Sat May 25 17:09:58 2013
New Revision: 250986
URL: http://svnweb.freebsd.org/changeset/base/250986
Log:
Fix some statical clang analyzer warnings.
Modified:
head/sys/dev/usb/usb_device.c
head/sys/dev/usb/usb_hub.c
head/sys/dev/usb/usb_msctest.c
Modified: head/sys/dev/usb/usb_device.c
==
--- head/sys/dev/usb/usb_device.c Sat May 25 16:58:12 2013
(r250985)
+++ head/sys/dev/usb/usb_device.c Sat May 25 17:09:58 2013
(r250986)
@@ -799,9 +799,6 @@ usb_config_parse(struct usb_device *udev
/* find maximum number of endpoints */
if (ep_max temp)
ep_max = temp;
-
- /* optimalisation */
- id = (struct usb_interface_descriptor *)ed;
}
}
Modified: head/sys/dev/usb/usb_hub.c
==
--- head/sys/dev/usb/usb_hub.c Sat May 25 16:58:12 2013(r250985)
+++ head/sys/dev/usb/usb_hub.c Sat May 25 17:09:58 2013(r250986)
@@ -341,7 +341,6 @@ uhub_reattach_port(struct uhub_softc *sc
DPRINTF(reattaching port %d\n, portno);
- err = 0;
timeout = 0;
udev = sc-sc_udev;
child = usb_bus_port_get_device(udev-bus,
Modified: head/sys/dev/usb/usb_msctest.c
==
--- head/sys/dev/usb/usb_msctest.c Sat May 25 16:58:12 2013
(r250985)
+++ head/sys/dev/usb/usb_msctest.c Sat May 25 17:09:58 2013
(r250986)
@@ -802,7 +802,6 @@ usb_msc_eject(struct usb_device *udev, u
if (sc == NULL)
return (USB_ERR_INVAL);
- err = 0;
switch (method) {
case MSC_EJECT_STOPUNIT:
err = bbb_command_start(sc, DIR_IN, 0, NULL, 0,
@@ -844,6 +843,7 @@ usb_msc_eject(struct usb_device *udev, u
break;
default:
DPRINTF(Unknown eject method (%d)\n, method);
+ err = 0;
break;
}
DPRINTF(Eject CD command status: %s\n, usbd_errstr(err));
I don't know about the top one, but the bottom two are the safer way to
code, and should not have been changed. Unless we feel guaranteed the
compiler can detect all uninitialized use and will break the build, an
early initialization to a sane value is absolutely the right thing to do,
even if it will be overwritten. If the compiler feels sure the
initialization isn't needed, it does not have to emit the code. But any
coding change after the (missing) initialization can create a bug now
(well, it depends on how the code is structured, but from the three lines
of context svn diff provides it's not clear a bug couldn't easily be
introduced).
Thanks,
matthew
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r249105 - in head/sys/cam: ata scsi
On Fri, Apr 5, 2013 at 8:21 AM, Bruce Evans b...@optusnet.com.au wrote: This method works well in userland too. Instead of assert() or abort(), use an null dereference, or more portably, a signal Digressing quite a bit, doesn't abort() send a signal already, i.e. SIGABRT? And doesn't __assert() call abort()? Cheers, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r248649 - in head: share/man/man9 sys/kern sys/sys
On Sat, Mar 23, 2013 at 8:11 AM, Will Andrews w...@freebsd.org wrote:
Author: will
Date: Sat Mar 23 15:11:53 2013
New Revision: 248649
URL: http://svnweb.freebsd.org/changeset/base/248649
Log:
Extend taskqueue(9) to enable per-taskqueue callbacks.
The scope of these callbacks is primarily to support actions that affect the
taskqueue's thread environments. They are entirely optional, and
consequently are introduced as a new API: taskqueue_set_callback().
This interface allows the caller to specify that a taskqueue requires a
callback and optional context pointer for a given callback type.
The callback types included in this commit can be used to register a
constructor and destructor for thread-local storage using osd(9). This
allows a particular taskqueue to define that its threads require a specific
type of TLS, without the need for a specially-orchestrated task-based
mechanism for startup and shutdown in order to accomplish it.
Two callback types are supported at this point:
- TASKQUEUE_CALLBACK_TYPE_INIT, called by every thread when it starts, prior
to processing any tasks.
- TASKQUEUE_CALLBACK_TYPE_SHUTDOWN, called by every thread when it exits,
after it has processed its last task but before the taskqueue is
reclaimed.
While I'm here:
- Add two new macros, TQ_ASSERT_LOCKED and TQ_ASSERT_UNLOCKED, and use them
in appropriate locations.
- Fix taskqueue.9 to mention taskqueue_start_threads(), which is a required
interface for all consumers of taskqueue(9).
Reviewed by: kib (all), eadler (taskqueue.9), brd (taskqueue.9)
Approved by: ken (mentor)
Sponsored by: Spectra Logic
MFC after:1 month
Modified:
head/share/man/man9/taskqueue.9
head/sys/kern/subr_taskqueue.c
head/sys/sys/taskqueue.h
Modified: head/share/man/man9/taskqueue.9
==
--- head/share/man/man9/taskqueue.9 Sat Mar 23 14:52:31 2013
(r248648)
+++ head/share/man/man9/taskqueue.9 Sat Mar 23 15:11:53 2013
(r248649)
@@ -53,12 +53,23 @@ struct task {
void*ta_context;/* argument for handler */
};
+enum taskqueue_callback_type {
+ TASKQUEUE_CALLBACK_TYPE_INIT,
+ TASKQUEUE_CALLBACK_TYPE_SHUTDOWN,
+};
+
+typedef void (*taskqueue_callback_fn)(void *context);
+
struct timeout_task;
.Ed
.Ft struct taskqueue *
.Fn taskqueue_create const char *name int mflags taskqueue_enqueue_fn
enqueue void *context
.Ft struct taskqueue *
.Fn taskqueue_create_fast const char *name int mflags
taskqueue_enqueue_fn enqueue void *context
+.Ft int
+.Fn taskqueue_start_threads struct taskqueue **tqp int count int pri
const char *name ...
+.Ft void
+.Fn taskqueue_set_callback struct taskqueue *queue enum
taskqueue_callback_type cb_type taskqueue_callback_fn callback void
*context
.Ft void
.Fn taskqueue_free struct taskqueue *queue
.Ft int
@@ -127,6 +138,23 @@ should be used to free the memory used b
Any tasks that are on the queue will be executed at this time after
which the thread servicing the queue will be signaled that it should exit.
.Pp
+Once a taskqueue has been created, its threads should be started using
+.Fn taskqueue_start_threads .
+Callbacks may optionally be registered using
+.Fn taskqueue_set_callback .
+Currently, callbacks may be registered for the following purposes:
+.Bl -tag -width TASKQUEUE_CALLBACK_TYPE_SHUTDOWN
+.It Dv TASKQUEUE_CALLBACK_TYPE_INIT
+This callback is called by every thread in the taskqueue, before it executes
+any tasks.
+This callback must be set before the taskqueue's threads are started.
+.It Dv TASKQUEUE_CALLBACK_TYPE_SHUTDOWN
+This callback is called by every thread in the taskqueue, after it executes
+its last task.
+This callback will always be called before the taskqueue structure is
+reclaimed.
+.El
+.Pp
To add a task to the list of tasks queued on a taskqueue, call
.Fn taskqueue_enqueue
with pointers to the queue and task.
Modified: head/sys/kern/subr_taskqueue.c
==
--- head/sys/kern/subr_taskqueue.c Sat Mar 23 14:52:31 2013
(r248648)
+++ head/sys/kern/subr_taskqueue.c Sat Mar 23 15:11:53 2013
(r248649)
@@ -63,6 +63,8 @@ struct taskqueue {
int tq_spin;
int tq_flags;
int tq_callouts;
+ taskqueue_callback_fn tq_callbacks[TASKQUEUE_NUM_CALLBACKS];
+ void*tq_cb_contexts[TASKQUEUE_NUM_CALLBACKS];
};
#defineTQ_FLAGS_ACTIVE (1 0)
@@ -78,6 +80,7 @@ struct taskqueue {
else\
mtx_lock((tq)-tq_mutex); \
} while (0)
+#define
Re: svn commit: r247014 - head/lib/libc/stdlib
On Tue, Feb 19, 2013 at 3:57 PM, Giorgos Keramidas keram...@freebsd.org wrote:
Author: keramida (doc committer)
Date: Tue Feb 19 23:57:39 2013
New Revision: 247014
URL: http://svnweb.freebsd.org/changeset/base/247014
Log:
Add a sample program that shows how a custom comparison function and
qsort(3) can work together to sort an array of integers.
PR: docs/176197
Submitted by: Fernando, fapesteguia at opensistemas.com
Approved by:gjb (mentor)
MFC after: 1 week
Modified:
head/lib/libc/stdlib/qsort.3
Modified: head/lib/libc/stdlib/qsort.3
==
--- head/lib/libc/stdlib/qsort.3Tue Feb 19 23:46:51 2013
(r247013)
+++ head/lib/libc/stdlib/qsort.3Tue Feb 19 23:57:39 2013
(r247014)
@@ -32,7 +32,7 @@
.\ @(#)qsort.38.1 (Berkeley) 6/4/93
.\ $FreeBSD$
.\
-.Dd September 30, 2003
+.Dd February 20, 2013
.Dt QSORT 3
.Os
.Sh NAME
@@ -211,6 +211,52 @@ Previous versions of
did not permit the comparison routine itself to call
.Fn qsort 3 .
This is no longer true.
+.Sh EXAMPLES
+A sample program that sorts an array of
+.Vt int
+values in place using
+.Fn qsort ,
+and then prints the sorted array to standard output is:
+.Bd -literal
+#include stdio.h
+#include stdlib.h
+#include string.h
+
+/*
+ * Custom comparison function that can compare 'int' values through pointers
+ * passed by qsort(3).
+ */
+static int
+int_compare(const void *p1, const void *p2)
+{
+int *left = (int *)p1;
+int *right = (int *)p2;
These should be declared const int *. And the cast shouldn't be
needed in C, since void * can be assigned to any other pointer type.
Cheers,
matthew
+
+if (*left *right)
+return (-1);
+else if (*left *right)
+return (1);
+else
+return (0);
+}
+
+/*
+ * Sort an array of 'int' values and print it to standard output.
+ */
+int
+main(void)
+{
+ int int_array[] = { 4, 5, 9, 3, 0, 1, 7, 2, 8, 6 };
+ const int array_size = sizeof(int_array) / sizeof(int_array[0]);
+ int k;
+
+ qsort(int_array, array_size, sizeof(int), int_compare);
+ for (k = 0; k array_size; k++)
+printf( %d, int_array[k]);
+printf(\\n);
+exit(EXIT_SUCCESS);
+}
+.Ed
.Sh ERRORS
The
.Fn heapsort
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r244112 - head/sys/kern
On Thu, Dec 13, 2012 at 1:42 AM, Andriy Gapon a...@freebsd.org wrote: on 13/12/2012 09:16 Adrian Chadd said the following: Hi, I think the fundamental problem here is we have some pretty different ideas of what KASSERT should be, versus what it actually is in various parts of the code. Oh, and another part of the problem is that the discussion is opinion based. But it didn't have to be. Compare this: We think that feature F is a very good idea, we think that it will be used by many people and it will provide a lot of benefits. So here you are - the code is in the tree. To this: We have been using feature F, it has proved to be a very good idea as it provided these benefits and spared us from these problems. So here you are - the code is in the tree. If I have a differing opinion in the first case I usually state it (and can be pulled into an argument about it). If I have a different opinion in the second case, I try to adjust my opinion to the stated reality. Since we're lost in semantics, we're not going to get any further on this discussion just for now, so let's take a break and think about other things for now. Tools, not policy. A non-panic-ing KASSERT is a tool, not enabled by default. You don't need to use it. Someone does, so why can't we provide the tool? Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r243134 - head/sys/sys
On Thu, Nov 15, 2012 at 10:25 PM, Konstantin Belousov k...@freebsd.org wrote: Author: kib Date: Fri Nov 16 06:25:20 2012 New Revision: 243134 URL: http://svnweb.freebsd.org/changeset/base/243134 Log: Alphabetically reorder the forward-declarations of the structures. Add the declaration for enum idtype, to be used later. Forward declarations of enums isn't an ISO C feature, but a gcc extension. While the kernel uses many gcc extensions, it hides most under a #define so unsupported compilers can continue along. This unsupported feature can't be hidden. Does the forward declaration prevent another warning? Thanks, matthew Reported and reviewed by: bde MFC after:28 days Modified: head/sys/sys/syscallsubr.h Modified: head/sys/sys/syscallsubr.h == --- head/sys/sys/syscallsubr.h Fri Nov 16 06:22:14 2012(r243133) +++ head/sys/sys/syscallsubr.h Fri Nov 16 06:25:20 2012(r243134) @@ -35,25 +35,26 @@ #include sys/mount.h struct file; +enum idtype; struct itimerval; struct image_args; struct jail; +struct kevent; +struct kevent_copyops; +struct kld_file_stat; +struct ksiginfo; struct mbuf; struct msghdr; struct msqid_ds; +struct ogetdirentries_args; struct rlimit; struct rusage; -struct __wrusage; union semun; +struct sendfile_args; struct sockaddr; struct stat; -struct kevent; -struct kevent_copyops; -struct kld_file_stat; -struct ksiginfo; -struct sendfile_args; struct thr_param; -struct ogetdirentries_args; +struct __wrusage; intkern___getcwd(struct thread *td, u_char *buf, enum uio_seg bufseg, u_int buflen); ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r241471 - head/share/man/man9
On Thu, Oct 11, 2012 at 9:55 PM, Gleb Smirnoff gleb...@freebsd.org wrote:
On Fri, Oct 12, 2012 at 01:31:03AM +, Kevin Lo wrote:
K Author: kevlo
K Date: Fri Oct 12 01:31:02 2012
K New Revision: 241471
K URL: http://svn.freebsd.org/changeset/base/241471
K
K Log:
K Since the moduledata structure member priv is a void pointer, using
K NULL instead of 0 when dealing with pointers.
K
K Modified:
K head/share/man/man9/module.9
K
K Modified: head/share/man/man9/module.9
K
==
K --- head/share/man/man9/module.9 Thu Oct 11 23:41:18 2012
(r241470)
K +++ head/share/man/man9/module.9 Fri Oct 12 01:31:02 2012
(r241471)
K @@ -99,7 +99,7 @@ static int foo_handler(module_t mod, int
K static moduledata_t mod_data= {
K foo,
K foo_handler,
K -0
K +NULL
K };
K
K MODULE_VERSION(foo, 1);
I think we should provide C99 sparse initializers for structures in
all manpages in section 9, as well as use only such initializers in any new
code added to tree.
For man pages and .c files, that'd be fine. But since it's still
possible to build C++ kernel modules, header files can't do this since
named initializers don't have the same syntax in C++ (unless they
fixed this in C++11?)
Thanks,
matthew
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r241373 - head/lib/libc/stdlib
On Tue, Oct 9, 2012 at 10:16 AM, David Chisnall thera...@freebsd.org wrote: On 9 Oct 2012, at 17:33, Andrey Chernov wrote: Do you check assembler output for _both_ cases? In my testing clang and gcc xor's 'junk' properly in case it have 'volatile' keyword (as in srandomdev()) and elide it without 'volatile'. IMHO this change should be backed out for srandomdev() and adding 'volatile' for sranddev() instead. In it's original form, it is very dangerous - the whole expression reduces to undefined and so the LLVM IR for the call is: call void @srand(i32 undef) The back end is then free to use any value for the call argument, including any register value or 0. Since the value is passed in a register, it will probably just use whatever the last value there is, which may or may not be anything sensible. On MIPS, for example, this is most likely to be tv, and so is 100% deterministic. Adding the volatile means that we are doing an XOR with a value left on the stack. If this is early on in the application, then it is most likely to be 0. If it's later on, then there may be a value here, but it's still not very likely to be something particularly unpredictable. The original behavior can be recovered by using inline assembly to fetch the value from a register into a local C variable; this would at least not rely on undefined behavior. But I agree it's of dubious value anyways. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r240549 - head/sys/arm/tegra
On Sun, Sep 16, 2012 at 2:53 AM, Alexey Dokuchaev da...@freebsd.org wrote: On Sun, Sep 16, 2012 at 10:37:56AM +0100, Chris Rees wrote: On 16 September 2012 10:28, Alexey Dokuchaev da...@freebsd.org wrote: I thought preferred and more style(9) compliant way to code [empty endless loop] is: for (;;) continue; Actually: for (;;) ; Explicit continue is supposed to tell reviewer that original author did not make a typo, but indeed knew what he was doing. Lonely semicolon is too ambiguous in this case. The semicolon being on its own line is not ambiguous, and is good style everywhere. A semicolon on the same line as a while or for is a red flag that it may be unintentional. Cheers, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r240067 - head/sys/kern
On Mon, Sep 3, 2012 at 1:52 AM, Aleksandr Rybalko r...@freebsd.org wrote:
Author: ray
Date: Mon Sep 3 08:52:05 2012
New Revision: 240067
URL: http://svn.freebsd.org/changeset/base/240067
Log:
Add kern.hintmode sysctl variable to show current state of hints:
0 - loader hints in environment only;
1 - static hints only
2 - fallback mode (Dynamic KENV with fallback to kernel environment)
Add kern.hintmode write handler, accept only value 2. That will switch
static KENV to dynamic. So it will be possible to change device hints.
Approved by: adrian (mentor)
Modified:
head/sys/kern/subr_hints.c
Modified: head/sys/kern/subr_hints.c
==
--- head/sys/kern/subr_hints.c Mon Sep 3 07:18:24 2012(r240066)
+++ head/sys/kern/subr_hints.c Mon Sep 3 08:52:05 2012(r240067)
@@ -29,8 +29,10 @@ __FBSDID($FreeBSD$);
#include sys/param.h
#include sys/lock.h
+#include sys/malloc.h
#include sys/mutex.h
#include sys/systm.h
+#include sys/sysctl.h
Putting on my style-nazi hat. sysctl comes before systm alphabetically.
#include sys/bus.h
/*
@@ -42,6 +44,81 @@ static int use_kenv;
static char *hintp;
/*
+ * Define kern.hintmode sysctl, which only accept value 2, that cause to
+ * switch from Static KENV mode to Dynamic KENV. So systems that have hints
+ * compiled into kernel will be able to see/modify KENV (and hints too).
+ */
+
+static int
+sysctl_hintmode(SYSCTL_HANDLER_ARGS)
+{
+ int error, i, from_kenv, value, eqidx;
+ const char *cp;
+ char *line, *eq;
These are not sorted properly; pointers come before scalars, and
within the group they should be sorted alphabetically; e.g. eqidx
comes before all the other ints.
+
+ from_kenv = 0;
+ cp = kern_envp;
+ value = hintmode;
+
+ /* Fetch candidate for new hintmode value */
+ error = sysctl_handle_int(oidp, value, 0, req);
+ if (error || !req-newptr)
This may be copying existing code, but style(9) explicitly forbids
using '!' except on boolean expressions. Since req-newptr is a
pointer, an explicit comparison to NULL should be made. This error is
repeated later.
Thanks,
matthew
+ return (error);
+
+ if (value != 2)
+ /* Only accept swithing to hintmode 2 */
+ return (EINVAL);
+
+ /* Migrate from static to dynamic hints */
+ switch (hintmode) {
+ case 0:
+ if (dynamic_kenv)
+ /* Already here */
+ hintmode = value; /* XXX: Need we switch or not ? */
+ return (0);
+ from_kenv = 1;
+ cp = kern_envp;
+ break;
+ case 1:
+ cp = static_hints;
+ break;
+ case 2:
+ /* Nothing to do, hintmode already 2 */
+ return (0);
+ }
+
+ while (cp) {
+ i = strlen(cp);
+ if (i == 0)
+ break;
+ if (from_kenv) {
+ if (strncmp(cp, hint., 5) != 0)
+ /* kenv can have not only hints */
+ continue;
+ }
+ eq = strchr(cp, '=');
+ if (!eq)
+ /* Bad hint value */
+ continue;
+ eqidx = eq - cp;
+
+ line = malloc(i+1, M_TEMP, M_WAITOK);
+ strcpy(line, cp);
+ line[eqidx] = '\0';
+ setenv(line, line + eqidx + 1);
+ free(line, M_TEMP);
+ cp += i + 1;
+ }
+
+ hintmode = value;
+ use_kenv = 1;
+ return (0);
+}
+
+SYSCTL_PROC(_kern, OID_AUTO, hintmode, CTLTYPE_INT|CTLFLAG_RW,
+hintmode, 0, sysctl_hintmode, I, Get/set current hintmode);
+
+/*
* Evil wildcarding resource string lookup.
* This walks the supplied env string table and returns a match.
* The start point can be remembered for incremental searches.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r238502 - in head/sys: kern vm
On Sun, Jul 15, 2012 at 1:29 PM, Matthew D Fleming m...@freebsd.org wrote: Author: mdf Date: Sun Jul 15 20:29:48 2012 New Revision: 238502 URL: http://svn.freebsd.org/changeset/base/238502 Log: Fix a bug with memguard(9) on 32-bit architectures without a VM_KMEM_MAX_SIZE. The code was not taking into account the size of the kernel_map, which the kmem_map is allocated from, so it could produce a sub-map size too large to fit. The simplest solution is to ignore VM_KMEM_MAX entirely and base the memguard map's size off the kernel_map's size, since this is always relevant and always smaller. Found by: Justin Hibbits MFC after: 3 days Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r236380 - head/sys/vm
On Fri, Jun 1, 2012 at 2:14 AM, Bruce Evans b...@optusnet.com.au wrote: +SYSCTL_OID(_vm, OID_AUTO, swap_free, CTLTYPE_INT|CTLFLAG_RD|CTLFLAG_MPSAFE, + NULL, 0, sysctl_vm_swap_free, Q, + Blocks of free swap storage.); Bug 9 is a style bug. I didn't even know that the raw SYSCTL_OID() could be misused like this. The normal SYSCTL_PROC() is identical with SYSCTL_OID() except it checks that the access flags are not 0. Few or no SYSCTL_FOO()s have no access flags, and this is not one. It has rather excessive access flags (I think CTLFLAG_MPSAFE is unnecessary. I wanted to correct this one point. CTLFLAG_MPSAFE is helpful, because its use prevents kern_sysctl from taking Giant before calling the sysctl handler. It's probably nearing the case, now, that any sysctl *without* CTLFLAG_MPSAFE is incorrect, except perhaps for a few that set/get text strings that don't want to roll their own serialization. Cheers, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r235797 - head/contrib/gcc
On Tue, May 22, 2012 at 1:05 PM, Bruce Evans b...@optusnet.com.au wrote: On Tue, 22 May 2012, David E. O'Brien wrote: Log: Do not incorrectly warn when printing a quad_t using %qd on 64-bit platforms. I think I like this, since it is technically correct, and will find a different set of type mismatches. We run with the following at Isilon, which is somewhat bogus because it allows a bit of sloppiness in types, but is also terribly convenient since it means no casting on printf arguments is needed: --- contrib/gcc/c-format.c 2012-05-22 14:08:23.538266746 -0700 +++ /data/sb/head/src/contrib/gcc/c-format.c2012-05-16 12:59:40.937016702 -0700 @@ -2298,10 +2570,20 @@ check_format_types (format_wanted_type * equivalent but the above test won't consider them equivalent. */ if (wanted_type == char_type_node (!pedantic || i 2) char_type_flag) continue; + + /* Isilon: FreeBSD defines int64_t (and others) as one type (e.g. long +long) on i386 and another type (e.g. long) on amd64. This prevents +the use of a common format specifier. Treat equal sized integer types +as equivalent. */ + if (TREE_CODE (wanted_type) == INTEGER_TYPE + TREE_CODE (cur_type) == INTEGER_TYPE + int_size_in_bytes (wanted_type) == int_size_in_bytes (cur_type)) +continue; + /* Now we have a type mismatch. */ format_type_warning (types-name, format_start, format_length, wanted_type, types-pointer_count, types-wanted_type_name, orig_cur_type, arg_num); } If there's no objections, I (or David or anyone else) can commit to the FreeBSD repository. Cheers, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r231220 - head/sys/sys
On Wed, Feb 8, 2012 at 10:36 AM, Konstantin Belousov k...@freebsd.org wrote:
Author: kib
Date: Wed Feb 8 18:36:07 2012
New Revision: 231220
URL: http://svn.freebsd.org/changeset/base/231220
Log:
Trim 8 unused bytes from struct vnode on 64-bit architectures.
Doesn't this change the KBI? So should __FreeBSD_version be bumped?
Thanks,
matthew
Modified:
head/sys/sys/vnode.h
Modified: head/sys/sys/vnode.h
==
--- head/sys/sys/vnode.h Wed Feb 8 18:22:10 2012 (r231219)
+++ head/sys/sys/vnode.h Wed Feb 8 18:36:07 2012 (r231220)
@@ -149,8 +149,8 @@ struct vnode {
struct lock *v_vnlock; /* u pointer to vnode lock */
int v_holdcnt; /* i prevents recycling. */
int v_usecount; /* i ref count of users */
- u_long v_iflag; /* i vnode flags (see below)
*/
- u_long v_vflag; /* v vnode flags */
+ u_int v_iflag; /* i vnode flags (see below)
*/
+ u_int v_vflag; /* v vnode flags */
int v_writecount; /* v ref count of writers */
/*
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r228878 - head/include
On Thu, Dec 29, 2011 at 6:54 PM, Sean C. Farley s...@freebsd.org wrote: On Sun, 25 Dec 2011, Ed Schouten wrote: Author: ed Date: Sun Dec 25 20:15:41 2011 New Revision: 228878 URL: http://svn.freebsd.org/changeset/base/228878 Log: Remove unneeded guard. There is no reason why stdbool.h needs an include guard. It is already protected by __bool_true_false_are_defined. Modified: head/include/stdbool.h Modified: head/include/stdbool.h == --- head/include/stdbool.h Sun Dec 25 18:15:31 2011 (r228877) +++ head/include/stdbool.h Sun Dec 25 20:15:41 2011 (r228878) @@ -26,9 +26,6 @@ * $FreeBSD$ */ -#ifndef _STDBOOL_H_ -#define _STDBOOL_H_ - #ifndef __bool_true_false_are_defined #define __bool_true_false_are_defined 1 @@ -44,5 +41,3 @@ typedef int _Bool; #endif /* !__cplusplus */ #endif /* __bool_true_false_are_defined */ - -#endif /* !_STDBOOL_H_ */ I just thought of this while reviewing the change: should __bool_true_false_are_defined be set only if __cplusplus is not set? It should be set for C99, but I wonder if it should be set for C++. My quick googling didn't show anything at all about the C++ standard and stdbool.h or __bool_true_false_are_defined. It was probably originally set because bool, true, and false are all C++ keywords so certain code that wanted to ifdef on them didn't also need to check __cplusplus. Also, is there a style requirement that the guard for a header file be based off of the name of the file? I did not see anything obvious for this within style(9), but I am curious. I think it's just common use to make sure different headers use a different include guard, so they only protect their contents, not any other file's. The C standard only mentions the symbols bool, true, false, and __bool_true_false_are_defined in regards to stdbool.h. Cheers, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r228625 - head/usr.bin/csup
On Sat, Dec 17, 2011 at 5:14 AM, Dimitry Andric d...@freebsd.org wrote: Author: dim Date: Sat Dec 17 13:14:44 2011 New Revision: 228625 URL: http://svn.freebsd.org/changeset/base/228625 Log: In usr.bin/csup/auth.c, use the correct number of bytes for zeroing the shared secret, and use long long format to snprintf a time_t. If casting is necessary, style prefers intmax_t or uintmax_t, since those are always wide enough. Thanks, matthew MFC after: 1 week Modified: head/usr.bin/csup/auth.c Modified: head/usr.bin/csup/auth.c == --- head/usr.bin/csup/auth.c Sat Dec 17 12:52:58 2011 (r228624) +++ head/usr.bin/csup/auth.c Sat Dec 17 13:14:44 2011 (r228625) @@ -254,7 +254,7 @@ auth_makesecret(struct srvrecord *auth, MD5_Update(md5, :, 1); MD5_Update(md5, auth-password, strlen(auth-password)); MD5_Final(md5sum, md5); - memset(secret, 0, sizeof(secret)); + memset(secret, 0, MD5_CHARS_MAX); strcpy(secret, md5salt); auth_readablesum(md5sum, secret + strlen(md5salt)); } @@ -302,8 +302,9 @@ auth_makechallenge(struct config *config } gettimeofday(tv, NULL); MD5_Init(md5); - snprintf(buf, sizeof(buf), %s:%ld:%ld:%ld:%d:%d, - inet_ntoa(laddr.sin_addr), tv.tv_sec, tv.tv_usec, random(), pid, ppid); + snprintf(buf, sizeof(buf), %s:%lld:%ld:%ld:%d:%d, + inet_ntoa(laddr.sin_addr), (long long)tv.tv_sec, tv.tv_usec, + random(), pid, ppid); MD5_Update(md5, buf, strlen(buf)); MD5_Final(md5sum, md5); auth_readablesum(md5sum, challenge); ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r228625 - head/usr.bin/csup
On Sat, Dec 17, 2011 at 1:54 PM, Dimitry Andric d...@freebsd.org wrote: On 2011-12-17 22:32, m...@freebsd.org wrote: ... In usr.bin/csup/auth.c, use the correct number of bytes for zeroing the shared secret, and use long long format to snprintf a time_t. If casting is necessary, style prefers intmax_t or uintmax_t, since those are always wide enough. I don't see anything about that in style(9), maybe it should be added then? Probably; Bruce has mentioned it many times in the past, and as bz@ notes, it's a well-defined type with a well-defined conversion specifier. Also, long long is a bit of a hack that came in before C99 standardized on a few wider types, and the PRIu64 macros are really hideous. Thanks, matthew In any case, I only changed the %ld format to %lld, because time_t is int, long or long long depending on arch. Long long is just the widest type required in this case. ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r227873 - head/usr.bin/procstat
On Tue, Nov 22, 2011 at 11:34 PM, Mikolaj Golub troc...@freebsd.org wrote:
Author: trociny
Date: Wed Nov 23 07:34:09 2011
New Revision: 227873
URL: http://svn.freebsd.org/changeset/base/227873
Log:
Fix build, hopefully.
Reviewed by: kib
Modified:
head/usr.bin/procstat/procstat_auxv.c
Modified: head/usr.bin/procstat/procstat_auxv.c
==
--- head/usr.bin/procstat/procstat_auxv.c Wed Nov 23 07:12:26 2011
(r227872)
+++ head/usr.bin/procstat/procstat_auxv.c Wed Nov 23 07:34:09 2011
(r227873)
@@ -42,14 +42,13 @@
#include procstat.h
-static char auxv[sizeof(Elf_Auxinfo) * 256];
+static Elf_Auxinfo auxv[256];
void
procstat_auxv(struct kinfo_proc *kipp)
{
- Elf_Auxinfo *aux;
- int i, error, name[4];
- size_t len;
+ int error, name[4];
+ size_t len, i;
if (!hflag)
printf(%5s %-16s %-53s\n, PID, COMM, AUXV);
@@ -58,7 +57,7 @@ procstat_auxv(struct kinfo_proc *kipp)
name[1] = KERN_PROC;
name[2] = KERN_PROC_AUXV;
name[3] = kipp-ki_pid;
- len = sizeof(auxv);
+ len = sizeof(auxv) * sizeof(*auxv);
error = sysctl(name, 4, auxv, len, NULL, 0);
if (error 0 errno != ESRCH) {
warn(sysctl: kern.proc.auxv: %d: %d, kipp-ki_pid, errno);
@@ -72,106 +71,116 @@ procstat_auxv(struct kinfo_proc *kipp)
printf( -\n);
return;
}
- for (aux = (Elf_Auxinfo *)auxv, i = 0; i 256; i++, aux++) {
- switch(aux-a_type) {
+ for (i = 0; i len; i++) {
+ switch(auxv[i].a_type) {
case AT_NULL:
- printf( (%d)\n, i + 1);
+ printf( (%zu)\n, i + 1);
return;
case AT_IGNORE:
printf( AT_IGNORE=0x%lu,
- (unsigned long)aux-a_un.a_val);
+ (unsigned long)auxv[i].a_un.a_val);
I didn't see this before, but this gives very misleading output. The
0x prefix implies the output will be hex, but it's printed as decimal,
here and below.
I don't know if there's a style preference for 0x%lx versus %#lx,
though, or a preference for a different type for the print (uintmax_t,
for example). There is probably a preference for using u_long rather
than unsigned long, since it's shorter.
Thanks,
matthew
break;
case AT_EXECFD:
printf( AT_EXECFD=0x%lu,
- (unsigned long)aux-a_un.a_val);
+ (unsigned long)auxv[i].a_un.a_val);
break;
case AT_PHDR:
printf( AT_PHDR=0x%lu,
- (unsigned long)aux-a_un.a_val);
+ (unsigned long)auxv[i].a_un.a_val);
break;
case AT_PHENT:
printf( AT_PHENT=0x%lu,
- (unsigned long)aux-a_un.a_val);
+ (unsigned long)auxv[i].a_un.a_val);
break;
case AT_PHNUM:
printf( AT_PHNUM=0x%lu,
- (unsigned long)aux-a_un.a_val);
+ (unsigned long)auxv[i].a_un.a_val);
break;
case AT_PAGESZ:
printf( AT_PAGESZ=0x%lu,
- (unsigned long)aux-a_un.a_val);
+ (unsigned long)auxv[i].a_un.a_val);
break;
case AT_BASE:
printf( AT_BASE=0x%lu,
- (unsigned long)aux-a_un.a_val);
+ (unsigned long)auxv[i].a_un.a_val);
break;
case AT_FLAGS:
printf( AT_FLAGS=0x%lu,
- (unsigned long)aux-a_un.a_val);
+ (unsigned long)auxv[i].a_un.a_val);
break;
case AT_ENTRY:
printf( AT_ENTRY=0x%lu,
- (unsigned long)aux-a_un.a_val);
+ (unsigned long)auxv[i].a_un.a_val);
break;
+#ifdef AT_NOTELF
case AT_NOTELF:
printf( AT_NOTELF=0x%lu,
- (unsigned long)aux-a_un.a_val);
+ (unsigned long)auxv[i].a_un.a_val);
break;
+#endif
+#ifdef AT_UID
case AT_UID:
printf( AT_UID=0x%lu,
- (unsigned long)aux-a_un.a_val);
+ (unsigned long)auxv[i].a_un.a_val);
break;
+#endif
+#ifdef AT_EUID
case AT_EUID:
Re: svn commit: r227812 - head/lib/libc/string
On Mon, Nov 21, 2011 at 6:50 PM, Eitan Adler ead...@freebsd.org wrote: Author: eadler (ports committer) Date: Tue Nov 22 02:50:24 2011 New Revision: 227812 URL: http://svn.freebsd.org/changeset/base/227812 Log: - fix some style(9) nits with my last commit - add a comment explaining why I used '|' instead of '||' Submitted by: danfe@ Approved by: emaste@ Modified: head/lib/libc/string/strcasecmp.c head/lib/libc/string/strncmp.c Modified: head/lib/libc/string/strcasecmp.c == --- head/lib/libc/string/strcasecmp.c Tue Nov 22 02:27:59 2011 (r227811) +++ head/lib/libc/string/strcasecmp.c Tue Nov 22 02:50:24 2011 (r227812) @@ -49,7 +49,7 @@ strcasecmp_l(const char *s1, const char *us1 = (const u_char *)s1, *us2 = (const u_char *)s2; if (s1 == s2) - return (0); + return (0); FIX_LOCALE(locale); @@ -73,8 +73,9 @@ strncasecmp_l(const char *s1, const char *us1 = (const u_char *)s1, *us2 = (const u_char *)s2; - if (( s1 == s2) | (n == 0)) - return (0); + /* use a bitwise or to avoid an additional branch instruction */ + if ((s1 == s2) | (n == 0)) + return (0); I guess I'm a little confused. Do we really have profiling information at this level that suggests the overhead of the branch is significant? I thought most hardware had pretty good branch-prediction, particularly with speculative execution. Wouldn't something like __predict_false() have more value for performance, or is all this just guess-work? I would much rather have the code say what it means unless there's real, measurable performance differences from doing otherwise. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r227588 - in head: share/man/man9 sys/kern sys/sys
On Wed, Nov 16, 2011 at 1:51 PM, Pawel Jakub Dawidek p...@freebsd.org wrote: Author: pjd Date: Wed Nov 16 21:51:17 2011 New Revision: 227588 URL: http://svn.freebsd.org/changeset/base/227588 Log: Constify arguments for locking KPIs where possible. This enables locking consumers to pass their own structures around as const and be able to assert locks embedded into those structures. Thank you! This is one of many (minor) diffs I have at $WORK. Cheers, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r227541 - head/sys/dev/usb/controller
On Tue, Nov 15, 2011 at 12:48 PM, Hans Petter Selasky
hsela...@freebsd.org wrote:
Author: hselasky
Date: Tue Nov 15 20:48:57 2011
New Revision: 227541
URL: http://svn.freebsd.org/changeset/base/227541
Log:
Some brands of XHCI controllers needs more time to reset.
... and since there's no guarantee that hz is 1000 or has any
particular value, most of these seem a bit spurious.
Is there some reason these functions aren't asking for a delay in
terms of milli- or microseconds, and converting to hz internally? I
would expect a delay while waiting for hardware to have a wall-clock
time, not a time relative to hz, which has no predefined range.
Thanks,
matthew
Reported by: Jan Henrik Sylvester
MFC after: 1 week
Modified:
head/sys/dev/usb/controller/xhci.c
Modified: head/sys/dev/usb/controller/xhci.c
==
--- head/sys/dev/usb/controller/xhci.c Tue Nov 15 20:41:50 2011
(r227540)
+++ head/sys/dev/usb/controller/xhci.c Tue Nov 15 20:48:57 2011
(r227541)
@@ -292,7 +292,7 @@ xhci_start_controller(struct xhci_softc
XWRITE4(sc, oper, XHCI_USBCMD, XHCI_CMD_HCRST);
for (i = 0; i != 100; i++) {
- usb_pause_mtx(NULL, hz / 1000);
+ usb_pause_mtx(NULL, hz / 100);
temp = XREAD4(sc, oper, XHCI_USBCMD)
(XHCI_CMD_HCRST | XHCI_STS_CNR);
if (!temp)
@@ -453,7 +453,7 @@ xhci_start_controller(struct xhci_softc
XHCI_CMD_INTE | XHCI_CMD_HSEE);
for (i = 0; i != 100; i++) {
- usb_pause_mtx(NULL, hz / 1000);
+ usb_pause_mtx(NULL, hz / 100);
temp = XREAD4(sc, oper, XHCI_USBSTS) XHCI_STS_HCH;
if (!temp)
break;
@@ -487,7 +487,7 @@ xhci_halt_controller(struct xhci_softc *
XWRITE4(sc, oper, XHCI_USBCMD, 0);
for (i = 0; i != 100; i++) {
- usb_pause_mtx(NULL, hz / 1000);
+ usb_pause_mtx(NULL, hz / 100);
temp = XREAD4(sc, oper, XHCI_USBSTS) XHCI_STS_HCH;
if (temp)
break;
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r227541 - head/sys/dev/usb/controller
On Tue, Nov 15, 2011 at 1:20 PM, m...@freebsd.org wrote: On Tue, Nov 15, 2011 at 1:02 PM, Hans Petter Selasky hsela...@c2i.net wrote: For USB compliant operation, the USB stack requires hz to be greater or equal to 250 hz, to put it like that. Mostly a requirement in USB gadget/device mode. Really? That's news to me. Is that documented somewhere? I know we still use hz=100 internally, but we're on stable/7 still so not using the new USB stack yet. ... and I also just remembered that I have seen recommendations that, when FreeBSD is used as a virtual machine, hz should be set to 100 so that the virtual interrupt overhead is reduced. Those two recommendations are at odds with each other. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r227541 - head/sys/dev/usb/controller
On Tue, Nov 15, 2011 at 1:22 PM, Hans Petter Selasky hsela...@c2i.net wrote: On Tuesday 15 November 2011 22:20:18 m...@freebsd.org wrote: On Tue, Nov 15, 2011 at 1:02 PM, Hans Petter Selasky hsela...@c2i.net wrote: For USB compliant operation, the USB stack requires hz to be greater or equal to 250 hz, to put it like that. Mostly a requirement in USB gadget/device mode. Really? That's news to me. Is that documented somewhere? I know we still use hz=100 internally, but we're on stable/7 still so not using the new USB stack yet. No it is not documented anywhere. This delay is mostly critical if you enable USB power saving features like suspend and resume. Then there are some software timers which should not derive too much. Most of the time the delays in USB are not critical. Transfer timers are in the seconds range and that works fine with hz=100. Where and how should I document such are requirement? Add something during system init? if (hz 250) printf(USB: hz is too low (ignored)\n); I'm not sure what functions we have for detecting the OS instance is virtualized, but something like that would be useful if it's really important. Perhaps: USB: hz value less than 250 may cause functional issues Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r227473 - head/sbin/geom/class/multipath
On Sun, Nov 13, 2011 at 2:46 PM, Garrett Cooper yaneg...@gmail.com wrote:
On Sun, Nov 13, 2011 at 2:08 PM, Pawel Jakub Dawidek p...@freebsd.org wrote:
On Sat, Nov 12, 2011 at 12:16:23PM -0800, Garrett Cooper wrote:
On Sat, Nov 12, 2011 at 12:01 PM, Alexander Motin m...@freebsd.org wrote:
Author: mav
Date: Sat Nov 12 20:01:30 2011
New Revision: 227473
URL: http://svn.freebsd.org/changeset/base/227473
Log:
Fix build on some archs after r227464.
Modified:
head/sbin/geom/class/multipath/geom_multipath.c
Modified: head/sbin/geom/class/multipath/geom_multipath.c
==
--- head/sbin/geom/class/multipath/geom_multipath.c Sat Nov 12
19:55:48 2011 (r227472)
+++ head/sbin/geom/class/multipath/geom_multipath.c Sat Nov 12
20:01:30 2011 (r227473)
@@ -133,7 +133,8 @@ mp_label(struct gctl_req *req)
uint8_t *sector, *rsector;
char *ptr;
uuid_t uuid;
- uint32_t secsize = 0, ssize, status;
+ ssize_t secsize = 0, ssize;
+ uint32_t status;
const char *name, *name2, *mpname;
int error, i, nargs, fd;
@@ -161,8 +162,8 @@ mp_label(struct gctl_req *req)
disksize = msize;
} else {
if (secsize != ssize) {
- gctl_error(req, %s sector size %u
different.,
- name, ssize);
+ gctl_error(req, %s sector size %ju
different.,
+ name, (intmax_t)ssize);
Shouldn't that be uintmax_t, not intmax_t ?
No, ssize_t is signed. Although the best would be to use %zd for
ssize_t.
Thanks... Missed the leading s.
... except that the cast and conversion specifier aren't in sync. The
cast is signed; the conversion specifier is unsigned. %zd is still
best, since it's the conversion specifier for the variable's type.
Next best is %jd and cast to intmax_t, since the signedness is
preserved.
But in the end it doesn't matter a lot since whatever is printed in
the error message, it's likely derivable from reading the code what
the actual value used was.
Cheers,
matthew
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r222537 - in head/sys: kern sys
On Tue, May 31, 2011 at 10:29 AM, Kenneth D. Merry k...@freebsd.org wrote: Author: ken Date: Tue May 31 17:29:58 2011 New Revision: 222537 URL: http://svn.freebsd.org/changeset/base/222537 Log: Fix apparent garbage in the message buffer. While we have had a fix in place (options PRINTF_BUFR_SIZE=128) to fix scrambled console output, the message buffer and syslog were still getting log messages one character at a time. While all of the characters still made it into the log (courtesy of atomic operations), they were often interleaved when there were multiple threads writing to the buffer at the same time. This seems to panic my box with lock msgbuf 0xfe0127e0 already initialized. Unfortunately, though I booted with a fresh CURRENT this morning successfully, both /boot/kernel and /boot/kernel.old give this panic. To add insult to injury, when the kernel drops into the debugger, my keyboard input no longer works so I can't get a stack, etc. So: 1) Is there anything else I can do to help debug this? 2) how can I resurrect this box without a reinstall? I will try to repro on a virtual machine so I have a snapshot to come back to. Thanks, matthew This fixes message buffer accesses to use buffering logic as well, so that strings that are less than PRINTF_BUFR_SIZE will be put into the message buffer atomically. So now dmesg output should look the same as console output. subr_msgbuf.c: Convert most message buffer calls to use a new spin lock instead of atomic variables in some places. Add a new routine, msgbuf_addstr(), that adds a NUL-terminated string to a message buffer. This takes a priority argument, which allows us to eliminate some races (at least in the the string at a time case) that are present in the implementation of msglogchar(). (dangling and lastpri are static variables, and are subject to races when multiple callers are present.) msgbuf_addstr() also allows the caller to request that carriage returns be stripped out of the string. This matches the behavior of msglogchar(), but in testing so far it doesn't appear that any newlines are being stripped out. So the carriage return removal functionality may be a candidate for removal later on if further analysis shows that it isn't necessary. subr_prf.c: Add a new msglogstr() routine that calls msgbuf_logstr(). Rename putcons() to putbuf(). This now handles buffered output to the message log as well as the console. Also, remove the logic in putcons() (now putbuf()) that added a carriage return before a newline. The console path was the only path that needed it, and cnputc() (called by cnputs()) already adds a carriage return. So this duplication resulted in kernel-generated console output lines ending in '\r''\r''\n'. Refactor putchar() to handle the new buffering scheme. Add buffering to log(). Change log_console() to use msglogstr() instead of msglogchar(). Don't add extra newlines by default in log_console(). Hide that behavior behind a tunable/sysctl (kern.log_console_add_linefeed) for those who would like the old behavior. The old behavior led to the insertion of extra newlines for log output for programs that print out a string, and then a trailing newline on a separate write. (This is visible with dmesg -a.) msgbuf.h: Add a prototype for msgbuf_addstr(). Add three new fields to struct msgbuf, msg_needsnl, msg_lastpri and msg_lock. The first two are needed for log message functionality previously handled by msglogchar(). (Which is still active if buffering isn't enabled.) Include sys/lock.h and sys/mutex.h for the new mutex. Reviewed by: gibbs Modified: head/sys/kern/subr_msgbuf.c head/sys/kern/subr_prf.c head/sys/sys/msgbuf.h Modified: head/sys/kern/subr_msgbuf.c
Re: svn commit: r222537 - in head/sys: kern sys
On Tue, May 31, 2011 at 2:46 PM, Kenneth D. Merry k...@freebsd.org wrote: On Tue, May 31, 2011 at 14:00:18 -0700, m...@freebsd.org wrote: On Tue, May 31, 2011 at 10:29 AM, Kenneth D. Merry k...@freebsd.org wrote: Author: ken Date: Tue May 31 17:29:58 2011 New Revision: 222537 URL: http://svn.freebsd.org/changeset/base/222537 Log: ?Fix apparent garbage in the message buffer. ?While we have had a fix in place (options PRINTF_BUFR_SIZE=128) to fix ?scrambled console output, the message buffer and syslog were still getting ?log messages one character at a time. ?While all of the characters still ?made it into the log (courtesy of atomic operations), they were often ?interleaved when there were multiple threads writing to the buffer at the ?same time. This seems to panic my box with lock msgbuf 0xfe0127e0 already initialized. Unfortunately, though I booted with a fresh CURRENT this morning successfully, both /boot/kernel and /boot/kernel.old give this panic. To add insult to injury, when the kernel drops into the debugger, my keyboard input no longer works so I can't get a stack, etc. Uh-oh! So: 1) Is there anything else I can do to help debug this? 2) how can I resurrect this box without a reinstall? I will try to repro on a virtual machine so I have a snapshot to come back to. My guess is that this is an issue with the message buffer reinitialization path. lock_init() (called by mtx_init()) has an assert to make sure that the lock is initialized, and that is just a flag check. Since the spin lock is part of the message buffer structure, if it is held over from a previous boot, the LO_INITIALIZED flag may still be set. Try power cycling the machine. If it is an issue with re-initialization, that should clear the memory and allow you to boot. Hmm, apparently my previous presses of the power button weren't long enough. I let it sit off for 20 seconds and it boots okay now. Thanks, matthew My testing has been with VMs (under Xen), so the reinit path has probably not been tested as fully as it should have been. Sorry about that! As for the debugger, that's another issue altogether. It does work for me, but then again if the spin lock initialization is broken for the message buffer that may affect things. Try a cold boot and see if that helps. If so, I think we can probably just bzero the mutex in msgbuf_reinit() and that will fix things. Ken -- Kenneth Merry k...@freebsd.org ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r222537 - in head/sys: kern sys
On Tue, May 31, 2011 at 3:11 PM, Kenneth D. Merry k...@freebsd.org wrote: On Tue, May 31, 2011 at 15:02:37 -0700, m...@freebsd.org wrote: On Tue, May 31, 2011 at 2:46 PM, Kenneth D. Merry k...@freebsd.org wrote: On Tue, May 31, 2011 at 14:00:18 -0700, m...@freebsd.org wrote: On Tue, May 31, 2011 at 10:29 AM, Kenneth D. Merry k...@freebsd.org wrote: Author: ken Date: Tue May 31 17:29:58 2011 New Revision: 222537 URL: http://svn.freebsd.org/changeset/base/222537 Log: ?Fix apparent garbage in the message buffer. ?While we have had a fix in place (options PRINTF_BUFR_SIZE=128) to fix ?scrambled console output, the message buffer and syslog were still getting ?log messages one character at a time. ?While all of the characters still ?made it into the log (courtesy of atomic operations), they were often ?interleaved when there were multiple threads writing to the buffer at the ?same time. This seems to panic my box with lock msgbuf 0xfe0127e0 already initialized. Unfortunately, though I booted with a fresh CURRENT this morning successfully, both /boot/kernel and /boot/kernel.old give this panic. To add insult to injury, when the kernel drops into the debugger, my keyboard input no longer works so I can't get a stack, etc. Uh-oh! So: 1) Is there anything else I can do to help debug this? 2) how can I resurrect this box without a reinstall? I will try to repro on a virtual machine so I have a snapshot to come back to. My guess is that this is an issue with the message buffer reinitialization path. ?lock_init() (called by mtx_init()) has an assert to make sure that the lock is initialized, and that is just a flag check. Since the spin lock is part of the message buffer structure, if it is held over from a previous boot, the LO_INITIALIZED flag may still be set. Try power cycling the machine. ?If it is an issue with re-initialization, that should clear the memory and allow you to boot. Hmm, apparently my previous presses of the power button weren't long enough. I let it sit off for 20 seconds and it boots okay now. Okay, so it probably is the re-initialization code. Can you try this patch and see if it survives a warm boot? I also changed the initialization path, so we don't get tripped up by garbage left in memory. This patch survived a warm reboot (shutdown -r now). Also, does the debugger work now that it has booted successfully? The debugger (or rather, my keyboard in the debugger) works on a successful boot. I used sysctl debug.kdb.enter=1 to test it. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r221853 - in head/sys: dev/md dev/null sys vm
On Tue, May 31, 2011 at 2:48 PM, Pieter de Goeje pie...@degoeje.nl wrote: On Sunday 29 May 2011 05:01:57 m...@freebsd.org wrote: On Sat, May 28, 2011 at 12:03 PM, Pieter de Goeje pie...@degoeje.nl wrote: To me it looks like it's not able to cache the zeroes anymore. Is this intentional? I tried to change ZERO_REGION_SIZE back to 64K but that didn't help. Hmm. I don't have access to my FreeBSD box over the weekend, but I'll run this on my box when I get back to work. Meanwhile you could try setting ZERO_REGION_SIZE to PAGE_SIZE and I think that will restore things to the original performance. Indeed it does. I couldn't find any authoritative docs stating wether or not the cache on this CPU is virtually indexed, but apparently at least some of it is. On my physical box (some Dell thing from about 2008), I ran 10 loops of dd if=/dev/zero of=/dev/null bs=XX count=XX where bs went by powers of 2 from 512 bytes to 2M, and count was set so that the dd always transferred 8GB. I compared ZERO_REGION_SIZE of 64k and 2M on amd64. The summary of the ministat(1) output is: bs=512b - no difference bs=1K - no difference bs=2k - no difference bs=4k - no difference bs=8k - no difference bs=16k - no difference bs=32k - no difference bs=64k - no difference bs=128k - 2M is 0.69% faster bs=256k - 2M is 0.98% faster bs=512k - 2M is 0.65% faster bs=1M - 2M is 1.02% faster bs=2M - 2M is 2.17% slower I'll play again with a 4K buffer. For some applications (/dev/zero) a small size is sufficient. For some (md(4)) a ZERO_REGION_SIZE at least as large as the sectorsize is desired so that a single kernel buffer pointer can be used to set up a uio for VOP_WRITE(9). Attached is the ministat output; I hope it makes it. :-) Thanks, matthew x /data/zero-amd64-small/zero-512.txt + /data/zero-amd64-large/zero-512.txt +--+ | + x| |+ + +x*x+ +x x * x+x +x| | |__|__AM___MA___|___| | +--+ N Min MaxMedian AvgStddev x 10 13.564276 13.666499 13.590373 13.591993 0.030172083 + 10 13.49174 13.616263 13.569925 13.568006 0.033884281 No difference proven at 95.0% confidence x /data/zero-amd64-small/zero-1024.txt + /data/zero-amd64-large/zero-1024.txt +--+ |++ ++xx x x +* ++ xx++| | ||___AAM__M|_| | +--+ N Min MaxMedian AvgStddev x 10 7.155384 7.182849 7.168076 7.16613820.01041489 + 10 7.124263 7.207363 7.170449 7.1647896 0.023453662 No difference proven at 95.0% confidence x /data/zero-amd64-small/zero-2048.txt + /data/zero-amd64-large/zero-2048.txt +--+ | + | |+ + +xx *x +* xx+ ++xx x| ||_|A_M__M__A_|_| | +--+ N Min MaxMedian AvgStddev x 10 3.827242 3.867095 3.837901 3.839988 0.012983755 + 10 3.809213 3.843682 3.835748 3.8302765 0.011340307 No difference proven at 95.0% confidence x /data/zero-amd64-small/zero-4096.txt + /data/zero-amd64-large/zero-4096.txt +--+ |+ + ++xxx x + + * x+ ++ x x x| | |___AM_M_A___|_| | +--+ N Min MaxMedian AvgStddev x 10 2.165541 2.201224 2.173227 2.1769029 0.013803193 + 10 2.161362 2.185911 2.172388 2.1719634 0.0088129371 No difference proven at 95.0% confidence x /data/zero-amd64-small/zero-8192.txt + /data/zero-amd64-large/zero-8192.txt +--+ |+x| |+ x + + +x +x++x+ x xx + x x| | |__|___A__M_A_|___| |
Re: svn commit: r221853 - in head/sys: dev/md dev/null sys vm
On Tue, May 31, 2011 at 3:47 PM, m...@freebsd.org wrote: On Tue, May 31, 2011 at 2:48 PM, Pieter de Goeje pie...@degoeje.nl wrote: On Sunday 29 May 2011 05:01:57 m...@freebsd.org wrote: On Sat, May 28, 2011 at 12:03 PM, Pieter de Goeje pie...@degoeje.nl wrote: To me it looks like it's not able to cache the zeroes anymore. Is this intentional? I tried to change ZERO_REGION_SIZE back to 64K but that didn't help. Hmm. I don't have access to my FreeBSD box over the weekend, but I'll run this on my box when I get back to work. Meanwhile you could try setting ZERO_REGION_SIZE to PAGE_SIZE and I think that will restore things to the original performance. Indeed it does. I couldn't find any authoritative docs stating wether or not the cache on this CPU is virtually indexed, but apparently at least some of it is. On my physical box (some Dell thing from about 2008), I ran 10 loops of dd if=/dev/zero of=/dev/null bs=XX count=XX where bs went by powers of 2 from 512 bytes to 2M, and count was set so that the dd always transferred 8GB. I compared ZERO_REGION_SIZE of 64k and 2M on amd64. The summary of the ministat(1) output is: bs=512b - no difference bs=1K - no difference bs=2k - no difference bs=4k - no difference bs=8k - no difference bs=16k - no difference bs=32k - no difference bs=64k - no difference bs=128k - 2M is 0.69% faster bs=256k - 2M is 0.98% faster bs=512k - 2M is 0.65% faster bs=1M - 2M is 1.02% faster bs=2M - 2M is 2.17% slower I'll play again with a 4K buffer. The data is harder to parse precisely, but in general it looks like on my box using a 4K buffer results in significantly worse performance when the dd(1) block size is larger than 4K. How much worse depends on the block size, but it goes from 6% at bs=8k to 17% at bs=256k. Showing 4k/64k/2M ZERO_REGION_SIZE graphically in the ministat(1) output also makes it clear that the difference between 64k and 2M is nearly insignificant on my box compared to using 4k. http://people.freebsd.org/~mdf/zero-ministat.txt Cheers, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r221853 - in head/sys: dev/md dev/null sys vm
On Mon, May 30, 2011 at 8:25 AM, Bruce Evans b...@optusnet.com.au wrote: On Sat, 28 May 2011 m...@freebsd.org wrote: On Sat, May 28, 2011 at 12:03 PM, Pieter de Goeje pie...@degoeje.nl wrote: On Friday 13 May 2011 20:48:01 Matthew D Fleming wrote: Author: mdf Date: Fri May 13 18:48:00 2011 New Revision: 221853 URL: http://svn.freebsd.org/changeset/base/221853 Log: Usa a globally visible region of zeros for both /dev/zero and the md device. There are likely other kernel uses of blob of zeros than can be converted. Reviewed by: alc MFC after: 1 week This change seems to reduce /dev/zero performance by 68% as measured by this command: dd if=/dev/zero of=/dev/null bs=64k count=10. x dd-8-stable + dd-9-current +-+ |+ | Argh, hard \xa0. [...binary garbage deleted] This particular measurement was against 8-stable but the results are the same for -current just before this commit. Basically througput drops from ~13GB/sec to 4GB/sec. Hardware is a Phenom II X4 945 with 8GB of 800Mhz DDR2 memory. FreeBSD/amd64 is installed. This processor has 6MB of L3 cache. To me it looks like it's not able to cache the zeroes anymore. Is this intentional? I tried to change ZERO_REGION_SIZE back to 64K but that didn't help. Hmm. I don't have access to my FreeBSD box over the weekend, but I'll run this on my box when I get back to work. Meanwhile you could try setting ZERO_REGION_SIZE to PAGE_SIZE and I think that will restore things to the original performance. Using /dev/zero always thrashes caches by the amount source buffer size + target buffer size (unless the arch uses nontemporal memory accesses for uiomove, which none do AFAIK). So a large source buffer is always just a pessimization. A large target buffer size is also a pessimization, but for the target buffer a fairly large size is needed to amortize the large syscall costs. In this PR, the target buffer size is 64K. ZERO_REGION_SIZE is 64K on i386 and 2M on amd64. 64K+64K on i386 is good for thrashing the L1 cache. That depends -- is the cache virtually or physically addressed? The zero_region only has 4k (PAGE_SIZE) of unique physical addresses. So most of the cache thrashing is due to the user-space buffer, if the cache is physically addressed. It will only have a noticeable impact on a current L2 cache in competition with other threads. It is hard to fit everything in the L1 cache even with non-bloated buffer sizes and 1 thread (16 for the source (I)cache, 0 for the source (D)cache and 4K for the target cache might work). On amd64, 2M+2M is good for thrashing most L2 caches. In this PR, the thrashing is limited by the target buffer size to about 64K+64K, up from 4K+64K, and it is marginal whether the extra thrashing from the larger source buffer makes much difference. The old zbuf source buffer size of PAGE_SIZE was already too large. Wouldn't this depend on how far down from the use of the buffer the actual copy happens? Another advantage to a large virtual buffer is that it reduces the number of times the copy loop in uiomove has to return up to the device layer that initiated the copy. This is all pretty fast, but again assuming a physical cache fewer trips is better. Thanks, matthew The source buffer size only needs to be large enough to amortize loop overhead. 1 cache line is enough in most cases. uiomove() and copyout() unfortunately don't support copying from register space, so there must be a source buffer. This may limit the bandwidth by a factor of 2 in some cases, since most modern CPUs can execute either 2 64-bit stores or 1 64-bit store and 1 64-bit load per cycle if everything is already in the L1 cache. However, target buffers for /dev/zero (or any user i/o) probably need to be larger than the L1 cache to amortize the syscall overhead, so there are usually plenty of cycles to spare for the unnecessary loads while the stores wait for caches. This behaviour is easy to see for regular files too (regular files get copied out from the buffer cache). You have limited control on the amount of thrashing by changing the target buffer size, and can determine cache sizes by looking at throughputs. Bruce ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r222084 - head/contrib/gperf/src
On Wed, May 18, 2011 at 2:31 PM, Dimitry Andric d...@freebsd.org wrote: On 2011-05-18 23:16, Pawel Jakub Dawidek wrote: On Wed, May 18, 2011 at 09:06:20PM +, Ben Laurie wrote: Author: benl Date: Wed May 18 21:06:20 2011 New Revision: 222084 URL: http://svn.freebsd.org/changeset/base/222084 Log: Fix clang warnings. Approved by: philip (mentor) [...] - fprintf (stderr, by changing asso_value['%c'] (char #%d) to %d\n, + fprintf (stderr, by changing asso_value['%c'] (char #%zd) to %d\n, *p, p - union_set + 1, asso_values[(unsigned char)(*p)]); Hmm, both 'p' and 'union_set' are 'char *' and %zd is for ssize_t. It is a bit strange that it fixes the warning. If you subtract two pointers, such as in this case, you get a ptrdiff_t. Strictly, this doesn't have to be exactly the same type as ssize_t, but in practice it will almost always be. You can also cast the result to intmax_t, and use %jd, then it will always be correct, but possibly have some small overhead. Or you can use %td which is the C99 conversion specifier for ptrdiff_t. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r221993 - in head/sys: kern sys
On Mon, May 16, 2011 at 9:18 AM, Poul-Henning Kamp p...@freebsd.org wrote:
Author: phk
Date: Mon May 16 16:18:40 2011
New Revision: 221993
URL: http://svn.freebsd.org/changeset/base/221993
Log:
Change the length quantities of sbufs to be ssize_t rather than int.
Why?
I don't object at all to changing the API to return ssize_t (though I
see no need), but changing the struct breaks the KBI for no
perceivable gain. Do we really expect someone to put more than 2GB
into something that is a string buffer?
Thanks,
matthew
Constify a couple of arguments.
Modified:
head/sys/kern/subr_sbuf.c
head/sys/sys/sbuf.h
Modified: head/sys/kern/subr_sbuf.c
==
--- head/sys/kern/subr_sbuf.c Mon May 16 15:59:50 2011 (r221992)
+++ head/sys/kern/subr_sbuf.c Mon May 16 16:18:40 2011 (r221993)
@@ -94,7 +94,8 @@ _assert_sbuf_integrity(const char *fun,
KASSERT(s-s_buf != NULL,
(%s called with uninitialized or corrupt sbuf, fun));
KASSERT(s-s_len s-s_size,
- (wrote past end of sbuf (%d = %d), s-s_len, s-s_size));
+ (wrote past end of sbuf (%jd = %jd),
+ (intmax_t)s-s_len, (intmax_t)s-s_size));
}
static void
@@ -255,16 +256,17 @@ sbuf_clear(struct sbuf *s)
* Effectively truncates the sbuf at the new position.
*/
int
-sbuf_setpos(struct sbuf *s, int pos)
+sbuf_setpos(struct sbuf *s, ssize_t pos)
{
assert_sbuf_integrity(s);
assert_sbuf_state(s, 0);
KASSERT(pos = 0,
- (attempt to seek to a negative position (%d), pos));
+ (attempt to seek to a negative position (%jd), (intmax_t)pos));
KASSERT(pos s-s_size,
- (attempt to seek past end of sbuf (%d = %d), pos, s-s_size));
+ (attempt to seek past end of sbuf (%jd = %jd),
+ (intmax_t)pos, (intmax_t)s-s_size));
if (pos 0 || pos s-s_len)
return (-1);
@@ -640,7 +642,7 @@ sbuf_trim(struct sbuf *s)
* Check if an sbuf has an error.
*/
int
-sbuf_error(struct sbuf *s)
+sbuf_error(const struct sbuf *s)
{
return (s-s_error);
@@ -691,7 +693,7 @@ sbuf_data(struct sbuf *s)
/*
* Return the length of the sbuf data.
*/
-int
+ssize_t
sbuf_len(struct sbuf *s)
{
@@ -728,7 +730,7 @@ sbuf_delete(struct sbuf *s)
* Check if an sbuf has been finished.
*/
int
-sbuf_done(struct sbuf *s)
+sbuf_done(const struct sbuf *s)
{
return (SBUF_ISFINISHED(s));
Modified: head/sys/sys/sbuf.h
==
--- head/sys/sys/sbuf.h Mon May 16 15:59:50 2011 (r221992)
+++ head/sys/sys/sbuf.h Mon May 16 16:18:40 2011 (r221993)
@@ -44,8 +44,8 @@ struct sbuf {
sbuf_drain_func *s_drain_func; /* drain function */
void *s_drain_arg; /* user-supplied drain argument */
int s_error; /* current error code */
- int s_size; /* size of storage buffer */
- int s_len; /* current length of string */
+ ssize_t s_size; /* size of storage buffer */
+ ssize_t s_len; /* current length of string */
#define SBUF_FIXEDLEN 0x /* fixed length buffer
(default) */
#define SBUF_AUTOEXTEND 0x0001 /* automatically extend
buffer */
#define SBUF_USRFLAGMSK 0x /* mask of flags the user may
specify */
@@ -63,7 +63,7 @@ struct sbuf *sbuf_new(struct sbuf *, cha
#define sbuf_new_auto() \
sbuf_new(NULL, NULL, 0, SBUF_AUTOEXTEND)
void sbuf_clear(struct sbuf *);
-int sbuf_setpos(struct sbuf *, int);
+int sbuf_setpos(struct sbuf *, ssize_t);
int sbuf_bcat(struct sbuf *, const void *, size_t);
int sbuf_bcpy(struct sbuf *, const void *, size_t);
int sbuf_cat(struct sbuf *, const char *);
@@ -75,11 +75,11 @@ int sbuf_vprintf(struct sbuf *, const
int sbuf_putc(struct sbuf *, int);
void sbuf_set_drain(struct sbuf *, sbuf_drain_func *, void *);
int sbuf_trim(struct sbuf *);
-int sbuf_error(struct sbuf *);
+int sbuf_error(const struct sbuf *);
int sbuf_finish(struct sbuf *);
char *sbuf_data(struct sbuf *);
-int sbuf_len(struct sbuf *);
-int sbuf_done(struct sbuf *);
+ssize_t sbuf_len(struct sbuf *);
+int sbuf_done(const struct sbuf *);
void sbuf_delete(struct sbuf *);
#ifdef _KERNEL
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r220755 - in head: . contrib/gcc/doc contrib/gcc/objc contrib/libobjc etc/mtree gnu/lib gnu/lib/libobjc gnu/usr.bin/cc gnu/usr.bin/cc/cc1obj gnu/usr.bin/cc/cc_tools gnu/usr.bin/cc/doc
Trimming since I have a mostly-unrelated question... On Tue, Apr 19, 2011 at 5:40 AM, John Baldwin j...@freebsd.org wrote: On Monday, April 18, 2011 3:59:45 pm Warner Losh wrote: In this case, there was a new kernel thing just after, so it turned out OK. But let's not gratuitously bump the version since the granularity we have already allows the ports to make good choices on when to leave something in or out. Except that that directly contradicts our previously established policy that these version bumps are cheap and that we should do more of them (this came up a few years ago when we changed the policy so that the new stable branch after a release starts at N + 500 (e.g. 802500) rather than N + 100 to give more room for version bumps on current). I thought I remembered reading (within the past 2 years) that __FreeBSD_version should not be incremented more than once a day, since there was a limit of 100 before the version minor number was affected. Did I get the polarity backwards and that was the old policy? Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r220791 - in head: lib/libc/sys sys/compat/freebsd32 sys/kern sys/sys
On Mon, Apr 18, 2011 at 12:28 PM, Kostik Belousov kostik...@gmail.com wrote:
On Mon, Apr 18, 2011 at 04:32:22PM +, Matthew D Fleming wrote:
Author: mdf
Date: Mon Apr 18 16:32:22 2011
New Revision: 220791
URL: http://svn.freebsd.org/changeset/base/220791
Log:
Add the posix_fallocate(2) syscall. The default implementation in
vop_stdallocate() is filesystem agnostic and will run as slow as a
read/write loop in userspace; however, it serves to correctly
implement the functionality for filesystems that do not implement a
VOP_ALLOCATE.
Note that __FreeBSD_version was already bumped today to 900036 for any
ports which would like to use this function.
Also reserve space in the syscall table for posix_fadvise(2).
+#ifdef __notyet__
+ /*
+ * Check if the filesystem sets f_maxfilesize; if not use
+ * VOP_SETATTR to perform the check.
+ */
+ error = VFS_STATFS(vp-v_mount, sfs, td);
+ if (error != 0)
+ goto out;
+ if (sfs.f_maxfilesize) {
+ if (offset sfs.f_maxfilesize || len sfs.f_maxfilesize ||
+ offset + len sfs.f_maxfilesize) {
+ error = EFBIG;
+ goto out;
+ }
+ } else
+#endif
+ if (offset + len vap-va_size) {
+ VATTR_NULL(vap);
+ vap-va_size = offset + len;
+ error = VOP_SETATTR(vp, vap, td-td_ucred);
+ if (error != 0)
+ goto out;
+ }
I still do not see a reason to do VOP_SETATTR() there. VOP_WRITE() will
do auto-extend as needed. Also, see below.
The need is, as commented, to return EFBIG when the new file size will
be larger than the FS supports. Without this code, passing in
something like posix_fallocate(fd, 0, OFF_MAX) will run the filesystem
out of space.
+
+ while (len 0) {
+ if (should_yield()) {
+ VOP_UNLOCK(vp, 0);
+ locked = 0;
+ kern_yield(-1);
Please note that, despite dropping the vnode lock, the snapshot creation
is still blocked at this point, due to previous vn_start_write().
If doing vn_finished_write() there, then bwillwrite() before
next iteration is desired.
+ error = vn_lock(vp, LK_EXCLUSIVE);
+ if (error != 0)
+ break;
+ locked = 1;
+ error = VOP_GETATTR(vp, vap, td-td_ucred);
+ if (error != 0)
+ break;
+ }
+
+ /*
+ * Read and write back anything below the nominal file
+ * size. There's currently no way outside the filesystem
+ * to know whether this area is sparse or not.
+ */
+ cur = iosize;
+ if ((offset % iosize) != 0)
+ cur -= (offset % iosize);
+ if (cur len)
+ cur = len;
+ if (offset vap-va_size) {
+ aiov.iov_base = buf;
+ aiov.iov_len = cur;
+ auio.uio_iov = aiov;
+ auio.uio_iovcnt = 1;
+ auio.uio_offset = offset;
+ auio.uio_resid = cur;
+ auio.uio_segflg = UIO_SYSSPACE;
+ auio.uio_rw = UIO_READ;
+ auio.uio_td = td;
+ error = VOP_READ(vp, auio, 0, td-td_ucred);
+ if (error != 0)
+ break;
+ if (auio.uio_resid 0) {
+ bzero(buf + cur - auio.uio_resid,
+ auio.uio_resid);
+ }
+ } else {
+ bzero(buf, cur);
+ }
Wouldn't VOP_SETATTR() at the start of the function mostly prevent
this bzero from executing ?
Yes. If struct statfs had a member indicating the file system's max
file size, then the extend wouldn't be necessary. We have that
feature locally, but it's only implemented for ufs and our custom file
system, and it requires an ABI change so it's a bit of work to
upstream. And as with most of those things, it's hard to find the
time to upstream it outside of work hours.
I estimated what it would take to do the optimized implementation for UFS,
and I think that the following change would allow to lessen the code
duplication much.
What if the vnode lock drop and looping be handled by the syscall, instead
of the vop implementation ? In other words, allow the VOP_ALLOCATE()
to allocate less then requested, and return the allocated amount to
the caller. The loop would be centralized then, freeing fs from doing
the dance. Also, if fs considers that suitable, it would do a whole
allocation in one run.
I like this idea. Perhaps input to the vop should be pointers to
offset and len
Re: svn commit: r220791 - in head: lib/libc/sys sys/compat/freebsd32 sys/kern sys/sys
On Mon, Apr 18, 2011 at 12:47 PM, Pawel Jakub Dawidek p...@freebsd.org wrote:
On Mon, Apr 18, 2011 at 10:28:10PM +0300, Kostik Belousov wrote:
+ if (offset + len vap-va_size) {
+ VATTR_NULL(vap);
+ vap-va_size = offset + len;
+ error = VOP_SETATTR(vp, vap, td-td_ucred);
+ if (error != 0)
+ goto out;
+ }
I still do not see a reason to do VOP_SETATTR() there. VOP_WRITE() will
do auto-extend as needed. Also, see below.
Yeah, also when we extend file size we could skip reading zeros.
+ if (offset vap-va_size) {
[...]
+ } else {
+ bzero(buf, cur);
+ }
Wouldn't VOP_SETATTR() at the start of the function mostly prevent
this bzero from executing ?
Once we drop the vnode lock, the file size can change under us, no?
Yes, which is why the VOP_GETATTR is re-run. The SETATTR doesn't need
to be re-run, since the purpose was just to check that offset + len is
less than the filesystem's maximum supported file size.
I estimated what it would take to do the optimized implementation for UFS,
and I think that the following change would allow to lessen the code
duplication much.
What if the vnode lock drop and looping be handled by the syscall, instead
of the vop implementation ? In other words, allow the VOP_ALLOCATE()
to allocate less then requested, and return the allocated amount to
the caller. The loop would be centralized then, freeing fs from doing
the dance. Also, if fs considers that suitable, it would do a whole
allocation in one run.
I'd still go with SEEK_DATA/SEEK_HOLE loop as I suggested on arch@.
If you would like to spend time on it, having SEEK_DATA/SEEK_HOLE
support in UFS would be beneficial for other purposes too.
Well, if we had this functionality it could be used. I'd like the
framework but the filesystems need modification to support it. We
want it for OneFS at Isilon so eventually when I get to this part of
the project I'll have some wrapper code if someone doesn't get there
first.
Cheers,
matthew
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r220791 - in head: lib/libc/sys sys/compat/freebsd32 sys/kern sys/sys
2011/4/18 Kostik Belousov kostik...@gmail.com: On Mon, Apr 18, 2011 at 12:49:38PM -0700, m...@freebsd.org wrote: On Mon, Apr 18, 2011 at 12:28 PM, Kostik Belousov kostik...@gmail.com wrote: On Mon, Apr 18, 2011 at 04:32:22PM +, Matthew D Fleming wrote: Author: mdf Date: Mon Apr 18 16:32:22 2011 New Revision: 220791 URL: http://svn.freebsd.org/changeset/base/220791 Log: Add the posix_fallocate(2) syscall. The default implementation in vop_stdallocate() is filesystem agnostic and will run as slow as a read/write loop in userspace; however, it serves to correctly implement the functionality for filesystems that do not implement a VOP_ALLOCATE. Note that __FreeBSD_version was already bumped today to 900036 for any ports which would like to use this function. Also reserve space in the syscall table for posix_fadvise(2). The need is, as commented, to return EFBIG when the new file size will be larger than the FS supports. Without this code, passing in something like posix_fallocate(fd, 0, OFF_MAX) will run the filesystem out of space. Handling max file size and not overflowing the fs are different things. VOP_WRITE() will handle file size on its own too. I see no problem with exhausting free space if this is what user asked for. This violates the standard, though, since it would return ENOSPC instead of EFBIG. Also, this is just the default implementation. I'm adding a second VOP_SETATTR in vop_stdallocate() to restore the file size after extending, which will let the implementation use bzero/VOP_WRITE instead of VOP_READ/VOP_WRITE when past the original EOF. This is more of an issue when calling the vop iteratively, since subsequent calls don't know what the correct file size is. Cheers, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r218685 - head/sys/dev/acpica
On Mon, Feb 14, 2011 at 10:33 AM, John Baldwin j...@freebsd.org wrote: On Monday, February 14, 2011 12:20:20 pm Matthew D Fleming wrote: Author: mdf Date: Mon Feb 14 17:20:20 2011 New Revision: 218685 URL: http://svn.freebsd.org/changeset/base/218685 Log: Prevent reading from the ACPI_RESOURCE past its actual end. For paranoia limit to the size of the ACPI_RESOURCE as well. I think in practice that len would never be sizeof(ACPI_RESOURCE). You could probably get by with using a KASSERT() instead: KASSERT(res-Length = sizeof(ACPI_RESOURCE), resource too large)); bcopy(res, req-acpi_res, res-Length); Thanks. I wanted to be paranoid since the problem was sporadic. Anyone who can better test this code should feel free to modify it further. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r218424 - in head/sys: dev/sio kern pc98/cbus sys ufs/ffs
On Tue, Feb 8, 2011 at 1:18 AM, Kostik Belousov kostik...@gmail.com wrote: On Tue, Feb 08, 2011 at 12:16:36AM +, Matthew D Fleming wrote: Author: mdf Date: Tue Feb 8 00:16:36 2011 New Revision: 218424 URL: http://svn.freebsd.org/changeset/base/218424 Log: Based on discussions on the svn-src mailing list, rework r218195: - entirely eliminate some calls to uio_yeild() as being unnecessary, such as in a sysctl handler. Are you sure ? Please see r185983. I eliminated the uio_yield inside two sysctl handlers, not in userland_sysctl() itself. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r218195 - in head/sys: amd64/amd64 arm/arm i386/i386 ia64/ia64 kern mips/mips powerpc/powerpc sparc64/sparc64 sun4v/sun4v sys ufs/ffs
On Thu, Feb 3, 2011 at 4:50 AM, John Baldwin j...@freebsd.org wrote: On Thursday, February 03, 2011 2:47:20 am Juli Mallett wrote: On Wed, Feb 2, 2011 at 08:35, Matthew D Fleming m...@freebsd.org wrote: Author: mdf Date: Wed Feb 2 16:35:10 2011 New Revision: 218195 URL: http://svn.freebsd.org/changeset/base/218195 Log: Put the general logic for being a CPU hog into a new function should_yield(). Use this in various places. Encapsulate the common case of check-and-yield into a new function maybe_yield(). Change several checks for a magic number of iterations to use should_yield() instead. First off, I admittedly don't know or care very much about this area, but this commit stood out to me and I had a few minor concerns. I'm slightly uncomfortable with the flat namespace here. It isn't obvious from the names that maybe_yield() and should_yield() relate only to uio_yield() and not other types of yielding (from DELAY() to cpu_idle() to sched_yield().) The other problematic element here is that maybe_yield and should_yield could quite reasonably be variables or functions in existing code in the kernel, and although we don't try to protect against changes that could cause such collisions, we shouldn't do them gratuitously, and there's even something that seems aesthetically off about these; they seem...informal, even Linuxy. I think names like uio_should_yield() and uio_maybe_yield() wouldn't have nearly as much of a problem, since the context of the question of should is isolated to uio operations rather than, say, whether the scheduler would *like* for us, as the running thread, to yield, or other considerations that may be more general. I mostly agree, but these checks are no longer specific to uio. Matt used them to replace many ad-hoc checks using counters with hardcoded maximums in places like softupdates, etc. I don't have any good suggestions for what else you would call these. I'm not sure 'sched_amcpuhog() or sched_hoggingcpu()' are really better (and these are not scheduler dependent, so sched_ would probably not be a good prefix). Bruce correctly points out that the code doesn't work like I expect with PREEMPTION, which most people will be running. I'm thinking of adding a new per-thread field to record the last ticks value that a voluntary mi_switch() was done, so that there's a standard way of checking if a thread is being a hog; this will work for both PREEMPTION and !PREEMPTION, and would be appropriate for the places that previously used a counter. (This would require uio_yield() to be SW_VOL, but I can't see why it's not a voluntary context switch anyways). I'm happy to rename the functions (perhaps just yield_foo() rather than foo_yield()?) and stop using uio_yield as the base name since it's not a uio function. I wanted to keep the uio_yield symbol to preserve the KBI/KPI. Any suggestions for names are welcome. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r217830 - head/share/man/man9
On Wed, Jan 26, 2011 at 1:37 AM, Robert Watson rwat...@freebsd.org wrote: On Tue, 25 Jan 2011, Matthew D Fleming wrote: .Dv SBUF_AUTOEXTEND . .Pp The +.Fn sbuf_new_for_sysctl +function will set up an sbuf with a drain function to use +.Fn SYSCTL_OUT +when the internal buffer fills. +The sysctl old buffer will be wired, which allows for doing an +.Fn sbuf_printf +while holding a mutex. +.Pp +The .Fn sbuf_delete function clears the .Fa sbuf Hmm. Is this description missing mention of how wiring failures are handled? (Also, it should probably mention that this call can sleep for potentially quite long periods of time, even if sbuf_printf (and friends) can't). I'm not sure how much to write, since some of the wiring failures are dealt with by the sysctl subsystem and are not documented. The current state of the actual code is that a failure in vslock(9) is ignored, unless it's ENOMEM in which case sysctl_wire_old_buffer sets the sysctl_req-validlen to 0, which would behave perhaps slightly unexpectedly for the user since no data will be copied out. Any non-ENOMEM failure from vslock() presumably would also have been a failure from SYSCTL_OUT and this does get squashed, perhaps incorrectly. I'll think about saving the error code so that sbuf_finish can report it if nothing else has gone wrong. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r217830 - head/share/man/man9
On Wed, Jan 26, 2011 at 9:55 AM, Robert N. M. Watson rwat...@freebsd.org wrote: On 26 Jan 2011, at 17:12, m...@freebsd.org wrote: Hmm. Is this description missing mention of how wiring failures are handled? (Also, it should probably mention that this call can sleep for potentially quite long periods of time, even if sbuf_printf (and friends) can't). I'm not sure how much to write, since some of the wiring failures are dealt with by the sysctl subsystem and are not documented. The current state of the actual code is that a failure in vslock(9) is ignored, unless it's ENOMEM in which case sysctl_wire_old_buffer sets the sysctl_req-validlen to 0, which would behave perhaps slightly unexpectedly for the user since no data will be copied out. Any non-ENOMEM failure from vslock() presumably would also have been a failure from SYSCTL_OUT and this does get squashed, perhaps incorrectly. I'll think about saving the error code so that sbuf_finish can report it if nothing else has gone wrong. Yeah, no specific opinions on the right answer, except perhaps that sbuf_new_for_sysctl() failing due to ENOMEM is something worth making it easy to report to the user. The ENOMEM is already managed and squashed inside sysctl_wire_old_buffer(), so there's no way for sbuf_new_for_sysctl() to report it. It may end up happening automagically since it sets the validlen to 0. I suppose an important question is now often we see this actually failing I don't believe we've ever seen a memory failure relating to sysctls at Isilon and we've been using the equivalent of this code for a few years. Our machines aren't low memory but they are under memory pressure sometimes. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r217830 - head/share/man/man9
On Wed, Jan 26, 2011 at 1:10 PM, Robert N. M. Watson rwat...@freebsd.org wrote: On 26 Jan 2011, at 18:29, m...@freebsd.org wrote: I suppose an important question is now often we see this actually failing I don't believe we've ever seen a memory failure relating to sysctls at Isilon and we've been using the equivalent of this code for a few years. Our machines aren't low memory but they are under memory pressure sometimes. The kinds of cases I worry about are things like the tcp connection monitoring sysctls. Most systems have a dozen, hundred, or a thousand connections. Some have half a million or a million. If we switched to requiring wiring every page needed to store that list, it would do terrible things to the system. So really what I have in mind is: either we handle cases like that well, or we put in a clear warning and have obvious failure modes to catch the cases where it didn't work out. In practice, I think we would not want to switch the tcpcb/inpcb sysctl for this reason, but as people say ah, this is convenient we need to make sure it's handled well, and easy to debug problems when they do arise. But I think that problem exists today using sysctl for output, since it's non-iterative. In fact, it's often worse today, because in addition to the user-space buffer that needs to be large enough to hold the output, the kernel needs to malloc(9) a buffer to hold it before doing the one SYSCTL_OUT at the end that most routines I've seen use. For situations like this where there is a lot of output but it doesn't need to be serialized by a lock held across the whole data fetch, then yes, using sbuf_new_for_sysctl() would wire more memory. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r217830 - head/share/man/man9
On Wed, Jan 26, 2011 at 1:19 PM, Robert N. M. Watson rwat...@freebsd.org wrote: On 26 Jan 2011, at 21:14, m...@freebsd.org wrote: The kinds of cases I worry about are things like the tcp connection monitoring sysctls. Most systems have a dozen, hundred, or a thousand connections. Some have half a million or a million. If we switched to requiring wiring every page needed to store that list, it would do terrible things to the system. So really what I have in mind is: either we handle cases like that well, or we put in a clear warning and have obvious failure modes to catch the cases where it didn't work out. In practice, I think we would not want to switch the tcpcb/inpcb sysctl for this reason, but as people say ah, this is convenient we need to make sure it's handled well, and easy to debug problems when they do arise. But I think that problem exists today using sysctl for output, since it's non-iterative. In fact, it's often worse today, because in addition to the user-space buffer that needs to be large enough to hold the output, the kernel needs to malloc(9) a buffer to hold it before doing the one SYSCTL_OUT at the end that most routines I've seen use. For situations like this where there is a lot of output but it doesn't need to be serialized by a lock held across the whole data fetch, then yes, using sbuf_new_for_sysctl() would wire more memory. Right -- hence my concern about (a) appropriate documentation and (b) proper error handling. The sbuf routine looks convenient, easy to use, and exactly the semantic that I want in most cases. However, sometimes, it may silently break based on something rather abstract getting too big. We need users of the KPI to be aware of that limitation and hence not use it when that could occur, and when it does occur, generate a clear notice of some sort so that it can be tracked down. Upon further consideration, I don't think sbuf_new_for_sysctl() should be doing the wire. Whether the buffer needs to be wired or not is up to the implementation of the individual sysctl; *most* of them will be holding a lock when doing sbuf_print, but there's no guarantee. It's simpler to just leave this in the hands of the implementor, and it also enables better error reporting. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r217748 - head/sys/netinet/cc
For sbuf use for a sysctl you can use sbuf_init_for_sysctl() which
will, instead of growing, push the current data out using SYSCTL_OUT
to a wired user buffer. There's a few examples in the vm/ code. This
can sometimes significantly simplify the code since there's no need to
worry about held mutex/rwlock anymore.
Thanks,
matthew
On Sun, Jan 23, 2011 at 5:00 AM, Lawrence Stewart lstew...@freebsd.org wrote:
Author: lstewart
Date: Sun Jan 23 13:00:25 2011
New Revision: 217748
URL: http://svn.freebsd.org/changeset/base/217748
Log:
An sbuf configured with SBUF_AUTOEXTEND will call malloc with M_WAITOK when a
write to the buffer causes it to overflow. We therefore can't hold the CC
list
rwlock over a call to sbuf_printf() for an sbuf configured with
SBUF_AUTOEXTEND.
Switch to a fixed length sbuf which should be of sufficient size except in
the
very unlikely event that the sysctl is being processed as one or more new
algorithms are loaded. If that happens, we accept the race and may fail the
sysctl gracefully if there is insufficient room to print the names of all the
algorithms.
This should address a WITNESS warning and the potential panic that would
occur
if the sbuf call to malloc did sleep whilst holding the CC list rwlock.
Sponsored by: FreeBSD Foundation
Reported by: Nick Hibma
Reviewed by: bz
MFC after: 3 weeks
X-MFC with: r215166
Modified:
head/sys/netinet/cc/cc.c
Modified: head/sys/netinet/cc/cc.c
==
--- head/sys/netinet/cc/cc.c Sun Jan 23 12:44:17 2011 (r217747)
+++ head/sys/netinet/cc/cc.c Sun Jan 23 13:00:25 2011 (r217748)
@@ -128,20 +128,37 @@ cc_list_available(SYSCTL_HANDLER_ARGS)
{
struct cc_algo *algo;
struct sbuf *s;
- int err, first;
+ int err, first, nalgos;
- err = 0;
+ err = nalgos = 0;
first = 1;
- s = sbuf_new(NULL, NULL, TCP_CA_NAME_MAX, SBUF_AUTOEXTEND);
+
+ CC_LIST_RLOCK();
+ STAILQ_FOREACH(algo, cc_list, entries) {
+ nalgos++;
+ }
+ CC_LIST_RUNLOCK();
+
+ s = sbuf_new(NULL, NULL, nalgos * TCP_CA_NAME_MAX, SBUF_FIXEDLEN);
if (s == NULL)
return (ENOMEM);
+ /*
+ * It is theoretically possible for the CC list to have grown in size
+ * since the call to sbuf_new() and therefore for the sbuf to be too
+ * small. If this were to happen (incredibly unlikely), the sbuf will
+ * reach an overflow condition, sbuf_printf() will return an error and
+ * the sysctl will fail gracefully.
+ */
CC_LIST_RLOCK();
STAILQ_FOREACH(algo, cc_list, entries) {
err = sbuf_printf(s, first ? %s : , %s, algo-name);
- if (err)
+ if (err) {
+ /* Sbuf overflow condition. */
+ err = EOVERFLOW;
break;
+ }
first = 0;
}
CC_LIST_RUNLOCK();
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r217369 - in head/sys: cam/scsi sys
On Sat, Jan 15, 2011 at 6:55 AM, Bruce Evans b...@optusnet.com.au wrote: On Sat, 15 Jan 2011, Garrett Cooper wrote: On Fri, Jan 14, 2011 at 10:27 PM, Bruce Evans b...@optusnet.com.au wrote: On Fri, 14 Jan 2011, Garrett Cooper wrote: On Fri, Jan 14, 2011 at 6:42 PM, Bruce Evans b...@optusnet.com.au wrote: ... Oops. I think sizeof() and issigned() can be used to determine the type well enough in functions and initialized data (do a fuller type check if the compiler supports it), but I don't know how to do this in static sysctl declarations (since sizeof() can't be used in cpp expressions). Why not just create some dumb testcases that can be run at build time to determine that for you? Well, how? You are given SYSCTL_I(var, ...) and have to convert this to what is now in SYSCTL_INT(), using only the type of var, in hundreds or thousands of files. I don't even know how to do this with a test ... /* Only works for arithmetic types: */ #define isinteger(var) ((typeof(var))0.1 == 0) #define issigned(var) ((typeof(var))-1 0) ... This is what I meant: $ cat test_warnings.c #include sys/types.h size_t x = (int) -1; int y = 200L; $ gcc -Wconversion -Wstrict-overflow -Wsign-compare -c test_warnings.c test_size_t.c:3: warning: negative integer implicitly converted to unsigned type test_size_t.c:4: warning: overflow in implicit constant conversion $ With the right CFLAGS and a few properly written tests, and a few make rules, you can figure out what's what pretty easily *shrugs*. That's a lot more parsing than seems reasonable. Anyway, we already depend on gnu __typeof() being avaible for the much more central API of pcpu. All pcpu accesses on amd64 and i386 use it, except curthread (curthread has been hacked to use a more direct asm for compile-time efficiency). SYSCTL_I() works even better that I first thought. It automatically gives support for all typedefed integral types. No SYSCTL_FOO_T()s with messy MD ifdefs for matching up foo_t with an integral type are needed. Instead there are even messier MI ifdefs in SYSCTL_I() :-). But not so many. There are hundreds of typedefed types of interest, but only 8 different integer types to map to (8/16/32/64 bits signed and unsigned). The complication that int64_t is plain long on 64 bit arches but long long on 32-bit arches still arises. CTLTYPE_QUAD can be associated with int64_t on all arches, but the format for printing these is arch-dependent. (Note that quad_t's cannot be printed using %qd, and %qd is unusable, since %qd is just an alias for %lld, while quad_t is an alias for int64_t and int64_t is plain long on 64-bit arches so its format is %ld on these arches and %lld on others.) The printing is done entirely in user-space, so it's not too bad. I had figured to upcast everything to [u]intmax_t anyways, and what to cast from would just be [u]int32_t and [u]int64_t depending on reported size. Anything smaller should be copyout(9)'d as a 4 bit quantity. But, there's two factors at play here. The first is sysctl(8) which asks for the size when reading and manages it. The second is sysctl(2) where we have code like SCTL_MASK32 so a long on a 64-bit kernel looks like a long to a 32-bit app. It would be simpler to expose this as an 8-byte quantity on 64-bit kernel, and sysctl(8) will deal with this just fine, but that would break some uses of sysctl(2). However, I *think* it is safe to check the sysctl_req's oldidx/oldlen to see what size the user-space expects and cast a kernel long to int before SYSCTL_OUT. $WORK has gotten busy so I haven't had a chance to work on this much but I'm hopeful that the missus will watch the kids this weekend and allow me to hack. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r217369 - in head/sys: cam/scsi sys
On Sat, Jan 15, 2011 at 7:06 PM, Bruce Evans b...@optusnet.com.au wrote: On Sat, 15 Jan 2011 m...@freebsd.org wrote: On Sat, Jan 15, 2011 at 6:55 AM, Bruce Evans b...@optusnet.com.au wrote: The printing is done entirely in user-space, so it's not too bad. I had figured to upcast everything to [u]intmax_t anyways, and what to cast from would just be [u]int32_t and [u]int64_t depending on reported size. Anything smaller should be copyout(9)'d as a 4 bit quantity. I think it shouldn't be converted in the kernel. Applications using sysctl already have to deal with fields shorter than int in structs returned by sysctl(). They don't need to deal with integers shorter than int only since sysctl() doesn't support such integers yet. Short integers could still be returned as essentially themselves by packing them into a struct with nothing else. But, there's two factors at play here. The first is sysctl(8) which asks for the size when reading and manages it. The second is sysctl(2) where we have code like SCTL_MASK32 so a long on a 64-bit kernel looks like a long to a 32-bit app. It would be simpler to expose this as an 8-byte quantity on 64-bit kernel, and sysctl(8) will deal with this just fine, but that would break some uses of sysctl(2). What are the uses? I guess they are not-so-hard-coding data types as int and long, etc., and hard-coding them as int32_t and int64_t. Then you get an error when the kernel returns an unexpected length, but unexpected lengths are expected for 32-bit apps on 64-bit kernels. BTW, short writes are still horribly broken. Suppose there is a size mismatch causing an application tries to write 32 bits to a 64-bit kernel variable. Then no error is detected, and the kernel variable ois left with garbage in its top bits. The error detection last worked with 4.4BSD sysctl, and is still documented to work: % [EINVAL] A non-null newp is given and its specified length in % newlen is too large or too small. I think the case where newlen is too large still works. I guess the bug is potentially larger for 32 bit compat applications, but it is rarely seen for those too since only system management applications should be writing to kernel variables and there is no reason to run such applications in 32 bit mode. At Isilon all of userland is still 32-bit only. The reason is that for a while we supported 32-bit and 64-bit kernels in the field, and in fact for various reasons on install the 32-bit kernel always came up first and a reboot after install was forced for 64-bit capable hardware. But we needed the same userspace to work on both kernels. However, I *think* it is safe to check the sysctl_req's oldidx/oldlen to see what size the user-space expects and cast a kernel long to int before SYSCTL_OUT. Maybe if the value fits. The application might be probing for a size that works. Then it shouldn't care what size the value is returned in, provided the value fits. Writing is only slightly more delicate. The application must use a size in which the value fits. Then the kernel can expand the size if necessary. It just has to be careful to fill the top bits and not have sign extension bugs. sysctl_handle_i() can probably handle both directions. Lower-level sysctl code should still return an error if there is a size mismatch. Probing for a size that works should be passing in NULL for the old ptr to get a hint, and for scalars the sie doesn't change. It would be a somewhat malformed app that probes for the size anywhere below e.g. 512 bytes or 1 page. If a value doesn't fit, then the application will just have to detect the error and try a larger size (or fail if it doesn't support larger size). An example might be a 32-bit app reading 64-bit kernel pointers. These probably have the high bits set, so they won't fit in 32-bit sizes. But the application can easily read them using 64-bit sizes. Integers and pointers larger than 64 bits would be more interesting, since a compat application might not have any integer data type larger enough to represent them. So I changed my mind about converting in the kernel. Just try to convert to whatever size the application is trying to use. Don't even force = 32 bit sizes on applications. The proposed code does attempt to convert to whatever the app uses, but it assumes the app is using at least a 32-bit quantity. :-) More on the other thread. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r217369 - in head/sys: cam/scsi sys
On Thu, Jan 13, 2011 at 10:50 PM, Bruce Evans b...@optusnet.com.au wrote: On Thu, 13 Jan 2011 m...@freebsd.org wrote: There appear to be 330 uses of SYSCTL and QUAD on the same line in CURRENT. This seems reasonable to change them to S64, U64 and X64 so they correctly reflect the size they operate upon. What do y'all think? Now I suggest delaying this until they can be renamed to a type- generic SYSCTL_INT() (would probably need to be spelled differently, SYSCTL_I() say, even if SYSCTL_INT() was changed at the same time). I'm torn on this one. The compiler knows the type (unless, for SYSCTL_INT, NULL/0 is used, but that is also a compile-time check), but to interpret it requires the use of __builtin_foo which is a gcc extension and not part of standard C. Philosophically, while I like this kind of letting the compiler do the work, if you want C++ you know where to find it. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r217369 - in head/sys: cam/scsi sys
There appear to be 330 uses of SYSCTL and QUAD on the same line in
CURRENT. This seems reasonable to change them to S64, U64 and X64 so
they correctly reflect the size they operate upon.
What do y'all think?
Thanks,
matthew
On Thu, Jan 13, 2011 at 10:20 AM, Matthew D Fleming m...@freebsd.org wrote:
Author: mdf
Date: Thu Jan 13 18:20:33 2011
New Revision: 217369
URL: http://svn.freebsd.org/changeset/base/217369
Log:
Add a 64-bit hex-printed sysctl(9) since there is at least one place in
the code that wanted it. It is named X64 rather than XQUAD since the
quad name is a historical abomination that should not be perpetuated.
Modified:
head/sys/cam/scsi/scsi_da.c
head/sys/sys/sysctl.h
Modified: head/sys/cam/scsi/scsi_da.c
==
--- head/sys/cam/scsi/scsi_da.c Thu Jan 13 18:20:27 2011 (r217368)
+++ head/sys/cam/scsi/scsi_da.c Thu Jan 13 18:20:33 2011 (r217369)
@@ -1127,9 +1127,9 @@ dasysctlinit(void *context, int pending)
struct ccb_trans_settings_fc *fc = cts.xport_specific.fc;
if (fc-valid CTS_FC_VALID_WWPN) {
softc-wwpn = fc-wwpn;
- SYSCTL_ADD_XLONG(softc-sysctl_ctx,
+ SYSCTL_ADD_X64(softc-sysctl_ctx,
SYSCTL_CHILDREN(softc-sysctl_tree),
- OID_AUTO, wwpn, CTLTYPE_QUAD | CTLFLAG_RD,
+ OID_AUTO, wwpn, CTLFLAG_RD,
softc-wwpn, World Wide Port Name);
}
}
Modified: head/sys/sys/sysctl.h
==
--- head/sys/sys/sysctl.h Thu Jan 13 18:20:27 2011 (r217368)
+++ head/sys/sys/sysctl.h Thu Jan 13 18:20:33 2011 (r217369)
@@ -245,6 +245,8 @@ SYSCTL_ALLOWED_TYPES(ULONG, unsigned lon
SYSCTL_ALLOWED_TYPES(XLONG, unsigned long *a; long *b; );
SYSCTL_ALLOWED_TYPES(INT64, int64_t *a; long long *b; );
SYSCTL_ALLOWED_TYPES(UINT64, uint64_t *a; unsigned long long *b; );
+SYSCTL_ALLOWED_TYPES(XINT64, uint64_t *a; int64_t *b;
+ unsigned long long *c; long long *d; );
#ifdef notyet
#define SYSCTL_ADD_ASSERT_TYPE(type, ptr) \
@@ -389,7 +391,6 @@ SYSCTL_ALLOWED_TYPES(UINT64, uint64_t *a
SYSCTL_ADD_ASSERT_TYPE(INT64, ptr), 0, \
sysctl_handle_quad, Q, __DESCR(descr))
-/* Oid for a quad. The pointer must be non NULL. */
#define SYSCTL_UQUAD(parent, nbr, name, access, ptr, val, descr)
\
SYSCTL_ASSERT_TYPE(UINT64, ptr, parent, name); \
SYSCTL_OID(parent, nbr, name, \
@@ -402,6 +403,18 @@ SYSCTL_ALLOWED_TYPES(UINT64, uint64_t *a
SYSCTL_ADD_ASSERT_TYPE(UINT64, ptr), 0, \
sysctl_handle_quad, QU, __DESCR(descr))
+#define SYSCTL_X64(parent, nbr, name, access, ptr, val, descr) \
+ SYSCTL_ASSERT_TYPE(XINT64, ptr, parent, name); \
+ SYSCTL_OID(parent, nbr, name, \
+ CTLTYPE_QUAD | CTLFLAG_MPSAFE | (access), \
+ ptr, val, sysctl_handle_quad, QX, descr)
+
+#define SYSCTL_ADD_X64(ctx, parent, nbr, name, access, ptr, descr)
\
+ sysctl_add_oid(ctx, parent, nbr, name, \
+ CTLTYPE_QUAD | CTLFLAG_MPSAFE | (access), \
+ SYSCTL_ADD_ASSERT_TYPE(XINT64, ptr), 0, \
+ sysctl_handle_quad, QX, __DESCR(descr))
+
/* Oid for an opaque object. Specified by a pointer and a length. */
#define SYSCTL_OPAQUE(parent, nbr, name, access, ptr, len, fmt, descr) \
SYSCTL_OID(parent, nbr, name, CTLTYPE_OPAQUE|(access), \
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r217330 - head/sys/x86/x86
On Wed, Jan 12, 2011 at 1:21 PM, John Baldwin j...@freebsd.org wrote: On Wednesday, January 12, 2011 4:08:50 pm Matthew D Fleming wrote: Author: mdf Date: Wed Jan 12 21:08:49 2011 New Revision: 217330 URL: http://svn.freebsd.org/changeset/base/217330 Log: Fix a brain fart. Since this file is shared between i386 and amd64, a bus_size_t may be 32 or 64 bits. Change the bounce_zone alignment field to explicitly be 32 bits, as I can't really imagine a DMA device that needs anything close to 2GB alignment of data. Hmm, we do have devices with 4GB boundaries though. I think I'd prefer it if you instead if you did this: #if defined(amd64) || defined(PAE) #define SYSCTL_ADD_BUS_SIZE_T SYSCTL_ADD_UQUAD #else #define SYSCTL_ADD_BUS_SIZE_T SYSCTL_ADD_UINT #endif and then just used SYSCTL_ADD_BUS_SIZE_T() in the code so we could let the members in the bounce zone retain the same types passed to bus_dma_tag_create(). But would there be a device that can't start DMA except on a 4GB boundary? I thought that's what this member was for. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r217330 - head/sys/x86/x86
On Wed, Jan 12, 2011 at 4:06 PM, Bruce Evans b...@optusnet.com.au wrote: On Wed, 12 Jan 2011, John Baldwin wrote: Log: Fix a brain fart. Since this file is shared between i386 and amd64, a bus_size_t may be 32 or 64 bits. Change the bounce_zone alignment field to explicitly be 32 bits, as I can't really imagine a DMA device that needs anything close to 2GB alignment of data. Hmm, we do have devices with 4GB boundaries though. I think I'd prefer it if you instead if you did this: #if defined(amd64) || defined(PAE) #define SYSCTL_ADD_BUS_SIZE_T SYSCTL_ADD_UQUAD #else #define SYSCTL_ADD_BUS_SIZE_T SYSCTL_ADD_UINT #endif and then just used SYSCTL_ADD_BUS_SIZE_T() in the code so we could let the members in the bounce zone retain the same types passed to bus_dma_tag_create(). U_LONG should work on all arches. malloc(9) still uses u_long instead of size_t. This works for scalars even on the recently removed i386's with 32-bit longs where u_long is larger than size_t, since larger is a fail-safe direction. This fails for pointers. Newer parts of malloc() and uma are broken unless u_long is the same as uintptr_t, since they cast pointers to u_long. This direction is fail-safe too, but gcc warns about it. In this case for PAE u_long is (theoretically) too small, because a bus_size_t is an uint64_t. uquad_t should never be used, like unsigned long long. Similarly for signed types. Perhaps it could be removed in sysctl interfaces first. The name SYSCTL_ADD_UQUAD is a little misleading since it's really for a uint64_t. The name could be changed, but there's already plenty of existing uses of QUAD for int64_t which aren't being changed. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r216616 - in head/sys: kern sys
On Tue, Dec 21, 2010 at 8:29 AM, Matthew D Fleming m...@freebsd.org wrote:
Author: mdf
Date: Tue Dec 21 16:29:58 2010
New Revision: 216616
URL: http://svn.freebsd.org/changeset/base/216616
Log:
Move the fail_point_entry definition from fail.h to kern_fail.c, which
allows putting the enumeration constants of fail point types with the
text string that matches them.
Forgot the other part of the change. Mark the oid as MPSAFE since it
runs using an internal lock, add __BEGIN_DECLS/__END_DECLS tag around
the function declarations, and move a #define under the guard, since
using the define makes no sense if the sysctl node is not declared.
Thanks,
matthew
MFC after: 1 week
Modified:
head/sys/kern/kern_fail.c
head/sys/sys/fail.h
Modified: head/sys/kern/kern_fail.c
==
--- head/sys/kern/kern_fail.c Tue Dec 21 13:45:29 2010 (r216615)
+++ head/sys/kern/kern_fail.c Tue Dec 21 16:29:58 2010 (r216616)
@@ -76,6 +76,43 @@ MTX_SYSINIT(g_fp_mtx, g_fp_mtx, fail p
#define FP_LOCK() mtx_lock(g_fp_mtx)
#define FP_UNLOCK() mtx_unlock(g_fp_mtx)
+/**
+ * Failpoint types.
+ * Don't change these without changing fail_type_strings in fail.c.
+ * @ingroup failpoint_private
+ */
+enum fail_point_t {
+ FAIL_POINT_OFF, /** don't fail */
+ FAIL_POINT_PANIC, /** panic */
+ FAIL_POINT_RETURN, /** return an errorcode */
+ FAIL_POINT_BREAK, /** break into the debugger */
+ FAIL_POINT_PRINT, /** print a message */
+ FAIL_POINT_SLEEP, /** sleep for some msecs */
+ FAIL_POINT_INVALID, /** placeholder */
+};
+
+static const char *fail_type_strings[] = {
+ off,
+ panic,
+ return,
+ break,
+ print,
+ sleep,
+};
+
+/**
+ * Internal structure tracking a single term of a complete failpoint.
+ * @ingroup failpoint_private
+ */
+struct fail_point_entry {
+ enum fail_point_t fe_type; /** type of entry */
+ int fe_arg; /** argument to type (e.g. return
value) */
+ int fe_prob; /** likelihood of firing in
millionths */
+ int fe_count; /** number of times to fire, 0 means
always */
+
+ TAILQ_ENTRY(fail_point_entry) fe_entries; /** next entry in fail
point */
+};
+
static inline void
fail_point_sleep(struct fail_point *fp, struct fail_point_entry *ent,
int msecs, enum fail_point_return_code *pret)
@@ -102,15 +139,6 @@ enum {
PROB_DIGITS = 6, /* number of zero's in above number */
};
-static const char *fail_type_strings[] = {
- off,
- panic,
- return,
- break,
- print,
- sleep,
-};
-
static char *parse_fail_point(struct fail_point_entries *, char *);
static char *parse_term(struct fail_point_entries *, char *);
static char *parse_number(int *out_units, int *out_decimal, char *);
Modified: head/sys/sys/fail.h
==
--- head/sys/sys/fail.h Tue Dec 21 13:45:29 2010 (r216615)
+++ head/sys/sys/fail.h Tue Dec 21 16:29:58 2010 (r216616)
@@ -39,22 +39,6 @@
#include sys/queue.h
#include sys/sysctl.h
-
-/**
- * Failpoint types.
- * Don't change these without changing fail_type_strings in fail.c.
- * @ingroup failpoint_private
- */
-enum fail_point_t {
- FAIL_POINT_OFF, /** don't fail */
- FAIL_POINT_PANIC, /** panic */
- FAIL_POINT_RETURN, /** return an errorcode */
- FAIL_POINT_BREAK, /** break into the debugger */
- FAIL_POINT_PRINT, /** print a message */
- FAIL_POINT_SLEEP, /** sleep for some msecs */
- FAIL_POINT_INVALID, /** placeholder */
-};
-
/**
* Failpoint return codes, used internally.
* @ingroup failpoint_private
@@ -65,6 +49,7 @@ enum fail_point_return_code {
FAIL_POINT_RC_QUEUED, /** sleep_fn will be called */
};
+struct fail_point_entry;
TAILQ_HEAD(fail_point_entries, fail_point_entry);
/**
* Internal failpoint structure, tracking all the current details of the
@@ -84,18 +69,7 @@ struct fail_point {
#define FAIL_POINT_DYNAMIC_NAME 0x01 /** Must free name on
destroy */
-/**
- * Internal structure tracking a single term of a complete failpoint.
- * @ingroup failpoint_private
- */
-struct fail_point_entry {
- enum fail_point_t fe_type; /** type of entry */
- int fe_arg; /** argument to type (e.g. return
value) */
- int fe_prob; /** likelihood of firing in
millionths */
- int fe_count; /** number of times to fire, 0 means
always */
-
- TAILQ_ENTRY(fail_point_entry) fe_entries; /** next entry in fail
point */
-};
+__BEGIN_DECLS
Re: svn commit: r216616 - in head/sys: kern sys
On Tue, Dec 21, 2010 at 8:52 AM, Stefan Farfeleder stef...@freebsd.org wrote:
On Tue, Dec 21, 2010 at 04:29:58PM +, Matthew D Fleming wrote:
Author: mdf
Date: Tue Dec 21 16:29:58 2010
New Revision: 216616
URL: http://svn.freebsd.org/changeset/base/216616
Log:
Move the fail_point_entry definition from fail.h to kern_fail.c, which
allows putting the enumeration constants of fail point types with the
text string that matches them.
[snip]
+enum fail_point_t {
+ FAIL_POINT_OFF, /** don't fail */
+ FAIL_POINT_PANIC, /** panic */
+ FAIL_POINT_RETURN, /** return an errorcode */
+ FAIL_POINT_BREAK, /** break into the debugger */
+ FAIL_POINT_PRINT, /** print a message */
+ FAIL_POINT_SLEEP, /** sleep for some msecs */
+ FAIL_POINT_INVALID, /** placeholder */
+};
+
+static const char *fail_type_strings[] = {
+ off,
+ panic,
+ return,
+ break,
+ print,
+ sleep,
+};
FWIW, you can also do this in C99:
static const char *fail_type_strings[] = {
[FAIL_POINT_OFF] = off,
};
True. In this case I also wanted to get the stuff out of the header
that was really private.
Thanks,
matthew
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r216016 - head/sys/sparc64/include
On Tue, Dec 7, 2010 at 5:41 AM, Marius Strobl mar...@alchemy.franken.de wrote: On Mon, Dec 06, 2010 at 02:30:01PM -0800, m...@freebsd.org wrote: On Mon, Dec 6, 2010 at 2:07 PM, Marius Strobl mar...@alchemy.franken.de wrote: [lots of snip] With that one the kernel now survies memguard_init() but then panics right afterwards when kmeminit() calls kmem_suballoc(): KDB: debugger backends: ddb KDB: current backend: ddb Copyright (c) 1992-2010 The FreeBSD Project. Copyright (c) 1979, 1980, 1983, 1986, 1988, 1989, 1991, 1992, 1993, 1994 ? ? ? ?The Regents of the University of California. All rights reserved. FreeBSD is a registered trademark of The FreeBSD Foundation. FreeBSD 9.0-CURRENT #18 r215249:216120M: Mon Dec ?6 13:27:57 CET 2010 ? ?mar...@v20z.zeist.de:/home/marius/co/build/head2/sparc64.sparc64/usr/home/m4 WARNING: WITNESS option enabled, expect reduced performance. panic: kmem_suballoc: bad status return of 3 [more snip] Shooting in the dark a little... The bad status of 3 is presumably KERN_NO_SPACE because we attempted to allocate too much space from the kernel_map. What are the actual values of vm_kmem_size, kernel_map-min_offset, kernel_map-max_offset at panic time? vm_kmem_size=5610405888 min_offset=3221225472 max_offset=13958643712 This is on a US3i machine with 16GB RAM. So kernel_map is from 0xC000_ to 0x3_4000_, or 0x2_8000_ bytes large. Double the vm_kmem_size is 0x2_9CD0_, which is why this is failing. This to me says that, for the moment, you need the VM_MAX_KERNEL_ADDRESS define so that memguard does not take up too much virtual space; at the moment this hardware is somewhat constrained on virtual space. Thanks, matthew How much virtual space does sparc64 support (since earlier it was reported it's computed based on hardware capability, for this specific machine?) Currently, the limiting factor is that the kernel TSB is addressed virtually, which means that the TTEs for kernel TSB itself have to be put into locked dTLB slots taking up 1 4MB dTLB slot per 1GB. US3 family CPUs have 16 lockable 4MB dTLB and iTLB slots, which need to be shared between the TSB and the kernel itself though, i.e. a 9MB kernel also takes up 3 slots (US1/2 machines have 64 dTLB and iTLB slots but there these are also used for unlocked TTEs so we don't use more than 32 for kernel plus TSB on these). VM_MAX_KERNEL_ADDRESS is limited according to how many slots are available for the TSB, that's what I was referring to previously. Actually, US3 and later as well as SPARC64 V and later CPUs have a feature allowing the TSB to be addressed physically, circumventing the need to lock the TSB into the TLB, thus allowing the full 64-bit virtual address space to be used. Implementing that is on my TODO list but unfortunately that's not exactly straight forward and also requires some instructions to be patched at runtime so the kernel still works on US1/2 machines. Marius ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r216016 - head/sys/sparc64/include
On Mon, Dec 6, 2010 at 2:07 PM, Marius Strobl mar...@alchemy.franken.de wrote: [lots of snip] With that one the kernel now survies memguard_init() but then panics right afterwards when kmeminit() calls kmem_suballoc(): KDB: debugger backends: ddb KDB: current backend: ddb Copyright (c) 1992-2010 The FreeBSD Project. Copyright (c) 1979, 1980, 1983, 1986, 1988, 1989, 1991, 1992, 1993, 1994 The Regents of the University of California. All rights reserved. FreeBSD is a registered trademark of The FreeBSD Foundation. FreeBSD 9.0-CURRENT #18 r215249:216120M: Mon Dec 6 13:27:57 CET 2010 mar...@v20z.zeist.de:/home/marius/co/build/head2/sparc64.sparc64/usr/home/m4 WARNING: WITNESS option enabled, expect reduced performance. panic: kmem_suballoc: bad status return of 3 [more snip] Shooting in the dark a little... The bad status of 3 is presumably KERN_NO_SPACE because we attempted to allocate too much space from the kernel_map. What are the actual values of vm_kmem_size, kernel_map-min_offset, kernel_map-max_offset at panic time? How much virtual space does sparc64 support (since earlier it was reported it's computed based on hardware capability, for this specific machine?) Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r216161 - in head/sys: amd64/amd64 i386/i386
On Fri, Dec 3, 2010 at 1:54 PM, Jung-uk Kim j...@freebsd.org wrote: Author: jkim Date: Fri Dec 3 21:54:10 2010 New Revision: 216161 URL: http://svn.freebsd.org/changeset/base/216161 Log: Explicitly initialize TSC frequency. To calibrate TSC frequency, we use DELAY(9) and it may use TSC in turn if TSC frequency is non-zero. Doesn't ELF guarantee that the static data is already 0? On AIX the kernel had to explicitly zero the non-initialized data area, but AIX uses the a.out format. I thought FreeBSD/ELF meant that global variables without initializers were 0 by the time the OS started running. Thanks, matthew MFC after: 3 days Modified: head/sys/amd64/amd64/tsc.c head/sys/i386/i386/tsc.c Modified: head/sys/amd64/amd64/tsc.c == --- head/sys/amd64/amd64/tsc.c Fri Dec 3 21:52:01 2010 (r216160) +++ head/sys/amd64/amd64/tsc.c Fri Dec 3 21:54:10 2010 (r216161) @@ -46,7 +46,7 @@ __FBSDID($FreeBSD$); #include cpufreq_if.h -uint64_t tsc_freq; +uint64_t tsc_freq = 0; int tsc_is_broken; int tsc_is_invariant; static eventhandler_tag tsc_levels_tag, tsc_pre_tag, tsc_post_tag; Modified: head/sys/i386/i386/tsc.c == --- head/sys/i386/i386/tsc.c Fri Dec 3 21:52:01 2010 (r216160) +++ head/sys/i386/i386/tsc.c Fri Dec 3 21:54:10 2010 (r216161) @@ -46,7 +46,7 @@ __FBSDID($FreeBSD$); #include cpufreq_if.h -uint64_t tsc_freq; +uint64_t tsc_freq = 0; int tsc_is_broken; int tsc_is_invariant; u_int tsc_present; ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r213322 - head/sys/kern
On Fri, Oct 1, 2010 at 9:34 AM, Andriy Gapon a...@freebsd.org wrote: Author: avg Date: Fri Oct 1 09:34:41 2010 New Revision: 213322 URL: http://svn.freebsd.org/changeset/base/213322 Log: sysctls in kern_shutdown: add twin tunables also make couple of sysctl-controlled variables static Reviewed by: rwatson MFC after: 1 week Modified: head/sys/kern/kern_shutdown.c Modified: head/sys/kern/kern_shutdown.c == --- head/sys/kern/kern_shutdown.c Fri Oct 1 09:18:30 2010 (r213321) +++ head/sys/kern/kern_shutdown.c Fri Oct 1 09:34:41 2010 (r213322) @@ -98,21 +98,24 @@ int debugger_on_panic = 0; #else int debugger_on_panic = 1; #endif -SYSCTL_INT(_debug, OID_AUTO, debugger_on_panic, CTLFLAG_RW, +SYSCTL_INT(_debug, OID_AUTO, debugger_on_panic, CTLFLAG_RW | CTLFLAG_TUN, debugger_on_panic, 0, Run debugger on kernel panic); I thought CTLFLAG_TUN was only used to provide a more useful error message when writing to a read-only sysctl? I think the CTLFLAG_TUN should not be here for a RW sysctl. Thanks, matthew +TUNABLE_INT(debug.debugger_on_panic, debugger_on_panic); #ifdef KDB_TRACE -int trace_on_panic = 1; +static int trace_on_panic = 1; #else -int trace_on_panic = 0; +static int trace_on_panic = 0; #endif -SYSCTL_INT(_debug, OID_AUTO, trace_on_panic, CTLFLAG_RW, +SYSCTL_INT(_debug, OID_AUTO, trace_on_panic, CTLFLAG_RW | CTLFLAG_TUN, trace_on_panic, 0, Print stack trace on kernel panic); +TUNABLE_INT(debug.trace_on_panic, trace_on_panic); #endif /* KDB */ -int sync_on_panic = 0; -SYSCTL_INT(_kern, OID_AUTO, sync_on_panic, CTLFLAG_RW, +static int sync_on_panic = 0; +SYSCTL_INT(_kern, OID_AUTO, sync_on_panic, CTLFLAG_RW | CTLFLAG_TUN, sync_on_panic, 0, Do a sync before rebooting from a panic); +TUNABLE_INT(kern.sync_on_panic, sync_on_panic); SYSCTL_NODE(_kern, OID_AUTO, shutdown, CTLFLAG_RW, 0, Shutdown environment); ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r213305 - in head/sys: gdb kern sys
On Thu, Sep 30, 2010 at 10:05 AM, Andriy Gapon a...@freebsd.org wrote:
Author: avg
Date: Thu Sep 30 17:05:23 2010
New Revision: 213305
URL: http://svn.freebsd.org/changeset/base/213305
Log:
there must be only one SYSINIT with SI_SUB_RUN_SCHEDULER+SI_ORDER_ANY order
SI_SUB_RUN_SCHEDULER+SI_ORDER_ANY should only be used to call
scheduler() function which turns the initial thread into swapper proper
and thus there is no further SYSINIT processing.
Does this imply that scheduler() shouldn't be called from a sysinit at
all, and instead a hand-call after processing all the boot-time
sysinit's would make more sense? This prevents the bug from
reoccuring, and also prevents bugs with adding a SYSINIT that runs at
SI_SUB_RUN_SCHEDULER + 1 time.
Thanks,
matthew
Other SYSINITs with SI_SUB_RUN_SCHEDULER+SI_ORDER_ANY may get ordered
after scheduler() and thus never executed. That particular relative
order is semi-arbitrary.
Thus, change such places to use SI_ORDER_MIDDLE.
Also, use SI_ORDER_MIDDLE instead of correct, but less appealing,
SI_ORDER_ANY - 1.
MFC after: 1 week
Modified:
head/sys/gdb/gdb_cons.c
head/sys/kern/kern_ntptime.c
head/sys/sys/sched.h
Modified: head/sys/gdb/gdb_cons.c
==
--- head/sys/gdb/gdb_cons.c Thu Sep 30 16:47:01 2010 (r213304)
+++ head/sys/gdb/gdb_cons.c Thu Sep 30 17:05:23 2010 (r213305)
@@ -126,7 +126,7 @@ oktousecallout(void *data __unused)
{
calloutok = 1;
}
-SYSINIT(gdbhack, SI_SUB_RUN_SCHEDULER, SI_ORDER_ANY, oktousecallout, NULL);
+SYSINIT(gdbhack, SI_SUB_RUN_SCHEDULER, SI_ORDER_MIDDLE, oktousecallout,
NULL);
static void
gdb_cnputc(struct consdev *cp, int c)
Modified: head/sys/kern/kern_ntptime.c
==
--- head/sys/kern/kern_ntptime.c Thu Sep 30 16:47:01 2010
(r213304)
+++ head/sys/kern/kern_ntptime.c Thu Sep 30 17:05:23 2010
(r213305)
@@ -1035,5 +1035,5 @@ start_periodic_resettodr(void *arg __unu
periodic_resettodr, NULL);
}
-SYSINIT(periodic_resettodr, SI_SUB_RUN_SCHEDULER, SI_ORDER_ANY - 1,
+SYSINIT(periodic_resettodr, SI_SUB_RUN_SCHEDULER, SI_ORDER_MIDDLE,
start_periodic_resettodr, NULL);
Modified: head/sys/sys/sched.h
==
--- head/sys/sys/sched.h Thu Sep 30 16:47:01 2010 (r213304)
+++ head/sys/sys/sched.h Thu Sep 30 17:05:23 2010 (r213305)
@@ -173,7 +173,7 @@ static void name ## _add_proc(void *dumm
#name, CTLTYPE_LONG|CTLFLAG_RD|CTLFLAG_MPSAFE, \
ptr, 0, sysctl_dpcpu_long, LU, descr); \
} \
-SYSINIT(name, SI_SUB_RUN_SCHEDULER, SI_ORDER_ANY, name ## _add_proc, NULL);
+SYSINIT(name, SI_SUB_RUN_SCHEDULER, SI_ORDER_MIDDLE, name ## _add_proc,
NULL);
#define SCHED_STAT_DEFINE(name, descr)
\
DPCPU_DEFINE(unsigned long, name); \
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r212964 - head/sys/kern
On Tue, Sep 21, 2010 at 8:07 AM, Andriy Gapon a...@freebsd.org wrote:
Author: avg
Date: Tue Sep 21 15:07:44 2010
New Revision: 212964
URL: http://svn.freebsd.org/changeset/base/212964
Log:
kdb_backtrace: stack(9)-based code to print backtrace without any backend
The idea is to add KDB and KDB_TRACE options to GENERIC kernels on
stable branches, so that at least the minimal information is produced
for non-specific panics like traps on page faults.
The GENERICs in stable branches seem to already include STACK option.
Reviewed by: attilio
MFC after: 2 weeks
Modified:
head/sys/kern/subr_kdb.c
Modified: head/sys/kern/subr_kdb.c
==
--- head/sys/kern/subr_kdb.c Tue Sep 21 12:57:43 2010 (r212963)
+++ head/sys/kern/subr_kdb.c Tue Sep 21 15:07:44 2010 (r212964)
@@ -28,6 +28,7 @@
__FBSDID($FreeBSD$);
#include opt_kdb.h
+#include opt_stack.h
#include sys/param.h
#include sys/systm.h
@@ -37,6 +38,7 @@ __FBSDID($FreeBSD$);
#include sys/pcpu.h
#include sys/proc.h
#include sys/smp.h
+#include sys/stack.h
#include sys/sysctl.h
#include machine/kdb.h
@@ -300,6 +302,15 @@ kdb_backtrace(void)
printf(KDB: stack backtrace:\n);
kdb_dbbe-dbbe_trace();
}
+#ifdef STACK
+ else {
+ struct stack st;
+
+ printf(KDB: stack backtrace:\n);
+ stack_save(st);
+ stack_print(st);
I'd recommend using stack_print_ddb(), as that avoids any locking
which may hang depending on how the kernel panic'd.
Thanks,
matthew
+ }
+#endif
}
/*
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r212964 - head/sys/kern
On Tue, Sep 21, 2010 at 8:40 AM, Andriy Gapon a...@freebsd.org wrote: on 21/09/2010 18:27 Andriy Gapon said the following: on 21/09/2010 18:17 m...@freebsd.org said the following: I'd recommend using stack_print_ddb(), as that avoids any locking which may hang depending on how the kernel panic'd. It seems that stack_print_ddb() depends on DDB? But the point about locking is very good. How do you suggest we can deal with it? A dirty hack would be to check panicstr in linker_search_symbol_name and avoid locking, but I don't like that at all. Perhaps, some code in subr_stack.c could be taken outside DDB ifdef? I keep forgetting, but actually _mtx_lock_sleep() will just return if panicstr is set. _mtx_assert() is similarly guarded, so actually it should be mostly okay. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r212964 - head/sys/kern
On Tue, Sep 21, 2010 at 9:50 AM, John Baldwin j...@freebsd.org wrote: On Tuesday, September 21, 2010 12:31:01 pm m...@freebsd.org wrote: On Tue, Sep 21, 2010 at 8:40 AM, Andriy Gapon a...@freebsd.org wrote: on 21/09/2010 18:27 Andriy Gapon said the following: on 21/09/2010 18:17 m...@freebsd.org said the following: I'd recommend using stack_print_ddb(), as that avoids any locking which may hang depending on how the kernel panic'd. It seems that stack_print_ddb() depends on DDB? But the point about locking is very good. How do you suggest we can deal with it? A dirty hack would be to check panicstr in linker_search_symbol_name and avoid locking, but I don't like that at all. Perhaps, some code in subr_stack.c could be taken outside DDB ifdef? I keep forgetting, but actually _mtx_lock_sleep() will just return if panicstr is set. _mtx_assert() is similarly guarded, so actually it should be mostly okay. Err, I don't think _mtx_lock_sleep() is guarded in that fashion? I have an old patch to do that but have never committed it. If we want that we should probably change rwlocks and sxlocks to have also not block when panicstr is set. D'oh! Once again I was looking at Isilon source but not CURRENT. We had patches for mtx (back on 6.1) and haven't updated them for sx/rw for 7. We're also running with local patches to stop CPUs in panic() that Mr Jacob has a copy of. Regarding the original issue, then, I think in the short term using a safe stack_print() would be the correct thing. Changing the locking and stop_cpus logic will not happen in the next week. :-) Thanks, matthew --- //depot/vendor/freebsd/src/sys/kern/kern_mutex.c 2010-05-11 18:31:25.0 +++ //depot/projects/smpng/sys/kern/kern_mutex.c 2010-06-01 20:12:02.0 @@ -348,6 +348,15 @@ return; } + /* + * If we have already panic'd and this is the thread that called + * panic(), then don't block on any mutexes but silently succeed. + * Otherwise, the kernel will deadlock since the scheduler isn't + * going to run the thread that holds the lock we need. + */ + if (panicstr != NULL curthread-td_flags TDF_INPANIC) + return; + lock_profile_obtain_lock_failed(m-lock_object, contested, waittime); if (LOCK_LOG_TEST(m-lock_object, opts)) @@ -664,6 +673,15 @@ } /* + * If we failed to unlock this lock and we are a thread that has + * called panic(), it may be due to the bypass in _mtx_lock_sleep() + * above. In that case, just return and leave the lock alone to + * avoid changing the state. + */ + if (panicstr != NULL curthread-td_flags TDF_INPANIC) + return; + + /* * We have to lock the chain before the turnstile so this turnstile * can be removed from the hash list if it is empty. */ -- John Baldwin ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r212478 - head/sys/kern
On Sat, Sep 11, 2010 at 7:42 PM, Alexander Kabaev k...@freebsd.org wrote:
Author: kan
Date: Sat Sep 11 19:42:50 2010
New Revision: 212478
URL: http://svn.freebsd.org/changeset/base/212478
Log:
Add missing pointer increment to sbuf_cat.
Modified:
head/sys/kern/subr_sbuf.c
Modified: head/sys/kern/subr_sbuf.c
==
--- head/sys/kern/subr_sbuf.c Sat Sep 11 18:55:00 2010 (r212477)
+++ head/sys/kern/subr_sbuf.c Sat Sep 11 19:42:50 2010 (r212478)
@@ -441,7 +441,7 @@ sbuf_cat(struct sbuf *s, const char *str
return (-1);
while (*str != '\0') {
- sbuf_put_byte(*str, s);
+ sbuf_put_byte(*str++, s);
if (s-s_error != 0)
return (-1);
}
D'oh! Yeah, this looks right. I guess the sbuf functions that ran
when I tested didn't use sbuf_cat.
Sorry,
matthew
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r212439 - head/sys/fs/nfs
On Sun, Sep 12, 2010 at 12:41 AM, Rick Macklem rmack...@uoguelph.ca wrote:
Then, fid_reserved is no more reserved ? Should we rename it ?
Comment for fid_reserved about longword alignment is wrong.
Well, it's actually more broken than that.
fid_len - Most file systems set it to the size of their variant
of the entire structure, including the Xfid_len field.
ZFS sets it to the size of the structure - sizeof(uint16_t)
{ presumably subtracting out the size if Xfid_len? }.
And xfs, well, it does weird stuff with it I can't figure
out, but it is definitely not the size of the entire struct.
As such, exposing fid_len above the VOP_xxx() doesn't make much sense.
(After my commit yesterday, nothing above the VOP_VPTOFH() uses it.)
Personally, I'd lean towards a generic struct fid like...
struct fid {
uint8_t fid_data[MAXFIDSZ];
};
Isilon would love a generic struct like this, as to fit our filesystem
we had to make such a change locally anyways. :-)
Cheers,
matthew
with MAXFIDSZ increased appropriately, but this will require changes
to xfs and zfs, since they both set the generic fid_len.
If you go with...
struct fid {
uint16_t fid_len;
uint8_t fid_data[MAXFIDSZ];
};
then the hash functions in the two NFS servers need to be changed
(they assume 32bit alignment of fid_data), but they should be fixed
anyhow, since they mostly hash to 0 for ZFS at this time. (From what
I see ZFS file handles looking like.)
Or, you could just rename fid_reserved to fid_pad and not worry about it.
Maybe the ZFS folks could decide what they would prefer? rick
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r212368 - head/sys/dev/pci
On Thu, Sep 9, 2010 at 11:19 AM, John Baldwin j...@freebsd.org wrote: Author: jhb Date: Thu Sep 9 18:19:15 2010 New Revision: 212368 URL: http://svn.freebsd.org/changeset/base/212368 Log: - Rename the constant for the Master Data Parity Error flag in the PCI status register to map its current name. - Use PCIM_* rather than PCIR_* for constants for fields in various AER registers. I got about half of them right in the previous commit. MFC after: 1 week Modified: head/sys/dev/pci/pcireg.h This seems to break building CURRENT with this error: /data/sb/bsd.git/sys/dev/msk/if_msk.c: In function 'mskc_reset': /data/sb/bsd.git/sys/dev/msk/if_msk.c:1337: error: 'PCIM_STATUS_PERRREPORT' undeclared (first use in this function) /data/sb/bsd.git/sys/dev/msk/if_msk.c:1337: error: (Each undeclared identifier is reported only once /data/sb/bsd.git/sys/dev/msk/if_msk.c:1337: error: for each function it appears in.) /data/sb/bsd.git/sys/dev/msk/if_msk.c: In function 'msk_intr_hwerr': /data/sb/bsd.git/sys/dev/msk/if_msk.c:3408: error: 'PCIM_STATUS_PERRREPORT' undeclared (first use in this function) Thanks, matthew Modified: head/sys/dev/pci/pcireg.h == --- head/sys/dev/pci/pcireg.h Thu Sep 9 17:49:18 2010 (r212367) +++ head/sys/dev/pci/pcireg.h Thu Sep 9 18:19:15 2010 (r212368) @@ -67,7 +67,7 @@ #define PCIM_STATUS_CAPPRESENT 0x0010 #define PCIM_STATUS_66CAPABLE 0x0020 #define PCIM_STATUS_BACKTOBACK 0x0080 -#define PCIM_STATUS_PERRREPORT 0x0100 +#define PCIM_STATUS_MDPERR 0x0100 #define PCIM_STATUS_SEL_FAST 0x #define PCIM_STATUS_SEL_MEDIMUM 0x0200 #define PCIM_STATUS_SEL_SLOW 0x0400 @@ -689,18 +689,18 @@ /* Advanced Error Reporting */ #define PCIR_AER_UC_STATUS 0x04 -#define PCIR_AER_UC_TRAINING_ERROR 0x0001 -#define PCIR_AER_UC_DL_PROTOCOL_ERROR 0x0010 -#define PCIR_AER_UC_POISONED_TLP 0x1000 -#define PCIR_AER_UC_FC_PROTOCOL_ERROR 0x2000 -#define PCIR_AER_UC_COMPLETION_TIMEOUT 0x4000 -#define PCIR_AER_UC_COMPLETER_ABORT 0x8000 -#define PCIR_AER_UC_UNEXPECTED_COMPLETION 0x0001 -#define PCIR_AER_UC_RECEIVER_OVERFLOW 0x0002 -#define PCIR_AER_UC_MALFORMED_TLP 0x0004 -#define PCIR_AER_UC_ECRC_ERROR 0x0008 -#define PCIR_AER_UC_UNSUPPORTED_REQUEST 0x0010 -#define PCIR_AER_UC_ACS_VIOLATION 0x0020 +#define PCIM_AER_UC_TRAINING_ERROR 0x0001 +#define PCIM_AER_UC_DL_PROTOCOL_ERROR 0x0010 +#define PCIM_AER_UC_POISONED_TLP 0x1000 +#define PCIM_AER_UC_FC_PROTOCOL_ERROR 0x2000 +#define PCIM_AER_UC_COMPLETION_TIMEOUT 0x4000 +#define PCIM_AER_UC_COMPLETER_ABORT 0x8000 +#define PCIM_AER_UC_UNEXPECTED_COMPLETION 0x0001 +#define PCIM_AER_UC_RECEIVER_OVERFLOW 0x0002 +#define PCIM_AER_UC_MALFORMED_TLP 0x0004 +#define PCIM_AER_UC_ECRC_ERROR 0x0008 +#define PCIM_AER_UC_UNSUPPORTED_REQUEST 0x0010 +#define PCIM_AER_UC_ACS_VIOLATION 0x0020 #define PCIR_AER_UC_MASK 0x08 /* Shares bits with UC_STATUS */ #define PCIR_AER_UC_SEVERITY 0x0c /* Shares bits with UC_STATUS */ #define PCIR_AER_COR_STATUS 0x10 @@ -718,18 +718,18 @@ #define PCIM_AER_ECRC_CHECK_ENABLE 0x0100 #define PCIR_AER_HEADER_LOG 0x1c #define PCIR_AER_ROOTERR_CMD 0x2c /* Only for root complex ports */ -#define PCIR_AER_ROOTERR_COR_ENABLE 0x0001 -#define PCIR_AER_ROOTERR_NF_ENABLE 0x0002 -#define PCIR_AER_ROOTERR_F_ENABLE 0x0004 +#define PCIM_AER_ROOTERR_COR_ENABLE 0x0001 +#define PCIM_AER_ROOTERR_NF_ENABLE 0x0002 +#define PCIM_AER_ROOTERR_F_ENABLE 0x0004 #define PCIR_AER_ROOTERR_STATUS 0x30 /* Only for root complex ports */ -#define PCIR_AER_ROOTERR_COR_ERR 0x0001 -#define PCIR_AER_ROOTERR_MULTI_COR_ERR 0x0002 -#define PCIR_AER_ROOTERR_UC_ERR 0x0004 -#define PCIR_AER_ROOTERR_MULTI_UC_ERR 0x0008 -#define PCIR_AER_ROOTERR_FIRST_UC_FATAL 0x0010 -#define PCIR_AER_ROOTERR_NF_ERR 0x0020 -#define PCIR_AER_ROOTERR_F_ERR 0x0040 -#define PCIR_AER_ROOTERR_INT_MESSAGE 0xf800 +#define PCIM_AER_ROOTERR_COR_ERR 0x0001 +#define PCIM_AER_ROOTERR_MULTI_COR_ERR 0x0002 +#define PCIM_AER_ROOTERR_UC_ERR 0x0004 +#define PCIM_AER_ROOTERR_MULTI_UC_ERR 0x0008 +#define
Re: svn commit: r212182 - head/sys/kern
On Fri, Sep 3, 2010 at 10:23 AM, Matthew D Fleming m...@freebsd.org wrote: Author: mdf Date: Fri Sep 3 17:23:26 2010 New Revision: 212182 URL: http://svn.freebsd.org/changeset/base/212182 Log: Fix user-space libsbuf build. Why isn't CTASSERT available to user-space? Sorry for the churn. I am having a un-careful morning, it appears. Thanks, matthew Modified: head/sys/kern/subr_sbuf.c Modified: head/sys/kern/subr_sbuf.c == --- head/sys/kern/subr_sbuf.c Fri Sep 3 16:12:39 2010 (r212181) +++ head/sys/kern/subr_sbuf.c Fri Sep 3 17:23:26 2010 (r212182) @@ -116,8 +116,10 @@ _assert_sbuf_state(const char *fun, stru #endif /* _KERNEL INVARIANTS */ +#ifdef _KERNEL CTASSERT(powerof2(SBUF_MAXEXTENDSIZE)); CTASSERT(powerof2(SBUF_MAXEXTENDINCR)); +#endif static int sbuf_extendsize(int size) ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r212153 - head/sys/kern
On Thu, Sep 2, 2010 at 9:23 AM, Matthew D Fleming m...@freebsd.org wrote:
Author: mdf
Date: Thu Sep 2 16:23:05 2010
New Revision: 212153
URL: http://svn.freebsd.org/changeset/base/212153
Log:
Fix UP build.
Noticed by b.f. (bf1783 at gmail dot com)
We apologize for the inconvenience.
matthew
MFC after: 2 weeks
Modified:
head/sys/kern/sched_ule.c
Modified: head/sys/kern/sched_ule.c
==
--- head/sys/kern/sched_ule.c Thu Sep 2 16:11:12 2010 (r212152)
+++ head/sys/kern/sched_ule.c Thu Sep 2 16:23:05 2010 (r212153)
@@ -1797,8 +1797,10 @@ sched_switch(struct thread *td, struct t
srqflag = (flags SW_PREEMPT) ?
SRQ_OURSELF|SRQ_YIELDING|SRQ_PREEMPTED :
SRQ_OURSELF|SRQ_YIELDING;
+#ifdef SMP
if (THREAD_CAN_MIGRATE(td) !THREAD_CAN_SCHED(td,
ts-ts_cpu))
ts-ts_cpu = sched_pickcpu(td, 0);
+#endif
if (ts-ts_cpu == cpuid)
tdq_runq_add(tdq, td, srqflag);
else {
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r212115 - head/sys/kern
On Wed, Sep 1, 2010 at 2:31 PM, John Baldwin j...@freebsd.org wrote: On Wednesday, September 01, 2010 4:32:48 pm Matthew D Fleming wrote: Author: mdf Date: Wed Sep 1 20:32:47 2010 New Revision: 212115 URL: http://svn.freebsd.org/changeset/base/212115 Log: Fix a bug with sched_affinity() where it checks td_pinned of another thread in a racy manner, which can lead to attempting to migrate a thread that is pinned to a CPU. Instead, have sched_switch() determine which CPU a thread should run on if the current one is not allowed. KASSERT in sched_bind() that the thread is not yet pinned to a CPU. KASSERT in sched_switch() that only migratable threads or those moving due to a sched_bind() are changing CPUs. sched_affinity code came from j...@. MFC after: 2 weeks Cool, I guess this fixed it in your tests then? Yes, the stress case I pointed out in the earlier thread ran for quite a few minutes before I killed it. Previously it would crash (with the added assert) in a few seconds. Thanks, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r211463 - head/usr.bin/grep
On Wed, Aug 18, 2010 at 10:40 AM, Gabor Kovesdan ga...@freebsd.org wrote:
Author: gabor
Date: Wed Aug 18 17:40:10 2010
New Revision: 211463
URL: http://svn.freebsd.org/changeset/base/211463
Log:
- Refactor file reading code to use pure syscalls and an internal buffer
instead of stdio. This gives BSD grep a very big performance boost,
its speed is now almost comparable to GNU grep.
I didn't read all of the details in the profiling mails in the thread,
but does this mean that work on stdio would give a performance boost
to many apps? Or is there something specific about how grep(1) is
using its input that makes it a horse of a different color?
Thanks,
matthew
Submitted by: Dimitry Andric dimi...@andric.com
Approved by: delphij (mentor)
Modified:
head/usr.bin/grep/fastgrep.c
head/usr.bin/grep/file.c
head/usr.bin/grep/grep.h
head/usr.bin/grep/util.c
Modified: head/usr.bin/grep/fastgrep.c
==
--- head/usr.bin/grep/fastgrep.c Wed Aug 18 17:39:47 2010
(r211462)
+++ head/usr.bin/grep/fastgrep.c Wed Aug 18 17:40:10 2010
(r211463)
@@ -198,7 +198,7 @@ fastcomp(fastgrep_t *fg, const char *pat
}
int
-grep_search(fastgrep_t *fg, unsigned char *data, size_t len, regmatch_t
*pmatch)
+grep_search(fastgrep_t *fg, const unsigned char *data, size_t len,
regmatch_t *pmatch)
{
unsigned int j;
int ret = REG_NOMATCH;
Modified: head/usr.bin/grep/file.c
==
--- head/usr.bin/grep/file.c Wed Aug 18 17:39:47 2010 (r211462)
+++ head/usr.bin/grep/file.c Wed Aug 18 17:40:10 2010 (r211463)
@@ -2,7 +2,8 @@
/*-
* Copyright (c) 1999 James Howard and Dag-Erling Coïdan Smørgrav
- * Copyright (C) 2008-2009 Gabor Kovesdan ga...@freebsd.org
+ * Copyright (C) 2008-2010 Gabor Kovesdan ga...@freebsd.org
+ * Copyright (C) 2010 Dimitry Andric dimi...@andric.com
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -37,7 +38,8 @@ __FBSDID($FreeBSD$);
#include bzlib.h
#include err.h
#include errno.h
-#include stdio.h
+#include fcntl.h
+#include stddef.h
#include stdlib.h
#include string.h
#include unistd.h
@@ -47,222 +49,204 @@ __FBSDID($FreeBSD$);
#include grep.h
-static char fname[MAXPATHLEN]; /* file name */
+#define MAXBUFSIZ (32 * 1024)
+#define LNBUFBUMP 80
-#define MAXBUFSIZ (16 * 1024)
-#define PREREAD_M 0.2
+static gzFile gzbufdesc;
+static BZFILE* bzbufdesc;
-/* Some global variables for the buffering and reading. */
-static char *lnbuf;
-static size_t lnbuflen;
-static unsigned char *binbuf;
-static int binbufsiz;
-unsigned char *binbufptr;
-static int bzerr;
+static unsigned char buffer[MAXBUFSIZ];
+static unsigned char *bufpos;
+static size_t bufrem;
-#define iswbinary(ch) (!iswspace((ch)) iswcntrl((ch)) \
- (ch != L'\b') (ch != L'\0'))
+static unsigned char *lnbuf;
+static size_t lnbuflen;
-/*
- * Returns a single character according to the file type.
- * Returns -1 on failure.
- */
static inline int
-grep_fgetc(struct file *f)
+grep_refill(struct file *f)
{
- unsigned char c;
+ ssize_t nr;
+ int bzerr;
- switch (filebehave) {
- case FILE_STDIO:
- return (getc_unlocked(f-f));
- case FILE_GZIP:
- return (gzgetc(f-gzf));
- case FILE_BZIP:
- BZ2_bzRead(bzerr, f-bzf, c, 1);
- if (bzerr == BZ_STREAM_END)
- return (-1);
- else if (bzerr != BZ_SEQUENCE_ERROR bzerr != BZ_OK)
- errx(2, %s, getstr(2));
- return (c);
- }
- return (-1);
+ bufpos = buffer;
+ bufrem = 0;
+
+ if (filebehave == FILE_GZIP)
+ nr = gzread(gzbufdesc, buffer, MAXBUFSIZ);
+ else if (filebehave == FILE_BZIP bzbufdesc != NULL) {
+ nr = BZ2_bzRead(bzerr, bzbufdesc, buffer, MAXBUFSIZ);
+ switch (bzerr) {
+ case BZ_OK:
+ case BZ_STREAM_END:
+ /* No problem, nr will be okay */
+ break;
+ case BZ_DATA_ERROR_MAGIC:
+ /*
+ * As opposed to gzread(), which simply returns the
+ * plain file data, if it is not in the correct
+ * compressed format, BZ2_bzRead() instead aborts.
+ *
+ * So, just restart at the beginning of the file
again,
+ * and use plain reads from now on.
+ */
+ BZ2_bzReadClose(bzerr,
Re: svn commit: r211304 - head/lib/libutil
On Sat, Aug 14, 2010 at 2:34 PM, Dag-Erling Smorgrav d...@freebsd.org wrote:
Author: des
Date: Sat Aug 14 14:34:36 2010
New Revision: 211304
URL: http://svn.freebsd.org/changeset/base/211304
Log:
Simplify expand_number() by combining the (unrolled) loop with the
switch. Since expand_number() does not accept negative numbers, switch
from int64_t to uint64_t; this makes it easier to check for overflow.
MFC after: 3 weeks
Modified:
head/lib/libutil/expand_number.c
head/lib/libutil/libutil.h
Modified: head/lib/libutil/expand_number.c
==
--- head/lib/libutil/expand_number.c Sat Aug 14 14:18:02 2010
(r211303)
+++ head/lib/libutil/expand_number.c Sat Aug 14 14:34:36 2010
(r211304)
@@ -37,7 +37,7 @@ __FBSDID($FreeBSD$);
/*
* Convert an expression of the following forms to a int64_t.
- * 1) A positive decimal number.
+ * 1) A positive decimal number.
* 2) A positive decimal number followed by a 'b' or 'B' (mult by 1).
* 3) A positive decimal number followed by a 'k' or 'K' (mult by 1
10).
* 4) A positive decimal number followed by a 'm' or 'M' (mult by 1
20).
@@ -47,14 +47,12 @@ __FBSDID($FreeBSD$);
* 8) A positive decimal number followed by a 'e' or 'E' (mult by 1
60).
*/
int
-expand_number(const char *buf, int64_t *num)
+expand_number(const char *buf, uint64_t *num)
{
- static const char unit[] = bkmgtpe;
- char *endptr, s;
- int64_t number;
- int i;
+ uint64_t number;
+ char *endptr;
- number = strtoimax(buf, endptr, 0);
+ number = strtoumax(buf, endptr, 0);
if (endptr == buf) {
/* No valid digits. */
@@ -68,15 +66,23 @@ expand_number(const char *buf, int64_t *
return (0);
}
- s = tolower(*endptr);
- switch (s) {
- case 'b':
- case 'k':
- case 'm':
- case 'g':
- case 't':
- case 'p':
+#define SHIFT(n, b) \
+ do { if ((n b) n) goto overflow; n = b; } while (0)
I think it's possible for the number to overflow but also not shrink.
e.g. 0x12345678T.
Perhaps if (((n b) b) != n) would be better.
Thanks,
matthew
+
+ switch (tolower((unsigned char)*endptr)) {
case 'e':
+ SHIFT(number, 10);
+ case 'p':
+ SHIFT(number, 10);
+ case 't':
+ SHIFT(number, 10);
+ case 'g':
+ SHIFT(number, 10);
+ case 'm':
+ SHIFT(number, 10);
+ case 'k':
+ SHIFT(number, 10);
+ case 'b':
break;
default:
/* Unrecognized unit. */
@@ -84,17 +90,11 @@ expand_number(const char *buf, int64_t *
return (-1);
}
- for (i = 0; unit[i] != '\0'; i++) {
- if (s == unit[i])
- break;
- if ((number 0 (number 10) number) ||
- (number = 0 (number 10) number)) {
- errno = ERANGE;
- return (-1);
- }
- number = 10;
- }
-
*num = number;
return (0);
+
+overflow:
+ /* Overflow */
+ errno = ERANGE;
+ return (-1);
}
Modified: head/lib/libutil/libutil.h
==
--- head/lib/libutil/libutil.h Sat Aug 14 14:18:02 2010 (r211303)
+++ head/lib/libutil/libutil.h Sat Aug 14 14:34:36 2010 (r211304)
@@ -109,7 +109,7 @@ int forkpty(int *_amaster, char *_name,
struct termios *_termp, struct winsize *_winp);
int humanize_number(char *_buf, size_t _len, int64_t _number,
const char *_suffix, int _scale, int _flags);
-int expand_number(const char *_buf, int64_t *_num);
+int expand_number(const char *_buf, uint64_t *_num);
const char *uu_lockerr(int _uu_lockresult);
int uu_lock(const char *_ttyname);
int uu_unlock(const char *_ttyname);
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r209578 - head/sys/sys
2010/6/29 Kostik Belousov kostik...@gmail.com:
On Mon, Jun 28, 2010 at 02:07:02PM -0700, Matthew Fleming wrote:
On Mon, Jun 28, 2010 at 10:59 AM, Konstantin Belousov k...@freebsd.org
wrote:
Author: kib
Date: Mon Jun 28 17:59:45 2010
New Revision: 209578
URL: http://svn.freebsd.org/changeset/base/209578
Log:
Use C99 initializers for the struct sysent generated by MAKE_SYSENT().
MFC after: 1 week
Modified:
head/sys/sys/sysent.h
Modified: head/sys/sys/sysent.h
==
--- head/sys/sys/sysent.h Mon Jun 28 17:45:00 2010 (r209577)
+++ head/sys/sys/sysent.h Mon Jun 28 17:59:45 2010 (r209578)
@@ -144,10 +144,10 @@ struct syscall_module_data {
#define MAKE_SYSENT(syscallname) \
static struct sysent syscallname##_sysent = { \
- (sizeof(struct syscallname ## _args ) \
+ .sy_narg = (sizeof(struct syscallname ## _args ) \
/ sizeof(register_t)), \
- (sy_call_t *) syscallname, \
- SYS_AUE_##syscallname \
+ .sy_call = (sy_call_t *) syscallname, \
+ .sy_auevent = SYS_AUE_##syscallname, \
}
#define SYSCALL_MODULE(name, offset, new_sysent, evh, arg) \
This change prevents (I assume) the use of MAKE_SYSENT() in a C++
kernel module, as C++ does not support the .name = value style of
named initializers.
gcc does allow name: value initializers and it's easy to patch it to
accept .name = value, but it's not strictly conforming C++ code
anymore.
I do not mind reverting this, I think it would be better then
having #ifdef __cplusplus and two definitions. I really wanted to
have a way to provide sparce initializator for the struct sysent.
I managed to not require it for r209579.
I agree it's really handy to have sparse initializers; I just haven't
thought of a way to do that and continue to allow 3rd party c++
modules. Perhaps we'll get a new c++ standard soon that supports this
syntax. :-)
Thanks,
matthew
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r209119 - head/sys/sys
On Sun, Jun 13, 2010 at 10:10 AM, Pawel Jakub Dawidek p...@freebsd.org wrote:
On Sun, Jun 13, 2010 at 02:39:55AM +, Lawrence Stewart wrote:
Author: lstewart
Date: Sun Jun 13 02:39:55 2010
New Revision: 209119
URL: http://svn.freebsd.org/changeset/base/209119
Log:
Add a utility macro to simplify calculating an aggregate sum from a DPCPU
counter variable.
Sponsored by: FreeBSD Foundation
Reviewed by: jhb, rpaulo, rwatson (previous version of patch)
MFC after: 1 week
Modified:
head/sys/sys/pcpu.h
Modified: head/sys/sys/pcpu.h
==
--- head/sys/sys/pcpu.h Sun Jun 13 01:27:29 2010 (r209118)
+++ head/sys/sys/pcpu.h Sun Jun 13 02:39:55 2010 (r209119)
@@ -106,6 +106,17 @@ extern uintptr_t dpcpu_off[];
#define DPCPU_ID_GET(i, n) (*DPCPU_ID_PTR(i, n))
#define DPCPU_ID_SET(i, n, v) (*DPCPU_ID_PTR(i, n) = v)
+/*
+ * Utility macros.
+ */
+#define DPCPU_SUM(n, var, sum)
\
+do { \
+ (sum) = 0; \
+ u_int i; \
+ CPU_FOREACH(i) \
+ (sum) += (DPCPU_ID_PTR(i, n))-var; \
+} while (0)
I'd suggest first swapping variable declaration and '(sum) = 0;'.
Also using 'i' as a counter in macro can easly lead to name collision.
If you need to do it, I'd suggest '_i' or something.
Maybe it would be better to make it an inline function rather than macro?
(Relevant but almost a thread hijack):
At Isilon we've run into a lot of problems with variable declarations
in macros, especially with -Wshadow turned on. We ended up
backporting __COUNTER__ from later versions of gcc and then using it
to make unique variable names.
- is the backport (or a fresh implementation) something that could be
done within the scope of the GPL license?
- is it something FreeBSD would be interested in?
- is __COUNTER__ supported by clang?
- if not, could it be?
-Wshadow found several nasty bugs in our code, and apart from a few
spurious warnings it has been handy to have when building our
filesystem.
Thanks,
matthew
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
