svn commit: r244202 - head/sys/netpfil/pf

2012-12-14 Thread Gleb Smirnoff
Author: glebius
Date: Fri Dec 14 08:02:35 2012
New Revision: 244202
URL: http://svnweb.freebsd.org/changeset/base/244202

Log:
  Fix VIMAGE build broken in r244185.
  
  Submitted by: Nikolai Lifanov 

Modified:
  head/sys/netpfil/pf/if_pfsync.c

Modified: head/sys/netpfil/pf/if_pfsync.c
==
--- head/sys/netpfil/pf/if_pfsync.c Fri Dec 14 07:48:03 2012
(r244201)
+++ head/sys/netpfil/pf/if_pfsync.c Fri Dec 14 08:02:35 2012
(r244202)
@@ -615,7 +615,7 @@ pfsync_input(struct mbuf *m, __unused in
 
len = ntohs(ph->len) + offset;
if (m->m_pkthdr.len < len) {
-   pfsyncstats.pfsyncs_badlen++;
+   V_pfsyncstats.pfsyncs_badlen++;
goto done;
}
 
___
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"


svn commit: r244209 - head/sys/sys

2012-12-14 Thread Andrey V. Elsukov
Author: ae
Date: Fri Dec 14 12:37:35 2012
New Revision: 244209
URL: http://svnweb.freebsd.org/changeset/base/244209

Log:
  Add an #include guard to the sys/fnv_hash.h.
  
  MFC after:3 days

Modified:
  head/sys/sys/fnv_hash.h

Modified: head/sys/sys/fnv_hash.h
==
--- head/sys/sys/fnv_hash.h Fri Dec 14 11:38:15 2012(r244208)
+++ head/sys/sys/fnv_hash.h Fri Dec 14 12:37:35 2012(r244209)
@@ -7,6 +7,8 @@
  *
  * $FreeBSD$
  */
+#ifndef _SYS_FNV_HASH_H_
+#define_SYS_FNV_HASH_H_
 
 typedef u_int32_t Fnv32_t;
 typedef u_int64_t Fnv64_t;
@@ -66,3 +68,4 @@ fnv_64_str(const char *str, Fnv64_t hval
}
return hval;
 }
+#endif /* _SYS_FNV_HASH_H_ */
___
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"


svn commit: r244210 - head/sys/netpfil/pf

2012-12-14 Thread Gleb Smirnoff
Author: glebius
Date: Fri Dec 14 13:01:16 2012
New Revision: 244210
URL: http://svnweb.freebsd.org/changeset/base/244210

Log:
Fix error in r235991. No-sleep version of IFNET_RLOCK() should
  be used here, since we may hold the main pf rulesets rwlock.
  
  Reported by:  Fleuriot Damien 

Modified:
  head/sys/netpfil/pf/pf_if.c

Modified: head/sys/netpfil/pf/pf_if.c
==
--- head/sys/netpfil/pf/pf_if.c Fri Dec 14 12:37:35 2012(r244209)
+++ head/sys/netpfil/pf/pf_if.c Fri Dec 14 13:01:16 2012(r244210)
@@ -478,10 +478,10 @@ pfi_table_update(struct pfr_ktable *kt, 
if (kif->pfik_ifp != NULL)
pfi_instance_add(kif->pfik_ifp, net, flags);
else if (kif->pfik_group != NULL) {
-   IFNET_RLOCK();
+   IFNET_RLOCK_NOSLEEP();
TAILQ_FOREACH(ifgm, &kif->pfik_group->ifg_members, ifgm_next)
pfi_instance_add(ifgm->ifgm_ifp, net, flags);
-   IFNET_RUNLOCK();
+   IFNET_RUNLOCK_NOSLEEP();
}
 
if ((e = pfr_set_addrs(&kt->pfrkt_t, V_pfi_buffer, V_pfi_buffer_cnt, 
&size2,
___
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"


svn commit: r244215 - head/sbin/savecore

2012-12-14 Thread Pawel Jakub Dawidek
Author: pjd
Date: Fri Dec 14 15:01:23 2012
New Revision: 244215
URL: http://svnweb.freebsd.org/changeset/base/244215

Log:
  Whitespace cleanups.

Modified:
  head/sbin/savecore/savecore.c

Modified: head/sbin/savecore/savecore.c
==
--- head/sbin/savecore/savecore.c   Fri Dec 14 14:59:35 2012
(r244214)
+++ head/sbin/savecore/savecore.c   Fri Dec 14 15:01:23 2012
(r244215)
@@ -194,7 +194,7 @@ check_space(const char *savedir, off_t d
syslog(LOG_ERR, "%s: %m", savedir);
exit(1);
}
-   spacefree = ((off_t) fsbuf.f_bavail * fsbuf.f_bsize) / 1024;
+   spacefree = ((off_t) fsbuf.f_bavail * fsbuf.f_bsize) / 1024;
totfree = ((off_t) fsbuf.f_bfree * fsbuf.f_bsize) / 1024;
 
(void)snprintf(path, sizeof(path), "%s/minfree", savedir);
@@ -209,7 +209,7 @@ check_space(const char *savedir, off_t d
}
 
needed = dumpsize / 1024 + 2;   /* 2 for info file */
-   if (((minfree > 0) ? spacefree : totfree) - needed < minfree) {
+   if (((minfree > 0) ? spacefree : totfree) - needed < minfree) {
syslog(LOG_WARNING,
"no dump, not enough free space on device (%lld available, need %lld)",
(long long)(minfree > 0 ? spacefree : totfree),
@@ -262,7 +262,7 @@ DoRegularFile(int fd, off_t dumpsize, ch
if (he >= hs + BLOCKSIZE)
break;
}
-   
+
/* back down to a block boundary */
he &= BLOCKMASK;
 
@@ -433,7 +433,7 @@ DoFile(const char *savedir, const char *
syslog(LOG_ERR,
"unknown version (%d) in last dump header on %s",
dtoh32(kdhl.version), device);
-   
+
status = STATUS_BAD;
if (force == 0)
goto closefd;
@@ -444,7 +444,7 @@ DoFile(const char *savedir, const char *
syslog(LOG_ERR,
"unknown version (%d) in last dump header on %s",
dtoh32(kdhl.version), device);
-   
+
status = STATUS_BAD;
if (force == 0)
goto closefd;
@@ -472,7 +472,7 @@ DoFile(const char *savedir, const char *
syslog(LOG_ERR,
"unknown version (%d) in last dump header on %s",
dtoh32(kdhl.version), device);
-   
+
status = STATUS_BAD;
if (force == 0)
goto closefd;
___
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"


svn commit: r244216 - head/sbin/savecore

2012-12-14 Thread Pawel Jakub Dawidek
Author: pjd
Date: Fri Dec 14 15:03:12 2012
New Revision: 244216
URL: http://svnweb.freebsd.org/changeset/base/244216

Log:
  If we are not going to clear the dump (we are either just checking if the dump
  exists or we want to keep it), open device read-only.
  
  Obtained from:WHEEL Systems

Modified:
  head/sbin/savecore/savecore.c

Modified: head/sbin/savecore/savecore.c
==
--- head/sbin/savecore/savecore.c   Fri Dec 14 15:01:23 2012
(r244215)
+++ head/sbin/savecore/savecore.c   Fri Dec 14 15:03:12 2012
(r244216)
@@ -394,7 +394,7 @@ DoFile(const char *savedir, const char *
if (verbose)
printf("checking for kernel dump on device %s\n", device);
 
-   fd = open(device, O_RDWR);
+   fd = open(device, (checkfor || keep) ? O_RDONLY : O_RDWR);
if (fd < 0) {
syslog(LOG_ERR, "%s: %m", device);
return;
@@ -612,7 +612,7 @@ DoFile(const char *savedir, const char *
printf("dump saved\n");
 
 nuke:
-   if (clear || !keep) {
+   if (!keep) {
if (verbose)
printf("clearing dump header\n");
memcpy(kdhl.magic, KERNELDUMPMAGIC_CLEARED, sizeof kdhl.magic);
___
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"


svn commit: r244217 - head/sbin/savecore

2012-12-14 Thread Pawel Jakub Dawidek
Author: pjd
Date: Fri Dec 14 15:04:39 2012
New Revision: 244217
URL: http://svnweb.freebsd.org/changeset/base/244217

Log:
  The clear option (-c) is not compatible with keep (-k) and compress (-z)
  options.
  
  Obtained from:WHEEL Systems

Modified:
  head/sbin/savecore/savecore.c

Modified: head/sbin/savecore/savecore.c
==
--- head/sbin/savecore/savecore.c   Fri Dec 14 15:03:12 2012
(r244216)
+++ head/sbin/savecore/savecore.c   Fri Dec 14 15:04:39 2012
(r244217)
@@ -681,6 +681,8 @@ main(int argc, char **argv)
}
if (checkfor && (clear || force || keep))
usage();
+   if (clear && (compress || keep))
+   usage();
argc -= optind;
argv += optind;
if (argc >= 1) {
___
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"


svn commit: r244218 - in head: etc/rc.d sbin/savecore

2012-12-14 Thread Pawel Jakub Dawidek
Author: pjd
Date: Fri Dec 14 15:12:08 2012
New Revision: 244218
URL: http://svnweb.freebsd.org/changeset/base/244218

Log:
  - When checking if a dump exists on the given device there is no need to
provide dump directory. Eliminate this redundant argument. This changes
the usage, but the only risk here is that a warning will be printed
about directory given as device.
  
  - Update usage of -C option.
  
  - When clearing dump header from the given device there is also no need to
provide dump directory, although additional arguments for -c were not
documented.
  
  - Document that -v can be used with -c and that list of devices can be given.
  
  Obtained from:WHEEL Systems

Modified:
  head/etc/rc.d/savecore
  head/sbin/savecore/savecore.8
  head/sbin/savecore/savecore.c

Modified: head/etc/rc.d/savecore
==
--- head/etc/rc.d/savecore  Fri Dec 14 15:04:39 2012(r244217)
+++ head/etc/rc.d/savecore  Fri Dec 14 15:12:08 2012(r244218)
@@ -62,7 +62,7 @@ savecore_start()
;;
esac
 
-   if savecore -C "${dumpdir}" "${dev}" >/dev/null; then
+   if savecore -C "${dev}" >/dev/null; then
savecore ${savecore_flags} ${dumpdir} ${dumpdev}
if checkyesno crashinfo_enable; then
${crashinfo_program} -d ${dumpdir}

Modified: head/sbin/savecore/savecore.8
==
--- head/sbin/savecore/savecore.8   Fri Dec 14 15:04:39 2012
(r244217)
+++ head/sbin/savecore/savecore.8   Fri Dec 14 15:12:08 2012
(r244218)
@@ -28,7 +28,7 @@
 .\" From: @(#)savecore.8   8.1 (Berkeley) 6/5/93
 .\" $FreeBSD$
 .\"
-.Dd December 24, 2008
+.Dd December 14, 2012
 .Dt SAVECORE 8
 .Os
 .Sh NAME
@@ -37,10 +37,12 @@
 .Sh SYNOPSIS
 .Nm
 .Fl c
+.Op Fl v
+.Op Ar device ...
 .Nm
 .Fl C
 .Op Fl v
-.Op Ar directory device
+.Op Ar device ...
 .Nm
 .Op Fl fkvz
 .Op Ar directory Op Ar device ...

Modified: head/sbin/savecore/savecore.c
==
--- head/sbin/savecore/savecore.c   Fri Dec 14 15:04:39 2012
(r244217)
+++ head/sbin/savecore/savecore.c   Fri Dec 14 15:12:08 2012
(r244218)
@@ -636,8 +636,8 @@ static void
 usage(void)
 {
fprintf(stderr, "%s\n%s\n%s\n",
-   "usage: savecore -c",
-   "   savecore -C [-v] [directory device]",
+   "usage: savecore -c [-v] [device ...]",
+   "   savecore -C [-v] [device ...]",
"   savecore [-fkvz] [directory [device ...]]");
exit (1);
 }
@@ -685,7 +685,7 @@ main(int argc, char **argv)
usage();
argc -= optind;
argv += optind;
-   if (argc >= 1) {
+   if (argc >= 1 && !checkfor && !clear) {
error = chdir(argv[0]);
if (error) {
syslog(LOG_ERR, "chdir(%s): %m", argv[0]);
___
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

2012-12-14 Thread John Baldwin
On Thursday, December 13, 2012 4:02:15 am Gleb Smirnoff wrote:
> On Wed, Dec 12, 2012 at 04:53:48PM -0800, Alfred Perlstein wrote:
> A> The problem again is that not all the KASSERTS are inviolable, if you 
> A> want to do a project to split them, then please do, it would really be 
> A> helpful, as for now, they are a mis-mash of death/warnings and there are 
> A> at least three vendors who approve of this as well as 3 long term 
> A> committers that approved my change (not including Adrian).
> 
> Can you show examples of not inviolable KASSERTs?

There are none.  They are all assertions for a reason.  However, in my 
experience at several large consumers of FreeBSD, no one wants to run with 
INVARIANTS in production.  Not because we don't want panics (believe me, 
Yahoo! gets plenty of panics even with INVARIANTS disabled), but because the 
performance overhead, and redefining INVARIANTS into printf doesn't address 
that at all.

Also, in regards to "if you think an a condition should be inviolable, make it 
a panic".  I _did_ do that in WITNESS and you just reverted them!  In all the 
cases of things like mismatching slock -> xlock you are about to corrupt 
WITNESS' internal state (leading to false positives or missed warnings) as 
well as the state of the locks themselves resulting in either hangs or random 
data corruption.

Also, if you don't have a console wired up on all your machines (which not 
everyone does these days) a hang is _far_ worse than a crashdump, as when the 
machine hangs, you have to power cycle it, and you won't find the messages in 
/var/log/messages.  With a straight-up panic if someone wants to run with 
INVARIANTS enabled they would instead have a nice crashdump that could be 
examined after the machine comes back up that points to the offending 
location.

So in general, I will never use this and find it doesn't add any benefit 
whatsoever.  OTOH, if it is not invasive (e.g. your original commit), then I 
think it might be ok if there are some folks who might actually find it 
useful.  That said, I think direct use of kassert_panic() such as your changes 
to WITNESS is wrong.  If you are going to change explicit panics, make them 
into a KASSERT() instead of changing a panic into a kassert_panic().

-- 
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: r244154 - head/bin/ps

2012-12-14 Thread John Baldwin
On Thursday, December 13, 2012 6:12:44 am Pawel Jakub Dawidek wrote:
> On Wed, Dec 12, 2012 at 11:06:52PM +0200, Konstantin Belousov wrote:
> > On Wed, Dec 12, 2012 at 03:45:04PM +, Pawel Jakub Dawidek wrote:
> > > Author: pjd
> > > Date: Wed Dec 12 15:45:03 2012
> > > New Revision: 244154
> > > URL: http://svnweb.freebsd.org/changeset/base/244154
> > > 
> > > Log:
> > >   Use kern.max_pid sysctl to obtain maximum PID number instead of using 
> > > local
> > >   define.
> > It is pid_max, not max_pid.
> > 
> > But the change is wrong. The kern.pid_max only limits newly allocated pids,
> > it does not magically moves existing pids, which are out of range, to the
> > limited region. See the corresponding commit log for the description.
> > It was added to make it easier to run FreeBSD 1.x binaries on the modern
> > kernels.
> 
> I saw CTLFLAG_TUN on the sysctl and assumed it is read-only...
> How about defining BSD_PID_MAX in sys/proc.h, which would be visible by
> userland as well and setting PID_MAX to BSD_PID_MAX?
> 
> This would also help bsnmpd.
> 
>   http://people.freebsd.org/~pjd/patches/PID_MAX.patch

This doesn't help your actual use case though where you want to boot a kernel
with a different PID_MAX.  I would much rather our tools learn such constants
from the kernel via sysctl than have them compiled in.  So, I would add a new
sysctl which exports the true PID_MAX constant (and is read-only and never
changes) and use that in ps, etc.

-- 
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: r244112 - head/sys/kern

2012-12-14 Thread Andriy Gapon
on 13/12/2012 19:01 m...@freebsd.org said the following:
> 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?

So if some code is a tool and it is disabled by default and someone believes 
that
they will use it, then the code is acceptable to include into the tree no
questions asked?

-- 
Andriy Gapon
___
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"


svn commit: r244224 - head/share/mk

2012-12-14 Thread Ed Maste
Author: emaste
Date: Fri Dec 14 18:30:01 2012
New Revision: 244224
URL: http://svnweb.freebsd.org/changeset/base/244224

Log:
  Minor refactoring prior to .symbols file changes
  
  - Combine .if x and .if !x using .else
  - Separate out beforelinking dependency
  - Add comments to clarify .if nesting
  
  Sponsored by: ADARA Networks

Modified:
  head/share/mk/bsd.lib.mk
  head/share/mk/bsd.prog.mk

Modified: head/share/mk/bsd.lib.mk
==
--- head/share/mk/bsd.lib.mkFri Dec 14 16:28:10 2012(r244223)
+++ head/share/mk/bsd.lib.mkFri Dec 14 18:30:01 2012(r244224)
@@ -39,9 +39,7 @@ CFLAGS+= ${DEBUG_FLAGS}
 .if ${MK_CTF} != "no" && ${DEBUG_FLAGS:M-g} != ""
 CTFFLAGS+= -g
 .endif
-.endif
-
-.if !defined(DEBUG_FLAGS)
+.else
 STRIP?=-s
 .endif
 
@@ -173,10 +171,9 @@ SOLINKOPTS+=   -Wl,--fatal-warnings -Wl,--
 .endif
 
 .if target(beforelinking)
-${SHLIB_NAME}: ${SOBJS} beforelinking
-.else
-${SHLIB_NAME}: ${SOBJS}
+${SHLIB_NAME}: beforelinking
 .endif
+${SHLIB_NAME}: ${SOBJS}
@${ECHO} building shared library ${SHLIB_NAME}
@rm -f ${.TARGET} ${SHLIB_LINK}
 .if defined(SHLIB_LINK)

Modified: head/share/mk/bsd.prog.mk
==
--- head/share/mk/bsd.prog.mk   Fri Dec 14 16:28:10 2012(r244223)
+++ head/share/mk/bsd.prog.mk   Fri Dec 14 18:30:01 2012(r244224)
@@ -46,10 +46,9 @@ PROG=${PROG_CXX}
 OBJS+=  ${SRCS:N*.h:R:S/$/.o/g}
 
 .if target(beforelinking)
-${PROG}: ${OBJS} beforelinking
-.else
-${PROG}: ${OBJS}
+${PROG}: beforelinking
 .endif
+${PROG}: ${OBJS}
 .if defined(PROG_CXX)
${CXX} ${CXXFLAGS} ${LDFLAGS} -o ${.TARGET} ${OBJS} ${LDADD}
 .else
@@ -76,10 +75,9 @@ SRCS=${PROG}.c
 OBJS=  ${PROG}.o
 
 .if target(beforelinking)
-${PROG}: ${OBJS} beforelinking
-.else
-${PROG}: ${OBJS}
+${PROG}: beforelinking
 .endif
+${PROG}: ${OBJS}
 .if defined(PROG_CXX)
${CXX} ${CXXFLAGS} ${LDFLAGS} -o ${.TARGET} ${OBJS} ${LDADD}
 .else
@@ -90,7 +88,7 @@ ${PROG}: ${OBJS}
 .endif
 .endif
 
-.endif
+.endif # !defined(SRCS)
 
 .if${MK_MAN} != "no" && !defined(MAN) && \
!defined(MAN1) && !defined(MAN2) && !defined(MAN3) && \
@@ -100,7 +98,7 @@ ${PROG}: ${OBJS}
 MAN=   ${PROG}.1
 MAN1=  ${MAN}
 .endif
-.endif
+.endif # defined(PROG)
 
 all: objwarn ${PROG} ${SCRIPTS}
 .if ${MK_MAN} != "no"
___
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: r244154 - head/bin/ps

2012-12-14 Thread Pawel Jakub Dawidek
On Fri, Dec 14, 2012 at 11:52:15AM -0500, John Baldwin wrote:
> On Thursday, December 13, 2012 6:12:44 am Pawel Jakub Dawidek wrote:
> > On Wed, Dec 12, 2012 at 11:06:52PM +0200, Konstantin Belousov wrote:
> > > On Wed, Dec 12, 2012 at 03:45:04PM +, Pawel Jakub Dawidek wrote:
> > > > Author: pjd
> > > > Date: Wed Dec 12 15:45:03 2012
> > > > New Revision: 244154
> > > > URL: http://svnweb.freebsd.org/changeset/base/244154
> > > > 
> > > > Log:
> > > >   Use kern.max_pid sysctl to obtain maximum PID number instead of using 
> > > > local
> > > >   define.
> > > It is pid_max, not max_pid.
> > > 
> > > But the change is wrong. The kern.pid_max only limits newly allocated 
> > > pids,
> > > it does not magically moves existing pids, which are out of range, to the
> > > limited region. See the corresponding commit log for the description.
> > > It was added to make it easier to run FreeBSD 1.x binaries on the modern
> > > kernels.
> > 
> > I saw CTLFLAG_TUN on the sysctl and assumed it is read-only...
> > How about defining BSD_PID_MAX in sys/proc.h, which would be visible by
> > userland as well and setting PID_MAX to BSD_PID_MAX?
> > 
> > This would also help bsnmpd.
> > 
> > http://people.freebsd.org/~pjd/patches/PID_MAX.patch
> 
> This doesn't help your actual use case though where you want to boot a kernel
> with a different PID_MAX.  I would much rather our tools learn such constants
> from the kernel via sysctl than have them compiled in.  So, I would add a new
> sysctl which exports the true PID_MAX constant (and is read-only and never
> changes) and use that in ps, etc.

In that case I'd prefer to make existing kern.pid_max sysctl read-only
and make it loader tunable. I don't expect there are many users of this
sysctl...

-- 
Pawel Jakub Dawidek   http://www.wheelsystems.com
FreeBSD committer http://www.FreeBSD.org
Am I Evil? Yes, I Am! http://tupytaj.pl


pgpQfA26S7SNa.pgp
Description: PGP signature


svn commit: r244226 - head/sys/kern

2012-12-14 Thread Rick Macklem
Author: rmacklem
Date: Fri Dec 14 21:49:06 2012
New Revision: 244226
URL: http://svnweb.freebsd.org/changeset/base/244226

Log:
  The group list for a non-default export entry (a host/subnet one)
  was being copied from the wrong place. This patch fixes that.
  This could cause access failures for mapped users, when the group
  permissions were needed.
  
  PR:   147998
  Submitted by: Christopher Key (cjk32 at cam.ac.uk)
  MFC after:2 weeks

Modified:
  head/sys/kern/vfs_export.c

Modified: head/sys/kern/vfs_export.c
==
--- head/sys/kern/vfs_export.c  Fri Dec 14 21:22:23 2012(r244225)
+++ head/sys/kern/vfs_export.c  Fri Dec 14 21:49:06 2012(r244226)
@@ -208,7 +208,7 @@ vfs_hang_addrlist(struct mount *mp, stru
np->netc_anon = crget();
np->netc_anon->cr_uid = argp->ex_anon.cr_uid;
crsetgroups(np->netc_anon, argp->ex_anon.cr_ngroups,
-   np->netc_anon->cr_groups);
+   argp->ex_anon.cr_groups);
np->netc_anon->cr_prison = &prison0;
prison_hold(np->netc_anon->cr_prison);
np->netc_numsecflavors = argp->ex_numsecflavors;
___
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: r244188 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

2012-12-14 Thread Pawel Jakub Dawidek
On Thu, Dec 13, 2012 at 05:39:08PM +, Steven Hartland wrote:
> Author: smh
> Date: Thu Dec 13 17:39:07 2012
> New Revision: 244188
> URL: http://svnweb.freebsd.org/changeset/base/244188
> 
> Log:
>   Added vfs.zfs.vdev.trim_on_init sysctl which allows full vdev trim on
>   initialisation to be enabled (1) / disabled (0) defaults to enabled.
>   
>   This is useful for devices which have a slow trim speed and are either
>   new or have otherwise already been wiped e.g. secure erase.
>   
>   PR: kern/173116
>   Submitted by:   Steven Hartland
>   Approved by:pjd (mentor)
> 
> Modified:
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_label.c
> 
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_label.c
> ==
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_label.c  Thu Dec 
> 13 17:06:38 2012(r244187)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_label.c  Thu Dec 
> 13 17:39:07 2012(r244188)
> @@ -148,6 +148,11 @@
>  #include 
>  #include 
>  
> +static boolean_t vdev_trim_on_init = B_TRUE;
> +SYSCTL_DECL(_vfs_zfs_vdev);
> +SYSCTL_INT(_vfs_zfs_vdev, OID_AUTO, trim_on_init, CTLFLAG_RW,
> +&vdev_trim_on_init, 0, "Enable/disable full vdev trim on 
> initialisation");

I'd also make it loader tunable.

-- 
Pawel Jakub Dawidek   http://www.wheelsystems.com
FreeBSD committer http://www.FreeBSD.org
Am I Evil? Yes, I Am! http://tupytaj.pl


pgpnNSjsjlwVE.pgp
Description: PGP signature


Re: svn commit: r244193 - head/sys/x86/include

2012-12-14 Thread Carl Delsey

On 12/13/12 21:49, Bruce Evans wrote:

On Thu, 13 Dec 2012, Jim Harris wrote:


Log:
 Add bus_space_read_8 and bus_space_write_8 for amd64.

 Rather than trying to KASSERT for callers that invoke this on
 IO tags, either do nothing (for write_8) or return ~0 (for read_8).


read_8 returns a uint64_t, so it cannot return the signed integer ~0.
It actually returns this signed integer converted to uint64_t.  On
amd64, this is the 32 bit value 0x.  The 64-bit value
0x should be returned.
I double checked and 0x is being returned when compiled 
by both clang and gcc.



 Using KASSERT here just makes bus.h too messy from both
 polluting bus.h with systm.h (for any number of drivers that include
 bus.h without first including systm.h) or ports that use bus.h
 directly (i.e. libpciaccess) as reported by zeising@.

 Also don't try to implement all of the other bus_space functions for
 8 byte access since realistically only these two are needed for some
 devices that expose 64-bit memory-mapped registers.


Good.

 Put the amd64-specific functions here rather than 
sys/amd64/include/bus.h

 so that we can keep this header unified for x86, as requested by mdf@
 and tijl@.


Not so good.  I don't really like any of the unified headers.
Me neither, but it sounds like there is a good reason for it, which I'll 
admit I don't fully understand. Sounds like it has something to do with 
cross-compiling.



Modified: head/sys/x86/include/bus.h
== 


--- head/sys/x86/include/bus.hThu Dec 13 21:39:59 2012 (r244192)
+++ head/sys/x86/include/bus.hThu Dec 13 21:40:11 2012 (r244193)
@@ -130,6 +130,7 @@
#define BUS_SPACE_MAXADDR0x
#endif


This file spells the F in hex constants in upper case.

In the above definition and in previous ones, it is careful to spell out
the constants and not depend on sign extension.  So it is also a style
bug to use ~0.
Are you saying it is a style bug to not match the style used above, 
regardless of whether that style is right or wrong, or are you saying 
(~0)  is always a style bug?




Style bug visible in the above: space instead of tab after #define.  This
style bugs is in all #define's near here, including the new one.

Type error in #define's just before the above: the above 
BUS_SPACE_MAXADDR

is for 32 bits.  For amd64 and i386-PAE, BUS_SPACE_MAXADDR is of course
64 bits, but the ifdef tangle for it is not tangled enough to be correct:
BUS_SPACE_MAXADDR is 0xULL, on both, but bus_addr_t is 
only

unsigned long long on i386-PAE.
I think this should be a separate patch though, since it is unrelated to 
this change.




+#define BUS_SPACE_INVALID_DATA(~0)
#define BUS_SPACE_UNRESTRICTED(~0)

/*
@@ -221,6 +222,12 @@ static __inline u_int32_t bus_space_read
   bus_space_handle_t handle,
   bus_size_t offset);

+#ifdef __amd64__
+static __inline uint64_t bus_space_read_8(bus_space_tag_t tag,
+  bus_space_handle_t handle,
+  bus_size_t offset);
+#endif
+


This is style-bug for bug compatible with the existing forward
declarations.  Forward declarations of inline functions are nonsense.
They are from NetBSD, for K&R support.  But FreeBSD never supported
K&R in bus-space headers, and the forward declarations never even
compiled with K&R, since they never used __P(()).  Almost 1/3 of the
x86 bus.h consists of these negatively useful forward declarations.
Some of them are almost as large as the full functions, since they are
misformatted worse, with parameters starting at about column 40 instead
of about column 20, so so that many lines are needed just for the
parameters (to line them up in perfectly non-KNF gnu style).

Same with this - separate patch



...
@@ -251,8 +258,16 @@ bus_space_read_4(bus_space_tag_t tag, bu
return (*(volatile u_int32_t *)(handle + offset));
}

-#if 0/* Cause a link error for bus_space_read_8 */
-#definebus_space_read_8(t, h, o)!!! bus_space_read_8 
unimplemented !!!

+#ifdef __amd64__
+static __inline uint64_t
+bus_space_read_8(bus_space_tag_t tag, bus_space_handle_t handle,
+ bus_size_t offset)
+{
+
+if (tag == X86_BUS_SPACE_IO) /* No 8 byte IO space access on x86 */


The comment is not indented, and should probably not be placed to the
right of the code.  This file mostly doesn't place comments there, and
when it does it doesn't capitalize them.  One that does also spells IO
as i/o.
All the #if 0 lines also start with an end of line comment, and they are 
all capitalized. By "not indented" are you saying that all end of line 
comments must be preceded by a tab?



+return (BUS_SPACE_INVALID_DATA);
+return (*(volatile uint64_t *)(handle + offset));
+}
#endif


Bruce
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn

svn commit: r244236 - head/share/mk

2012-12-14 Thread Ed Maste
Author: emaste
Date: Sat Dec 15 00:03:35 2012
New Revision: 244236
URL: http://svnweb.freebsd.org/changeset/base/244236

Log:
  Put shared library debug info into separate .symbols file
  
  Sponsored by: ADARA Networks

Modified:
  head/share/mk/bsd.lib.mk
  head/share/mk/sys.mk

Modified: head/share/mk/bsd.lib.mk
==
--- head/share/mk/bsd.lib.mkFri Dec 14 23:13:06 2012(r244235)
+++ head/share/mk/bsd.lib.mkSat Dec 15 00:03:35 2012(r244236)
@@ -173,11 +173,15 @@ SOLINKOPTS+=  -Wl,--fatal-warnings -Wl,--
 .if target(beforelinking)
 ${SHLIB_NAME}: beforelinking
 .endif
+.if defined(DEBUG_FLAGS)
+${SHLIB_NAME}.debug: ${SOBJS}
+.else
 ${SHLIB_NAME}: ${SOBJS}
+.endif
@${ECHO} building shared library ${SHLIB_NAME}
-   @rm -f ${.TARGET} ${SHLIB_LINK}
+   @rm -f ${SHLIB_NAME} ${SHLIB_LINK}
 .if defined(SHLIB_LINK)
-   @ln -fs ${.TARGET} ${SHLIB_LINK}
+   @ln -fs ${SHLIB_NAME} ${SHLIB_LINK}
 .endif
 .if !defined(NM)
@${CC} ${LDFLAGS} ${SSP_CFLAGS} ${SOLINKOPTS} \
@@ -191,6 +195,15 @@ ${SHLIB_NAME}: ${SOBJS}
 .if ${MK_CTF} != "no"
${CTFMERGE} ${CTFFLAGS} -o ${.TARGET} ${SOBJS}
 .endif
+
+.if defined(DEBUG_FLAGS)
+${SHLIB_NAME}: ${SHLIB_NAME}.debug ${SHLIB_NAME}.symbols
+   ${OBJCOPY} --strip-debug --add-gnu-debuglink=${SHLIB_NAME}.symbols \
+   ${SHLIB_NAME}.debug ${.TARGET}
+
+${SHLIB_NAME}.symbols: ${SHLIB_NAME}.debug
+   ${OBJCOPY} --only-keep-debug ${SHLIB_NAME}.debug ${.TARGET}
+.endif
 .endif
 
 .if defined(INSTALL_PIC_ARCHIVE) && defined(LIB) && !empty(LIB) && 
${MK_TOOLCHAIN} != "no"
@@ -267,6 +280,11 @@ _libinstall:
${INSTALL} ${STRIP} -o ${LIBOWN} -g ${LIBGRP} -m ${LIBMODE} \
${_INSTALLFLAGS} ${_SHLINSTALLFLAGS} \
${SHLIB_NAME} ${DESTDIR}${SHLIBDIR}
+.if defined(DEBUG_FLAGS)
+   ${INSTALL} -o ${LIBOWN} -g ${LIBGRP} -m ${LIBMODE} \
+   ${_INSTALLFLAGS} ${_SHLINSTALLFLAGS} \
+   ${SHLIB_NAME}.symbols ${DESTDIR}${SHLIBDIR}
+.endif
 .if defined(SHLIB_LINK)
 # ${_SHLIBDIRPREFIX} and ${_LDSCRIPTROOT} are both needed when cross-building
 # and when building 32 bits library shims.  ${_SHLIBDIRPREFIX} is the directory

Modified: head/share/mk/sys.mk
==
--- head/share/mk/sys.mkFri Dec 14 23:13:06 2012(r244235)
+++ head/share/mk/sys.mkSat Dec 15 00:03:35 2012(r244236)
@@ -134,6 +134,8 @@ NM  ?=  nm
 OBJC   ?=  cc
 OBJCFLAGS  ?=  ${OBJCINCLUDES} ${CFLAGS} -Wno-import
 
+OBJCOPY?=  objcopy
+
 PC ?=  pc
 PFLAGS ?=
 
___
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: r244236 - head/share/mk

2012-12-14 Thread Garrett Cooper
On Fri, Dec 14, 2012 at 4:03 PM, Ed Maste  wrote:
> Author: emaste
> Date: Sat Dec 15 00:03:35 2012
> New Revision: 244236
> URL: http://svnweb.freebsd.org/changeset/base/244236

...

> Modified: head/share/mk/sys.mk
> ==
> --- head/share/mk/sys.mkFri Dec 14 23:13:06 2012(r244235)
> +++ head/share/mk/sys.mkSat Dec 15 00:03:35 2012(r244236)
> @@ -134,6 +134,8 @@ NM  ?=  nm
>  OBJC   ?=  cc
>  OBJCFLAGS  ?=  ${OBJCINCLUDES} ${CFLAGS} -Wno-import
>
> +OBJCOPY?=  objcopy
> +
>  PC ?=  pc
>  PFLAGS ?=

This shouldn't be defined when !defined(%POSIX) is true. The
.endif should be pulled down to just above SHELL?= according to what
I'm reading in 
http://pubs.opengroup.org/onlinepubs/009695399/utilities/make.html
.
Other spots in the tree where OBJCOPY?= is noted can be probably
start to be reaped (thinking about sys/boot/, etc in particular). I
have a PR open for some work Warner started with an initial patch that
I've kind of been keeping up to date in git, but it needs to be
updated now per this change (and also because I didn't notice that
these variables weren't supposed to be defined when POSIX mode was
on).
Thanks!
-Garrett
___
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: r244188 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

2012-12-14 Thread Steven Hartland
- Original Message - 
From: "Pawel Jakub Dawidek" 

On Thu, Dec 13, 2012 at 05:39:08PM +, Steven Hartland wrote:

Author: smh
Date: Thu Dec 13 17:39:07 2012
New Revision: 244188
URL: http://svnweb.freebsd.org/changeset/base/244188

Log:
  Added vfs.zfs.vdev.trim_on_init sysctl which allows full vdev trim on
  initialisation to be enabled (1) / disabled (0) defaults to enabled.
  
  This is useful for devices which have a slow trim speed and are either

  new or have otherwise already been wiped e.g. secure erase.
  

...

+static boolean_t vdev_trim_on_init = B_TRUE;
+SYSCTL_DECL(_vfs_zfs_vdev);>
+SYSCTL_INT(_vfs_zfs_vdev, OID_AUTO, trim_on_init, CTLFLAG_RW,
+&vdev_trim_on_init, 0, "Enable/disable full vdev trim on initialisation");


I'd also make it loader tunable.


Will look into that, worth doing the other SYSCTL only ones at the same time?

   Regards
   Steve

___
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: r244154 - head/bin/ps

2012-12-14 Thread Konstantin Belousov
On Fri, Dec 14, 2012 at 10:42:47PM +0100, Pawel Jakub Dawidek wrote:
> On Fri, Dec 14, 2012 at 11:52:15AM -0500, John Baldwin wrote:
> > On Thursday, December 13, 2012 6:12:44 am Pawel Jakub Dawidek wrote:
> > > On Wed, Dec 12, 2012 at 11:06:52PM +0200, Konstantin Belousov wrote:
> > > > On Wed, Dec 12, 2012 at 03:45:04PM +, Pawel Jakub Dawidek wrote:
> > > > > Author: pjd
> > > > > Date: Wed Dec 12 15:45:03 2012
> > > > > New Revision: 244154
> > > > > URL: http://svnweb.freebsd.org/changeset/base/244154
> > > > > 
> > > > > Log:
> > > > >   Use kern.max_pid sysctl to obtain maximum PID number instead of 
> > > > > using local
> > > > >   define.
> > > > It is pid_max, not max_pid.
> > > > 
> > > > But the change is wrong. The kern.pid_max only limits newly allocated 
> > > > pids,
> > > > it does not magically moves existing pids, which are out of range, to 
> > > > the
> > > > limited region. See the corresponding commit log for the description.
> > > > It was added to make it easier to run FreeBSD 1.x binaries on the modern
> > > > kernels.
> > > 
> > > I saw CTLFLAG_TUN on the sysctl and assumed it is read-only...
> > > How about defining BSD_PID_MAX in sys/proc.h, which would be visible by
> > > userland as well and setting PID_MAX to BSD_PID_MAX?
> > > 
> > > This would also help bsnmpd.
> > > 
> > >   http://people.freebsd.org/~pjd/patches/PID_MAX.patch
> > 
> > This doesn't help your actual use case though where you want to boot a 
> > kernel
> > with a different PID_MAX.  I would much rather our tools learn such 
> > constants
> > from the kernel via sysctl than have them compiled in.  So, I would add a 
> > new
> > sysctl which exports the true PID_MAX constant (and is read-only and never
> > changes) and use that in ps, etc.
> 
> In that case I'd prefer to make existing kern.pid_max sysctl read-only
> and make it loader tunable. I don't expect there are many users of this
> sysctl...
> 
No, I described you the purpose of the sysctl. Requiring reboot just for
running the old binaries is not useful. Please do not break it.


pgpF4mkFwGjjP.pgp
Description: PGP signature


svn commit: r244237 - head/sys/kern

2012-12-14 Thread Konstantin Belousov
Author: kib
Date: Sat Dec 15 02:02:11 2012
New Revision: 244237
URL: http://svnweb.freebsd.org/changeset/base/244237

Log:
  Remove a special case for XEN, which is erronous and makes vfork(2)
  behaviour to differ from the documented, only on XEN.  If there are
  any issues with XEN pmap left, they should be fixed in pmap.
  
  MFC after:2 weeks

Modified:
  head/sys/kern/kern_fork.c

Modified: head/sys/kern/kern_fork.c
==
--- head/sys/kern/kern_fork.c   Sat Dec 15 00:03:35 2012(r244236)
+++ head/sys/kern/kern_fork.c   Sat Dec 15 02:02:11 2012(r244237)
@@ -150,11 +150,7 @@ sys_vfork(struct thread *td, struct vfor
int error, flags;
struct proc *p2;
 
-#ifdef XEN
-   flags = RFFDG | RFPROC; /* validate that this is still an issue */
-#else
flags = RFFDG | RFPROC | RFPPWAIT | RFMEM;
-#endif 
error = fork1(td, flags, 0, &p2, NULL, 0);
if (error == 0) {
td->td_retval[0] = p2->p_pid;
___
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"


svn commit: r244238 - head/sys/sys

2012-12-14 Thread Konstantin Belousov
Author: kib
Date: Sat Dec 15 02:03:06 2012
New Revision: 244238
URL: http://svnweb.freebsd.org/changeset/base/244238

Log:
  Line up the continuation backslashes.
  
  Sponsored by: The FreeBSD Foundation
  MFC after:3 days

Modified:
  head/sys/sys/mount.h

Modified: head/sys/sys/mount.h
==
--- head/sys/sys/mount.hSat Dec 15 02:02:11 2012(r244237)
+++ head/sys/sys/mount.hSat Dec 15 02:03:06 2012(r244238)
@@ -199,8 +199,8 @@ struct vnode *__mnt_vnode_next_all(struc
 struct vnode *__mnt_vnode_first_all(struct vnode **mvp, struct mount *mp);
 void  __mnt_vnode_markerfree_all(struct vnode **mvp, struct mount *mp);
 
-#define MNT_VNODE_FOREACH_ALL(vp, mp, mvp) \
-   for (vp = __mnt_vnode_first_all(&(mvp), (mp)); \
+#define MNT_VNODE_FOREACH_ALL(vp, mp, mvp) \
+   for (vp = __mnt_vnode_first_all(&(mvp), (mp));  \
(vp) != NULL; vp = __mnt_vnode_next_all(&(mvp), (mp)))
 
 #define MNT_VNODE_FOREACH_ALL_ABORT(mp, mvp)   \
___
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"


svn commit: r244239 - head/sys/ufs/ufs

2012-12-14 Thread Konstantin Belousov
Author: kib
Date: Sat Dec 15 02:03:59 2012
New Revision: 244239
URL: http://svnweb.freebsd.org/changeset/base/244239

Log:
  Fix a typo, resulting in the NULL pointer dereference.
  
  Reported and tested by:   pho
  Sponsored by: The FreeBSD Foundation
  MFC after:3 days

Modified:
  head/sys/ufs/ufs/ufs_quota.c

Modified: head/sys/ufs/ufs/ufs_quota.c
==
--- head/sys/ufs/ufs/ufs_quota.cSat Dec 15 02:03:06 2012
(r244238)
+++ head/sys/ufs/ufs/ufs_quota.cSat Dec 15 02:03:59 2012
(r244239)
@@ -1044,7 +1044,7 @@ again:
error = vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, td);
if (error) {
if (error == ENOENT) {
-   MNT_VNODE_FOREACH_ALL_ABORT(mp, mvp);
+   MNT_VNODE_FOREACH_ACTIVE_ABORT(mp, mvp);
goto again;
}
continue;
___
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"


svn commit: r244240 - in head/sys: kern sys

2012-12-14 Thread Konstantin Belousov
Author: kib
Date: Sat Dec 15 02:04:46 2012
New Revision: 244240
URL: http://svnweb.freebsd.org/changeset/base/244240

Log:
  When mnt_vnode_next_active iterator cannot lock the next vnode and
  yields, specify the user priority for the yield.  Otherwise, a
  higher-priority (kernel) thread could fall into the priority-inversion
  with the thread owning the mutex lock.
  
  On single-processor machines or UP kernels, do not loop adaptively
  when the next vnode cannot be locked, instead yield unconditionally.
  
  Restructure the iteration initializer and the iterator to remove code
  duplication.  Put the code to fetch and lock a vnode next to the
  current marker, into the mnt_vnode_next_active() function, and use it
  instead of repeating the loop.
  
  Reported by:  hrs, rmacklem
  Tested by:pho
  Sponsored by: The FreeBSD Foundation
  MFC after:3 days

Modified:
  head/sys/kern/vfs_subr.c
  head/sys/sys/mount.h

Modified: head/sys/kern/vfs_subr.c
==
--- head/sys/kern/vfs_subr.cSat Dec 15 02:03:59 2012(r244239)
+++ head/sys/kern/vfs_subr.cSat Dec 15 02:04:46 2012(r244240)
@@ -69,6 +69,7 @@ __FBSDID("$FreeBSD$");
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -4710,32 +4711,54 @@ __mnt_vnode_markerfree_all(struct vnode 
  * These are helper functions for filesystems to traverse their
  * active vnodes.  See MNT_VNODE_FOREACH_ACTIVE() in sys/mount.h
  */
-struct vnode *
-__mnt_vnode_next_active(struct vnode **mvp, struct mount *mp)
+static void
+mnt_vnode_markerfree_active(struct vnode **mvp, struct mount *mp)
+{
+
+   KASSERT((*mvp)->v_mount == mp, ("marker vnode mount list mismatch"));
+
+   MNT_ILOCK(mp);
+   MNT_REL(mp);
+   MNT_IUNLOCK(mp);
+   free(*mvp, M_VNODE_MARKER);
+   *mvp = NULL;
+}
+
+#ifdef SMP
+#defineALWAYS_YIELD(mp_ncpus == 1)
+#else
+#defineALWAYS_YIELD1
+#endif
+
+static struct vnode *
+mnt_vnode_next_active(struct vnode **mvp, struct mount *mp)
 {
struct vnode *vp, *nvp;
 
-   if (should_yield())
-   kern_yield(PRI_UNCHANGED);
-   mtx_lock(&vnode_free_list_mtx);
-restart:
+   mtx_assert(&vnode_free_list_mtx, MA_OWNED);
KASSERT((*mvp)->v_mount == mp, ("marker vnode mount list mismatch"));
+restart:
vp = TAILQ_NEXT(*mvp, v_actfreelist);
+   TAILQ_REMOVE(&mp->mnt_activevnodelist, *mvp, v_actfreelist);
while (vp != NULL) {
if (vp->v_type == VMARKER) {
vp = TAILQ_NEXT(vp, v_actfreelist);
continue;
}
if (!VI_TRYLOCK(vp)) {
-   if (should_yield()) {
+   if (ALWAYS_YIELD || should_yield()) {
+   TAILQ_INSERT_BEFORE(vp, *mvp, v_actfreelist);
mtx_unlock(&vnode_free_list_mtx);
-   kern_yield(PRI_UNCHANGED);
+   kern_yield(PRI_USER);
mtx_lock(&vnode_free_list_mtx);
+   goto restart;
}
-   goto restart;
+   continue;
}
-   if (vp->v_mount == mp && vp->v_type != VMARKER &&
-   (vp->v_iflag & VI_DOOMED) == 0)
+   KASSERT(vp->v_type != VMARKER, ("locked marker %p", vp));
+   KASSERT(vp->v_mount == mp || vp->v_mount == NULL,
+   ("alien vnode on the active list %p %p", vp, mp));
+   if (vp->v_mount == mp && (vp->v_iflag & VI_DOOMED) == 0)
break;
nvp = TAILQ_NEXT(vp, v_actfreelist);
VI_UNLOCK(vp);
@@ -4745,22 +4768,31 @@ restart:
/* Check if we are done */
if (vp == NULL) {
mtx_unlock(&vnode_free_list_mtx);
-   __mnt_vnode_markerfree_active(mvp, mp);
-   mtx_assert(MNT_MTX(mp), MA_NOTOWNED);
+   mnt_vnode_markerfree_active(mvp, mp);
return (NULL);
}
-   TAILQ_REMOVE(&mp->mnt_activevnodelist, *mvp, v_actfreelist);
TAILQ_INSERT_AFTER(&mp->mnt_activevnodelist, vp, *mvp, v_actfreelist);
mtx_unlock(&vnode_free_list_mtx);
ASSERT_VI_LOCKED(vp, "active iter");
KASSERT((vp->v_iflag & VI_ACTIVE) != 0, ("Non-active vp %p", vp));
return (vp);
 }
+#undef ALWAYS_YIELD
+
+struct vnode *
+__mnt_vnode_next_active(struct vnode **mvp, struct mount *mp)
+{
+
+   if (should_yield())
+   kern_yield(PRI_UNCHANGED);
+   mtx_lock(&vnode_free_list_mtx);
+   return (mnt_vnode_next_active(mvp, mp));
+}
 
 struct vnode *
 __mnt_vnode_first_active(struct vnode **mvp, struct mount *mp)
 {
-   struct vnode *vp, *nvp;
+   struct vnode *vp;
 
*mvp = malloc(sizeof(struct vnode), M_VNODE_MARKER, M_WAIT

Re: svn commit: r244112 - head/sys/kern

2012-12-14 Thread Robert Watson

On Fri, 14 Dec 2012, John Baldwin wrote:


On Thursday, December 13, 2012 4:02:15 am Gleb Smirnoff wrote:
On Wed, Dec 12, 2012 at 04:53:48PM -0800, Alfred Perlstein wrote: A> The 
problem again is that not all the KASSERTS are inviolable, if you A> want 
to do a project to split them, then please do, it would really be A> 
helpful, as for now, they are a mis-mash of death/warnings and there are A> 
at least three vendors who approve of this as well as 3 long term A> 
committers that approved my change (not including Adrian).


Can you show examples of not inviolable KASSERTs?


There are none.  They are all assertions for a reason.  However, in my 
experience at several large consumers of FreeBSD, no one wants to run with 
INVARIANTS in production.  Not because we don't want panics (believe me, 
Yahoo! gets plenty of panics even with INVARIANTS disabled), but because the 
performance overhead, and redefining INVARIANTS into printf doesn't address 
that at all.


In the past, FYI, the two major INVARIANTS hits were un-inlining of locking, 
and UMA using additional global locking to perform heavier-weight sanity 
checking.  For me, at least, the former wasn't such a problem, but the latter 
was extremely measurable.  I have occasionally wondered if we should have 
another option name for heavier-duty sanity checking, as is the case with 
WITNESS, SOCKBUF_DEBUG, etc, so that INVARIANTS overhead can be made tolerable 
for more real-world installations.  As you observe, our invariants tests are 
generally things where you catch critical problems in much more debuggable 
states, rather than sources of additional fragility, and it would be great if 
we could leave more in so that bug reports were able to contain more 
information.


Robert
___
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

2012-12-14 Thread Alfred Perlstein

On 12/14/12 4:12 PM, Robert Watson wrote:

On Fri, 14 Dec 2012, John Baldwin wrote:


On Thursday, December 13, 2012 4:02:15 am Gleb Smirnoff wrote:
On Wed, Dec 12, 2012 at 04:53:48PM -0800, Alfred Perlstein wrote: A> 
The problem again is that not all the KASSERTS are inviolable, if 
you A> want to do a project to split them, then please do, it would 
really be A> helpful, as for now, they are a mis-mash of 
death/warnings and there are A> at least three vendors who approve 
of this as well as 3 long term A> committers that approved my change 
(not including Adrian).


Can you show examples of not inviolable KASSERTs?


There are none.  They are all assertions for a reason.  However, in 
my experience at several large consumers of FreeBSD, no one wants to 
run with INVARIANTS in production.  Not because we don't want panics 
(believe me, Yahoo! gets plenty of panics even with INVARIANTS 
disabled), but because the performance overhead, and redefining 
INVARIANTS into printf doesn't address that at all.


In the past, FYI, the two major INVARIANTS hits were un-inlining of 
locking, and UMA using additional global locking to perform 
heavier-weight sanity checking.  For me, at least, the former wasn't 
such a problem, but the latter was extremely measurable.  I have 
occasionally wondered if we should have another option name for 
heavier-duty sanity checking, as is the case with WITNESS, 
SOCKBUF_DEBUG, etc, so that INVARIANTS overhead can be made tolerable 
for more real-world installations.  As you observe, our invariants 
tests are generally things where you catch critical problems in much 
more debuggable states, rather than sources of additional fragility, 
and it would be great if we could leave more in so that bug reports 
were able to contain more information.


Robert


Yes.

The KTR system offers an interesting reference for a model that allows 
us to make a compile time decision about which areas to trace.


There used to be a DIAGNOSTIC option for the more heavy checks, this is 
either removed or just not used these days.  Again, using a mask like 
KTR might be a win if we bring back the equivalent of heavy weight 
DIAGNOSTIC option.


Often I've been guilty of putting KASSERT(ptr != NULL) checks into the 
code too.  Those are really just to make it less painful when hitting 
that bug, so instead of having to do a lot of homework to figure out the 
fault address and line number, I can just get a pretty printed message 
under INVARIANTS.  Maybe those "pretty null checks" need to go under 
another option, perhaps DIAGNOSTIC, or another .. ??PRETTY_PANIC.


Anyhow, I've always been a huge fan of FreeBSD due the additional 
debugging checks it offered.  I was the one that suggested splassert() 
back in the day when we would continually find issues with spl calls.  
Additionally I found doing filesystem work a ton easier with 
DEBUG_VFS_LOCKS under FreeBSD than under Darwin which at the time did 
not have such a feature.


One of my great joys in developing FreeBSD is the flexibility and power 
it offers us as developers by giving us a huge library of debugging tools.


-Alfred
___
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: r244105 - in head/sys: kern sys

2012-12-14 Thread Rui Paulo
On 12 Dec 2012, at 07:42, John Baldwin  wrote:

> On Monday, December 10, 2012 8:23:51 pm Alfred Perlstein wrote:
>> Author: alfred
>> Date: Tue Dec 11 01:23:50 2012
>> New Revision: 244105
>> URL: http://svnweb.freebsd.org/changeset/base/244105
>> 
>> Log:
>>  Switch the hardwired WITNESS panics to kassert_panic.
>> 
>>  This is an ongoing effort to provide runtime debug information
>>  useful in the field that does not panic existing installations.
>> 
>>  This gives us the flexibility needed when shipping images to a
>>  potentially large audience with WITNESS enabled without worrying
>>  about formerly non-fatal LORs hurting a release.
>> 
>>  Sponsored by: iXsystems
> 
> Witness doesn't panic on LORs.  These are all bigger violations for things 
> like doing sx_sunlock() on a exclusively locked sx lock.  That is not safe 
> and 
> is merely going to result in data corruption and other unpleasantness.  This 
> sounds like a very bad idea.  Did you talk about this anywhere (I have not 
> caught up on various lists yet, so apologies if this has been discussed.)


Eww. This is indeed pretty bad. I would like to see this commit reverted.

Regards,
--
Rui Paulo

___
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: r244154 - head/bin/ps

2012-12-14 Thread Bruce Evans

On Sat, 15 Dec 2012, Konstantin Belousov wrote:


On Fri, Dec 14, 2012 at 10:42:47PM +0100, Pawel Jakub Dawidek wrote:

On Fri, Dec 14, 2012 at 11:52:15AM -0500, John Baldwin wrote:

On Thursday, December 13, 2012 6:12:44 am Pawel Jakub Dawidek wrote:

On Wed, Dec 12, 2012 at 11:06:52PM +0200, Konstantin Belousov wrote:

On Wed, Dec 12, 2012 at 03:45:04PM +, Pawel Jakub Dawidek wrote:

Author: pjd
Date: Wed Dec 12 15:45:03 2012
New Revision: 244154
URL: http://svnweb.freebsd.org/changeset/base/244154

Log:
  Use kern.max_pid sysctl to obtain maximum PID number instead of using local
  define.

It is pid_max, not max_pid.

But the change is wrong. The kern.pid_max only limits newly allocated pids,
it does not magically moves existing pids, which are out of range, to the
limited region. See the corresponding commit log for the description.
It was added to make it easier to run FreeBSD 1.x binaries on the modern
kernels.


I saw CTLFLAG_TUN on the sysctl and assumed it is read-only...
How about defining BSD_PID_MAX in sys/proc.h, which would be visible by
userland as well and setting PID_MAX to BSD_PID_MAX?

This would also help bsnmpd.

http://people.freebsd.org/~pjd/patches/PID_MAX.patch


This doesn't help your actual use case though where you want to boot a kernel
with a different PID_MAX.  I would much rather our tools learn such constants
from the kernel via sysctl than have them compiled in.  So, I would add a new
sysctl which exports the true PID_MAX constant (and is read-only and never
changes) and use that in ps, etc.


In that case I'd prefer to make existing kern.pid_max sysctl read-only
and make it loader tunable. I don't expect there are many users of this
sysctl...


No, I described you the purpose of the sysctl. Requiring reboot just for
running the old binaries is not useful. Please do not break it.


ps doesn't make any useful use of either its BSD_PID_MAX or the kernel's
PID_MAX anyway.  Its only use is to break cases where these BSD_PID_MAX
is smaller than the largest active pid.  There are some useful uses of
a limit:
- to "optimize" the parsing function by not supporting pids larger than
  necessary
- for formatting pids.  The 9 limit is useful for restricting the width
  of pid fields.  With a dynamic limit, you need complications for dynamic
  widths.  With a larger fixed limit, you need complications for dyamic
  widths anyway, since after expanding the pid fields there is no longer
  enough space for fixed widths in other fields.

  BTW, someone broke the formatting for ruptime(1) in -current.  It
  now uses a fixed and _far too large_ width for the host name, so that
  most lines have length precisely 80 and get double-spaced by
  auto=linefeed on 80-column terminals.

  "ps l" still uses < 80 columns, with 2 5-digit fields for pids.  Wide
  fields still eat into the field width for the COMMAND field.
  Interestingly, someone improved the formatting for the UID field -- it
  now seems to be dynamic and usually takes 3-4 columns, where it used to
  have the fixed width 5.
ps of course doesn't use the limit for these useful uses:

% #define   BSD_PID_MAX 9   /* Copy of PID_MAX from 
sys/proc.h. */
% static int
% addelem_pid(struct listinfo *inf, const char *elem)
% {
%   char *endp;
%   long tempid;
% 
% 	if (*elem == '\0') {

%   warnx("Invalid (zero-length) process id");
%   optfatal = 1;
%   return (0); /* Do not add this value. */
%   }
% 
% 	errno = 0;

%   tempid = strtol(elem, &endp, 10);

Here ps assumes that the limit is < LONG_MAX.  A reasonable assumption.

%   if (*endp != '\0' || tempid < 0 || elem == endp) {
%   warnx("Invalid %s: %s", inf->lname, elem);
%   errno = ERANGE;
%   } else if (errno != 0 || tempid > BSD_PID_MAX) {
%   warnx("%s too large: %s", inf->lname, elem);
%   errno = ERANGE;
%   }

All ps does with BSD_PID_MAX is to give an up-front error for values that
are so large that they can't match any pids.  It would be harmless for
it to simply accept all representable values and let them not match any
actual pid later.

Example uses:
- ps -p 0,745,756: all 3 pids are accepted
- ps -p -1:-1 is rejected because it gives tempid < -1
- ps -p 9: 9 is accepted because it is at the limit
- ps -p 99:99 is rejected because it is above the limit

%   if (errno == ERANGE) {
%   optfatal = 1;
%   return (0);
%   }
%   if (inf->count >= inf->maxcount)
%   expand_list(inf);
%   inf->l.pids[(inf->count)++] = tempid;

l.pids[N] has type pid_t.  We assume that the long tempid is
representable as a pid.  This is a valid assumption due to our earlier
checks.  9 is smaller than 2**31, so it fits in pid_t because pid_t
happens to be int32_t.  This would not necessarily be true of the limit
were dynamic.  The kern.pid_max sysctl actuall

Re: svn commit: r244112 - head/sys/kern

2012-12-14 Thread Julian Elischer

On 12/13/12 6:37 AM, Andriy Gapon wrote:

on 12/12/2012 19:06 Adrian Chadd said the following:

kassert()s are already optional. Ie, you can choose to not compile them in.

So the __dead2() code path bit for doing KASSERT() -> kassert_panic()
at compile time isn't a problem.

The problem is where you do panic() -> kassert_panic() (eg in the
Witness code) which is what Alfred discovered shortly after doing up
his initial patch.

Anything which is a KASSERT() can and should be treated as a run-time
warning just as much as a run-time "crash here so I can figure out
what broke." Having the warning in a production box is going to be
helpful for developers.

I have a quite different view on purpose and costs of KASSERTs.
Specifically referring to r243980 I do not think that "non-fatal asserts" should
really exist (or do exist).  I wish all this muddying of KASSERT meaning
would get reverted.

These quite sensitive changes were rushed in, IMO.


having worked in a place where asserts were by default NON fatal, I can
say that it is a REALLY BAD IDEA.
it never did anything but cost us time and effort..

one reason it was there was the lack of a debugger in Linux at the time
but all ports had top pay the price... (new linux does have one at last)







On 12 December 2012 07:46, John Baldwin  wrote:

On Tuesday, December 11, 2012 2:08:14 am Alfred Perlstein wrote:

Author: alfred
Date: Tue Dec 11 07:08:14 2012
New Revision: 244112
URL: http://svnweb.freebsd.org/changeset/base/244112

Log:
   Cleanup more of the kassert_panic.

   fix compile warnings on !amd64 and NULL derefs that would happen
   if kassert_panic() would return.

This is one reason why having kassert not panic is such a bad idea.  There are
tons of places where the compiler knows that panic() is __dead2, and there is
no cleanup code to handle what happens when an invariant is violated.  This is
not safe to run in the field unless your customers do not care about their
data.  If you are interested in doing regression tests, I am using a very
different approach for some locking regression tests I am working on in p4
that allow you to use a wrapper around setjmp/longjmp to "catch" panics
somewhat like exception handling in C++/Java (though much cruder).  However,
evne that is only intended for testing, not for production cases where
production data is at stake.

--
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"