svn commit: r228196 - in head: share/mk tools/build/options

[Max Khon]

Author: fjoe
Date: Fri Dec  2 09:09:54 2011
New Revision: 228196
URL: http://svn.freebsd.org/changeset/base/228196

Log:
  Revert to conservative defaults. WITH_PROFILE is a default now.
  
  NO_PROFILE overrides in Makefile.inc1 and bsd.own.m are left intact
  so that the reversal to the WITHOUT_PROFILE will be easier in future.

Added:
  head/tools/build/options/WITHOUT_PROFILE
 - copied, changed from r228148, head/tools/build/options/WITH_PROFILE
Deleted:
  head/tools/build/options/WITH_PROFILE
Modified:
  head/share/mk/bsd.own.mk

Modified: head/share/mk/bsd.own.mk
==
--- head/share/mk/bsd.own.mkFri Dec  2 02:19:03 2011(r228195)
+++ head/share/mk/bsd.own.mkFri Dec  2 09:09:54 2011(r228196)
@@ -383,6 +383,7 @@ __DEFAULT_YES_OPTIONS = \
 PMC \
 PORTSNAP \
 PPP \
+PROFILE \
 QUOTAS \
 RCMDS \
 RCS \
@@ -418,8 +419,7 @@ __DEFAULT_NO_OPTIONS = \
 ICONV \
 IDEA \
 LIBCPLUSPLUS \
-OFED \
-PROFILE
+OFED
 
 #
 # Default behaviour of some options depends on the architecture.  Unfortunately

Copied and modified: head/tools/build/options/WITHOUT_PROFILE (from r228148, 
head/tools/build/options/WITH_PROFILE)
==
--- head/tools/build/options/WITH_PROFILE   Tue Nov 29 21:28:48 2011
(r228148, copy source)
+++ head/tools/build/options/WITHOUT_PROFILEFri Dec  2 09:09:54 2011
(r228196)
@@ -1,2 +1,2 @@
 .\" $FreeBSD$
-Set to build profiled libraries.
+Set to avoid compiling profiled libraries.
___
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: r228197 - head/share/man/man5

[Max Khon]

Author: fjoe
Date: Fri Dec  2 09:28:19 2011
New Revision: 228197
URL: http://svn.freebsd.org/changeset/base/228197

Log:
  Regen.

Modified:
  head/share/man/man5/src.conf.5

Modified: head/share/man/man5/src.conf.5
==
--- head/share/man/man5/src.conf.5  Fri Dec  2 09:09:54 2011
(r228196)
+++ head/share/man/man5/src.conf.5  Fri Dec  2 09:28:19 2011
(r228197)
@@ -1,7 +1,7 @@
 .\" DO NOT EDIT-- this file is automatically generated.
 .\" from FreeBSD: head/tools/build/options/makeman 221733 2011-05-10 13:01:11Z 
ru
 .\" $FreeBSD$
-.Dd December 1, 2011
+.Dd December 2, 2011
 .Dt SRC.CONF 5
 .Os
 .Sh NAME
@@ -303,7 +303,7 @@ When set, the following options are also
 is set explicitly)
 .El
 .It Va WITH_CTF
-.\" $FreeBSD$
+.\" from FreeBSD: head/tools/build/options/WITH_CTF 228159 2011-11-30 
18:22:44Z fjoe
 Set to compile with CTF (Compact C Type Format) data.
 CTF data encapsulates a reduced form of debugging information
 similar to DWARF and the venerable stabs and is required for DTrace.
@@ -820,9 +820,9 @@ and related files.
 Set to not build
 .Xr ppp 8
 and related programs.
-.It Va WITH_PROFILE
+.It Va WITHOUT_PROFILE
 .\" from FreeBSD: head/tools/build/options/WITH_PROFILE 228143 2011-11-29 
19:46:17Z fjoe
-Set to build profiled libraries.
+Set to avoid compiling profiled libraries.
 .It Va WITHOUT_QUOTAS
 .\" from FreeBSD: head/tools/build/options/WITHOUT_QUOTAS 183242 2008-09-21 
22:02:26Z sam
 Set to not build
___
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: r228196 - in head: share/mk tools/build/options

[David O'Brien]

On Fri, Dec 02, 2011 at 09:09:54AM +, Max Khon wrote:
> Log:
>   Revert to conservative defaults. WITH_PROFILE is a default now.

Thank you.

>   NO_PROFILE overrides in Makefile.inc1 and bsd.own.m are left intact
>   so that the reversal to the WITHOUT_PROFILE will be easier in future.

As I mentioned, it may make sense to not build profiled libs by default
in -stable.  But if you do that, please add logic to 'make release' to
build profiled libs -- putting them in either base.txz or proflibs.txz
(or however you find tasteful).

-- 
-- David  (obr...@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: r228153 - head/usr.sbin/config

[David O'Brien]

On Wed, Nov 30, 2011 at 01:33:09PM +, Max Khon wrote:
> Log:
>   Generate ${NORMAL_CTFCONVERT} invocation without '@' modifier:
>   - ${NORMAL_CC} is also invoked without '@'
>   - Userland CTF support was changed previously to echo ctfconvert
>   invocations too

Thank you for your work on cleaning up the CTF part of the build.
Enabling WITH_CTF has been too different (and conflicting) from our
other knobs.
I also had local patches to not hide ctfconvert so one could easily see
when they didn't have WITH_CTF enabled in the right magical incantations.

Will you be MFC'ing the CTF build work?  Please. :-)

-- 
-- David  (obr...@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: r228192 - head/usr.sbin/bsdinstall/scripts

[Joel Dahl]

On 02-12-2011  0:38, Ken Smith wrote:
> Author: kensmith
> Date: Fri Dec  2 00:38:47 2011
> New Revision: 228192
> URL: http://svn.freebsd.org/changeset/base/228192
> 
> Log:
>   Add a screen that asks if the user would like to enable crash dumps,
>   giving them a very brief description of the trade-offs.  Whether the
>   user opts in or out add an entry to what will become /etc/rc.conf
>   explaining what dumpdev is and how to turn on/off crash dumps.  The folks
>   who handle interacting with users submitting PRs have asked for this.

Hmm. Two things I'd like to bring up:

* Not specifically aimed at this commit, but my recommendation
  would be that we keep bsdinstall as simple as possible: installing FreeBSD
  should require a minimum amount of keystrokes. I realise this is just one
  more screen, but I hope we don't turn bsdinstall into a configuration
  utility where you can disable/enable just about anything in rc.conf.

* Mentioning future system crashes during installation feels awkward. Is that
  really what we want? I understand the problem and how this helps us with
  debugging, but this is like saying to users that what they are installing
  is unstable and that it'll eventuelly crash and die. I know we discussed
  ways of making crash dumps smarter in order to not fill up /var, which in
  turn would allow us to always have it on. Maybe that is the right path?

-- 
Joel
___
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: r228198 - head/sys/dev/hwpmc

[Fabien Thomas]

Author: fabient
Date: Fri Dec  2 10:02:49 2011
New Revision: 228198
URL: http://svn.freebsd.org/changeset/base/228198

Log:
  Update Westmere uncore event exception list.
  
  Submitted by: Davide Italiano 
  MFC after:1 week

Modified:
  head/sys/dev/hwpmc/hwpmc_core.c

Modified: head/sys/dev/hwpmc/hwpmc_core.c
==
--- head/sys/dev/hwpmc/hwpmc_core.c Fri Dec  2 09:28:19 2011
(r228197)
+++ head/sys/dev/hwpmc/hwpmc_core.c Fri Dec  2 10:02:49 2011
(r228198)
@@ -1479,6 +1479,10 @@ iap_event_westmere_ok_on_counter(enum pm
/*
 * Events valid only on counter 0.
 */
+   case PMC_EV_IAP_EVENT_60H_01H:
+   case PMC_EV_IAP_EVENT_60H_02H:
+   case PMC_EV_IAP_EVENT_60H_04H:
+   case PMC_EV_IAP_EVENT_60H_08H:  
case PMC_EV_IAP_EVENT_B3H_01H:
case PMC_EV_IAP_EVENT_B3H_02H:
case PMC_EV_IAP_EVENT_B3H_04H:
@@ -1488,6 +1492,10 @@ iap_event_westmere_ok_on_counter(enum pm
/*
 * Events valid only on counter 0, 1.
 */
+   case PMC_EV_IAP_EVENT_4CH_01H:
+   case PMC_EV_IAP_EVENT_4EH_01H:
+   case PMC_EV_IAP_EVENT_4EH_02H:
+   case PMC_EV_IAP_EVENT_4EH_04H:
case PMC_EV_IAP_EVENT_51H_01H:
case PMC_EV_IAP_EVENT_51H_02H:
case PMC_EV_IAP_EVENT_51H_04H:
___
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: r228199 - head/lib/libc/locale

[David E. O'Brien]

Author: obrien
Date: Fri Dec  2 11:55:09 2011
New Revision: 228199
URL: http://svn.freebsd.org/changeset/base/228199

Log:
  Split sentences at period boundaries.

Modified:
  head/lib/libc/locale/duplocale.3
  head/lib/libc/locale/freelocale.3
  head/lib/libc/locale/localeconv.3
  head/lib/libc/locale/newlocale.3
  head/lib/libc/locale/querylocale.3
  head/lib/libc/locale/uselocale.3
  head/lib/libc/locale/xlocale.3

Modified: head/lib/libc/locale/duplocale.3
==
--- head/lib/libc/locale/duplocale.3Fri Dec  2 10:02:49 2011
(r228198)
+++ head/lib/libc/locale/duplocale.3Fri Dec  2 11:55:09 2011
(r228199)
@@ -36,25 +36,26 @@
 .Sh LIBRARY
 .Lb libc
 .Sh SYNOPSIS
-.In xlocale.h 
-.Ft locale_t 
+.In xlocale.h
+.Ft locale_t
 .Fn duplocale "locale_t locale"
 .Sh DESCRIPTION
 Duplicates an existing
 .Fa locale_t
-returning a new 
+returning a new
 .Fa locale_t
 that refers to the same locale values but has independent internal state.
 Various functions, such as
 .Xr mblen 3
-require presistent state.  These functions formerly used static variables and
-calls to them from multiple threads had undefined behavior.  They now use
-fields in the
+require presistent state.
+These functions formerly used static variables and calls to them from multiple
+threads had undefined behavior.
+They now use fields in the
 .Fa locale_t
 associated with the current thread by
 .Xr uselocale 3 .
 These calls are therefore only thread safe on threads with a unique per-thread
-locale.  
+locale.
 .Pt
 The locale returned by this call must be freed with
 .Xr freelocale 3 .
@@ -63,8 +64,9 @@ Ideally,
 .Xr uselocale 3
 should make a copy of the
 .Fa locale_t
-implicitly to ensure thread safety, and a copy of the global locale should be
-installed lazily on each thread.  The FreeBSD implementation does not do this,
+implicitly to ensure thread safety,
+and a copy of the global locale should be installed lazily on each thread.
+The FreeBSD implementation does not do this,
 for compatibility with Darwin.
 .Sh SEE ALSO
 .Xr freelocale 3 ,

Modified: head/lib/libc/locale/freelocale.3
==
--- head/lib/libc/locale/freelocale.3   Fri Dec  2 10:02:49 2011
(r228198)
+++ head/lib/libc/locale/freelocale.3   Fri Dec  2 11:55:09 2011
(r228199)
@@ -33,22 +33,22 @@
 .Nm freelocale
 .Nd Frees a locale created with
 .Xr duplocale 3
-or 
+or
 .Xr newlocale 3 .
 .Sh LIBRARY
 .Lb libc
 .Sh SYNOPSIS
-.In xlocale.h 
+.In xlocale.h
 .Ft int
 .Fn freelocale "locale_t locale"
 .Sh DESCRIPTION
 Frees a
 .Fa locale_t .
-This relinquishes any resources held exclusively by this locale.  Note that
-locales share reference-counted components, so a call to this function is not
-guaranteed to free all of the components.
+This relinquishes any resources held exclusively by this locale.
+Note that locales share reference-counted components,
+so a call to this function is not guaranteed to free all of the components.
 .Sh RETURN VALUES
-Returns 0 on success or -1 on error.  
+Returns 0 on success or -1 on error.
 .Sh SEE ALSO
 .Xr duplocale 3 ,
 .Xr localeconv 3 ,

Modified: head/lib/libc/locale/localeconv.3
==
--- head/lib/libc/locale/localeconv.3   Fri Dec  2 10:02:49 2011
(r228198)
+++ head/lib/libc/locale/localeconv.3   Fri Dec  2 11:55:09 2011
(r228199)
@@ -203,7 +203,7 @@ result similarly denotes an unavailable 
 The
 .Fn localeconv_l
 function takes an explicit locale parameter.  For more information, see
-.Xr xlocale 3 .  
+.Xr xlocale 3 .
 .Sh RETURN VALUES
 The
 .Fn localeconv
@@ -212,9 +212,10 @@ which may be altered by later calls to
 .Xr setlocale 3
 or
 .Fn localeconv .
-The return value for 
+The return value for
 .Fn localeconv_l
-is stored with the locale.  It will remain valid until a subsequent call to
+is stored with the locale.
+It will remain valid until a subsequent call to
 .Xr freelocale 3 .
 If a thread-local locale is in effect then the return value from
 .Fn localeconv

Modified: head/lib/libc/locale/newlocale.3
==
--- head/lib/libc/locale/newlocale.3Fri Dec  2 10:02:49 2011
(r228198)
+++ head/lib/libc/locale/newlocale.3Fri Dec  2 11:55:09 2011
(r228199)
@@ -31,40 +31,44 @@
 .Os
 .Sh NAME
 .Nm newlocale
-.Nd Creates a new locale 
+.Nd Creates a new locale
 .Sh LIBRARY
 .Lb libc
 .Sh SYNOPSIS
 .In xlocale
-.Ft 
+.Ft
 .Fn newlocale "int mask" "const char * locale" "locale_t base"
 .Sh DESCRIPTION
-Creates a new locale, inheriting some properties from an existing locale.  The
-.Fa mask 
+Creates a new locale, inheriting some properties from an existing locale.
+The
+.Fa mask
 defines the components that the new locale will have set to the locale with the
-name specified in the

svn commit: r228200 - in head: share/man/man4 sys/dev/ahci sys/dev/ata/chipsets

[Alexander Motin]

Author: mav
Date: Fri Dec  2 12:52:33 2011
New Revision: 228200
URL: http://svn.freebsd.org/changeset/base/228200

Log:
  Add hw.ahci.force tunable to control whether AHCI drivers should attach
  to known AHCI-capable chips (AMD/NVIDIA), configured for legacy emulation.
  
  Enabled by default to get additional performance and functionality of AHCI
  when it can't be enabled by BIOS. Can be disabled to honor BIOS settings if
  needed for some reason.
  
  MFC after:1 month

Modified:
  head/share/man/man4/ahci.4
  head/sys/dev/ahci/ahci.c
  head/sys/dev/ata/chipsets/ata-ati.c
  head/sys/dev/ata/chipsets/ata-nvidia.c

Modified: head/share/man/man4/ahci.4
==
--- head/share/man/man4/ahci.4  Fri Dec  2 11:55:09 2011(r228199)
+++ head/share/man/man4/ahci.4  Fri Dec  2 12:52:33 2011(r228200)
@@ -24,7 +24,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd May 17, 2011
+.Dd December 2, 2011
 .Dt AHCI 4
 .Os
 .Sh NAME
@@ -96,6 +96,9 @@ A manual bus reset is needed on device h
 .It Va hint.ahcich. Ns Ar X Ns Va .sata_rev
 setting to nonzero value limits maximum SATA revision (speed).
 Values 1, 2 and 3 are respectively 1.5, 3 and 6Gbps.
+.It Va hw.ahci.force
+setting to nonzero value forces driver attach to some known AHCI-capable
+chips even if they are configured for legacy IDE emulation. Default is 1.
 .El
 .Sh DESCRIPTION
 This driver provides the

Modified: head/sys/dev/ahci/ahci.c
==
--- head/sys/dev/ahci/ahci.cFri Dec  2 11:55:09 2011(r228199)
+++ head/sys/dev/ahci/ahci.cFri Dec  2 12:52:33 2011(r228200)
@@ -291,6 +291,9 @@ static struct {
 #define RECOVERY_REQUEST_SENSE 2
 #define recovery_slot  spriv_field1
 
+static int force_ahci = 1;
+TUNABLE_INT("hw.ahci.force", &force_ahci);
+
 static int
 ahci_probe(device_t dev)
 {
@@ -308,7 +311,8 @@ ahci_probe(device_t dev)
for (i = 0; ahci_ids[i].id != 0; i++) {
if (ahci_ids[i].id == devid &&
ahci_ids[i].rev <= revid &&
-   (valid || !(ahci_ids[i].quirks & AHCI_Q_NOFORCE))) {
+   (valid || (force_ahci == 1 &&
+!(ahci_ids[i].quirks & AHCI_Q_NOFORCE {
/* Do not attach JMicrons with single PCI function. */
if (pci_get_vendor(dev) == 0x197b &&
(pci_read_config(dev, 0xdf, 1) & 0x40) == 0)

Modified: head/sys/dev/ata/chipsets/ata-ati.c
==
--- head/sys/dev/ata/chipsets/ata-ati.c Fri Dec  2 11:55:09 2011
(r228199)
+++ head/sys/dev/ata/chipsets/ata-ati.c Fri Dec  2 12:52:33 2011
(r228200)
@@ -53,6 +53,7 @@ __FBSDID("$FreeBSD$");
 
 /* local prototypes */
 static int ata_ati_chipinit(device_t dev);
+static int ata_ati_dumb_ch_attach(device_t dev);
 static int ata_ati_ixp700_ch_attach(device_t dev);
 static int ata_ati_setmode(device_t dev, int target, int mode);
 
@@ -63,6 +64,8 @@ static int ata_ati_setmode(device_t dev,
 #define SII_MEMIO   1
 #define SII_BUG 0x04
 
+static int force_ahci = 1;
+TUNABLE_INT("hw.ahci.force", &force_ahci);
 
 /*
  * ATI chipset support functions
@@ -111,7 +114,10 @@ ata_ati_probe(device_t dev)
ctlr->chipinit = ata_sii_chipinit;
break;
 case ATI_AHCI:
-   ctlr->chipinit = ata_ahci_chipinit;
+   if (force_ahci == 1 || pci_get_subclass(dev) != PCIS_STORAGE_IDE)
+   ctlr->chipinit = ata_ahci_chipinit;
+   else
+   ctlr->chipinit = ata_ati_chipinit;
break;
 }
 return (BUS_PROBE_DEFAULT);
@@ -127,6 +133,11 @@ ata_ati_chipinit(device_t dev)
 if (ata_setup_interrupt(dev, ata_generic_intr))
return ENXIO;
 
+if (ctlr->chip->cfg1 == ATI_AHCI) {
+   ctlr->ch_attach = ata_ati_dumb_ch_attach;
+   ctlr->setmode = ata_sata_setmode;
+   return (0);
+}
 switch (ctlr->chip->chipid) {
 case ATA_ATI_IXP600:
/* IXP600 only has 1 PATA channel */
@@ -165,6 +176,17 @@ ata_ati_chipinit(device_t dev)
 }
 
 static int
+ata_ati_dumb_ch_attach(device_t dev)
+{
+   struct ata_channel *ch = device_get_softc(dev);
+
+   if (ata_pci_ch_attach(dev))
+   return ENXIO;
+   ch->flags |= ATA_SATA;
+   return (0);
+}
+
+static int
 ata_ati_ixp700_ch_attach(device_t dev)
 {
struct ata_pci_controller *ctlr = 
device_get_softc(device_get_parent(dev));

Modified: head/sys/dev/ata/chipsets/ata-nvidia.c
==
--- head/sys/dev/ata/chipsets/ata-nvidia.c  Fri Dec  2 11:55:09 2011
(r228199)
+++ head/sys/dev/ata/chipsets/ata-nvidia.c  Fri Dec  2 12:52:33 2011
(r228200)
@@ -65,6 +65,8 @@ static int ata_nvidia_setmode(device_t d
 #define NVAHCI  0x04
 #define NVNOFORCE   0x0

Re: svn commit: r228192 - head/usr.sbin/bsdinstall/scripts

[Erik Cederstrand]

Den 02/12/2011 kl. 10.44 skrev Joel Dahl:

> On 02-12-2011  0:38, Ken Smith wrote:
>> Author: kensmith
>> Date: Fri Dec  2 00:38:47 2011
>> New Revision: 228192
>> URL: http://svn.freebsd.org/changeset/base/228192
>> 
>> Log:
>>  Add a screen that asks if the user would like to enable crash dumps,
>>  giving them a very brief description of the trade-offs.  Whether the
>>  user opts in or out add an entry to what will become /etc/rc.conf
>>  explaining what dumpdev is and how to turn on/off crash dumps.  The folks
>>  who handle interacting with users submitting PRs have asked for this.
> 
> Hmm. Two things I'd like to bring up:
> 
> * Not specifically aimed at this commit, but my recommendation
>  would be that we keep bsdinstall as simple as possible: installing FreeBSD
>  should require a minimum amount of keystrokes. I realise this is just one
>  more screen, but I hope we don't turn bsdinstall into a configuration
>  utility where you can disable/enable just about anything in rc.conf.
> 
> * Mentioning future system crashes during installation feels awkward. Is that
>  really what we want? I understand the problem and how this helps us with
>  debugging, but this is like saying to users that what they are installing
>  is unstable and that it'll eventuelly crash and die. I know we discussed
>  ways of making crash dumps smarter in order to not fill up /var, which in
>  turn would allow us to always have it on. Maybe that is the right path?

Maybe do away with the dialog, and then unconditionally enable textdump in 
rc.conf, adding the magic so textdump_enable="YES" sets up textdump with sane 
defaults.

Thanks,
Erik___
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: r228192 - head/usr.sbin/bsdinstall/scripts

[Ken Smith]

On Fri, 2011-12-02 at 14:18 +0100, Erik Cederstrand wrote:
> Den 02/12/2011 kl. 10.44 skrev Joel Dahl:
> 
> > On 02-12-2011  0:38, Ken Smith wrote:
> >> Author: kensmith
> >> Date: Fri Dec  2 00:38:47 2011
> >> New Revision: 228192
> >> URL: http://svn.freebsd.org/changeset/base/228192
> >> 
> >> Log:
> >>  Add a screen that asks if the user would like to enable crash dumps,
> >>  giving them a very brief description of the trade-offs.  Whether the
> >>  user opts in or out add an entry to what will become /etc/rc.conf
> >>  explaining what dumpdev is and how to turn on/off crash dumps.  The folks
> >>  who handle interacting with users submitting PRs have asked for this.
> > 
> > Hmm. Two things I'd like to bring up:
> > 
> > * Not specifically aimed at this commit, but my recommendation
> >  would be that we keep bsdinstall as simple as possible: installing FreeBSD
> >  should require a minimum amount of keystrokes. I realise this is just one
> >  more screen, but I hope we don't turn bsdinstall into a configuration
> >  utility where you can disable/enable just about anything in rc.conf.
> > 
> > * Mentioning future system crashes during installation feels awkward. Is 
> > that
> >  really what we want? I understand the problem and how this helps us with
> >  debugging, but this is like saying to users that what they are installing
> >  is unstable and that it'll eventuelly crash and die. I know we discussed
> >  ways of making crash dumps smarter in order to not fill up /var, which in
> >  turn would allow us to always have it on. Maybe that is the right path?
> 
> Maybe do away with the dialog, and then unconditionally enable textdump in 
> rc.conf, adding the magic so textdump_enable="YES" sets up textdump with sane 
> defaults.
> 
> Thanks,
> Erik
> 

This just implements what we seemed to settle on during the latest round
of several discussions we've had about this issue.  It comes up each
I've turned off dumpdev as part of converting head into a stable branch,
which is a step that got started way before I started being the one to
do it.  I'll be happy to see what I just added go away but the
pre-condition for that is infrastructure that prevents crash dumps
(of any form, including textdumps) from filling up /var if the user
turns out to be clueless about them and doesn't realize they should
pay attention to them.  At least as of the last time we discussed it
this knob I just added is a compromise between people who worry about
crash dumps going unattended and filling /var versus the people who
want us to be able to collect debugging information from the users
who submit PRs *and* provide an *extremely* easy way for said users
to turn it on/off.

I normally hate dragging what others do into the discussions but I
know of several Linux's that offer to enable kdump in their installer
so we're not alone in that regard.

-- 
Ken Smith
- From there to here, from here to  |   kensm...@freebsd.org
  there, funny things are everywhere.   |
  - Theodor Geisel  |


signature.asc
Description: This is a digitally signed message part

Re: svn commit: r228196 - in head: share/mk tools/build/options

[Bruce Evans]

On Fri, 2 Dec 2011, Max Khon wrote:


Log:
 Revert to conservative defaults. WITH_PROFILE is a default now.

 NO_PROFILE overrides in Makefile.inc1 and bsd.own.m are left intact
 so that the reversal to the WITHOUT_PROFILE will be easier in future.


Thanks.  Not building profiled libraries might have been best in 1993
when makeworld took hours (4-10?).  When I optimized makeworld in 1998,
it still took more than 1 hour but I removing profiling from it was
intentionally left out (except for bootstrapping).  It would have saved
only a few minutes.  Now, a whole makeworld should take a few minutes
and the few extra seconds for profiling libraries doesn't matter.

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: r227961 - head/sbin/camcontrol

[Andre Albsmeier]

On Fri, 25-Nov-2011 at 05:03:37 +0100, Ed Maste wrote:
> Author: emaste
> Date: Fri Nov 25 04:03:37 2011
> New Revision: 227961
> URL: http://svn.freebsd.org/changeset/base/227961
> 
> Log:
>   Add firmware update support for SCSI devices.
> 
> ...
> 
> Added: head/sbin/camcontrol/fwdownload.c
> ==
> --- /dev/null   00:00:00 1970   (empty, because file is newly added)
> +++ head/sbin/camcontrol/fwdownload.c   Fri Nov 25 04:03:37 2011
> (r227961)
> @@ -0,0 +1,380 @@
> +/*-
> + * Copyright (c) 2011 Sandvine Incorporated. All rights reserved.
> + * Copyright (c) 2002-2011 Andre Albsmeier 
> + * All rights reserved.
> + *

We should not forget Marc Frajola who sent me his ddys.c for
updating IBM DDYS drives ages ago. He allowed me to use his
code but wanted to be credited -- I think we should continue
to honour his wish. His file contained the following note:

 * TeraSolutions IBM drive firmware update utility
 *
 * (C) Copyright 2000, TeraSolutions, Inc.  All Rights Reserved.
 *   This software contains proprietary trade secrets of
 *   TeraSolutions, Inc.  Redistribution without prior written
 *   consent is prohibited.
 *
 * By Marc Frajola, 08/15/2000

and in fwprog.c I had him mentioned this way:

 * ...
 *  Many thanks goes to Marc Frajola  from
 *  TeraSolutions for the initial idea and his programme for upgrading
 *  the firmware of I*M DDYS drives ...
 * ...

I think you guys find something that is appropriate...

Thanks,

-Andre
___
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: r228192 - head/usr.sbin/bsdinstall/scripts

[John Baldwin]

On 12/2/11 4:44 AM, Joel Dahl wrote:

On 02-12-2011  0:38, Ken Smith wrote:

Author: kensmith
Date: Fri Dec  2 00:38:47 2011
New Revision: 228192
URL: http://svn.freebsd.org/changeset/base/228192

Log:
   Add a screen that asks if the user would like to enable crash dumps,
   giving them a very brief description of the trade-offs.  Whether the
   user opts in or out add an entry to what will become /etc/rc.conf
   explaining what dumpdev is and how to turn on/off crash dumps.  The folks
   who handle interacting with users submitting PRs have asked for this.


Hmm. Two things I'd like to bring up:

* Not specifically aimed at this commit, but my recommendation
   would be that we keep bsdinstall as simple as possible: installing FreeBSD
   should require a minimum amount of keystrokes. I realise this is just one
   more screen, but I hope we don't turn bsdinstall into a configuration
   utility where you can disable/enable just about anything in rc.conf.

* Mentioning future system crashes during installation feels awkward. Is that
   really what we want? I understand the problem and how this helps us with
   debugging, but this is like saying to users that what they are installing
   is unstable and that it'll eventuelly crash and die. I know we discussed
   ways of making crash dumps smarter in order to not fill up /var, which in
   turn would allow us to always have it on. Maybe that is the right path?


All non-trivial software has bugs and eventually crashes.  I don't 
expect this to be a surprise to someone installing a UNIX-like operating 
system.  Note that this isn't the PC-BSD installer, but the FreeBSD 
installer.  Also, there have been long discussions about this and ample 
time for other patches to be developed but they haven't.  If you want 
this changed, implement the alternate solution.  Until then this is 
better than not having it at all.


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

svn commit: r228201 - in head/sys: arm/mv dev/fdt dev/ofw dev/uart powerpc/booke powerpc/powermac sparc64/sparc64

[Jayachandran C.]

Author: jchandra
Date: Fri Dec  2 15:24:39 2011
New Revision: 228201
URL: http://svn.freebsd.org/changeset/base/228201

Log:
  Fix OF_finddevice error return value in case of FDT.
  
  According to the open firmware standard, finddevice call has to return
  a phandle with value of -1 in case of error.
  
  This commit is to:
  - Fix the FDT implementation of this interface (ofw_fdt_finddevice) to
return (phandle_t)-1 in case of error, instead of 0 as it does now.
  - Fix up the callers of OF_finddevice() to compare the return value with
-1 instead of 0 to check for errors.
  - Since phandle_t is unsigned, the return value of OF_finddevice should
be checked with '== -1' rather than '<= 0' or '> 0', fix up these cases
as well.
  
  Reported by:  nwhitehorn
  
  Reviewed by:  raj
  Approved by:  raj, nwhitehorn

Modified:
  head/sys/arm/mv/common.c
  head/sys/arm/mv/mv_machdep.c
  head/sys/dev/fdt/fdt_common.c
  head/sys/dev/fdt/fdt_powerpc.c
  head/sys/dev/fdt/fdtbus.c
  head/sys/dev/ofw/ofw_fdt.c
  head/sys/dev/ofw/openfirm.c
  head/sys/dev/uart/uart_bus_fdt.c
  head/sys/powerpc/booke/platform_bare.c
  head/sys/powerpc/powermac/platform_powermac.c
  head/sys/sparc64/sparc64/ofw_machdep.c
  head/sys/sparc64/sparc64/vm_machdep.c

Modified: head/sys/arm/mv/common.c
==
--- head/sys/arm/mv/common.cFri Dec  2 12:52:33 2011(r228200)
+++ head/sys/arm/mv/common.cFri Dec  2 15:24:39 2011(r228201)
@@ -1693,7 +1693,7 @@ fdt_get_ranges(const char *nodename, voi
int len, tuple_size, tuples_count;
 
node = OF_finddevice(nodename);
-   if (node <= 0)
+   if (node == -1)
return (EINVAL);
 
if ((fdt_addrsize_cells(node, &addr_cells, &size_cells)) != 0)
@@ -1762,11 +1762,11 @@ win_cpu_from_dt(void)
/*
 * Retrieve CESA SRAM data.
 */
-   if ((node = OF_finddevice("sram")) != 0)
+   if ((node = OF_finddevice("sram")) != -1)
if (fdt_is_compatible(node, "mrvl,cesa-sram"))
goto moveon;
 
-   if ((node = OF_finddevice("/")) == 0)
+   if ((node = OF_finddevice("/")) != -1)
return (ENXIO);
 
if ((node = fdt_find_compatible(node, "mrvl,cesa-sram", 0)) == 0)
@@ -1796,7 +1796,7 @@ fdt_win_setup(void)
int err, i;
 
node = OF_finddevice("/");
-   if (node == 0)
+   if (node == -1)
panic("fdt_win_setup: no root node");
 
node = fdt_find_compatible(node, "simple-bus", 1);

Modified: head/sys/arm/mv/mv_machdep.c
==
--- head/sys/arm/mv/mv_machdep.cFri Dec  2 12:52:33 2011
(r228200)
+++ head/sys/arm/mv/mv_machdep.cFri Dec  2 15:24:39 2011
(r228201)
@@ -617,13 +617,13 @@ platform_mpp_init(void)
/*
 * Try to access the MPP node directly i.e. through /aliases/mpp.
 */
-   if ((node = OF_finddevice("mpp")) != 0)
+   if ((node = OF_finddevice("mpp")) != -1)
if (fdt_is_compatible(node, "mrvl,mpp"))
goto moveon;
/*
 * Find the node the long way.
 */
-   if ((node = OF_finddevice("/")) == 0)
+   if ((node = OF_finddevice("/")) == -1)
return (ENXIO);
 
if ((node = fdt_find_compatible(node, "simple-bus", 0)) == 0)
@@ -752,7 +752,7 @@ platform_devmap_init(void)
/*
 * PCI range(s).
 */
-   if ((root = OF_finddevice("/")) == 0)
+   if ((root = OF_finddevice("/")) == -1)
return (ENXIO);
 
for (child = OF_child(root); child != 0; child = OF_peer(child))
@@ -779,7 +779,7 @@ platform_devmap_init(void)
/*
 * CESA SRAM range.
 */
-   if ((child = OF_finddevice("sram")) != 0)
+   if ((child = OF_finddevice("sram")) != -1)
if (fdt_is_compatible(child, "mrvl,cesa-sram"))
goto moveon;
 

Modified: head/sys/dev/fdt/fdt_common.c
==
--- head/sys/dev/fdt/fdt_common.c   Fri Dec  2 12:52:33 2011
(r228200)
+++ head/sys/dev/fdt/fdt_common.c   Fri Dec  2 15:24:39 2011
(r228201)
@@ -74,13 +74,13 @@ fdt_immr_addr(vm_offset_t immr_va)
/*
 * Try to access the SOC node directly i.e. through /aliases/.
 */
-   if ((node = OF_finddevice("soc")) != 0)
+   if ((node = OF_finddevice("soc")) != -1)
if (fdt_is_compatible_strict(node, "simple-bus"))
goto moveon;
/*
 * Find the node the long way.
 */
-   if ((node = OF_finddevice("/")) == 0)
+   if ((node = OF_finddevice("/")) == -1)
return (ENXIO);
 
if ((node = fdt_find_compatible(node, "simple-bus", 1)) == 0)
@@ -576,7 +576,7 @@ fdt_get_mem_regi

Re: svn commit: r228192 - head/usr.sbin/bsdinstall/scripts

[Nathan Whitehorn]

On 12/02/11 09:17, John Baldwin wrote:

On 12/2/11 4:44 AM, Joel Dahl wrote:

On 02-12-2011  0:38, Ken Smith wrote:

Author: kensmith
Date: Fri Dec  2 00:38:47 2011
New Revision: 228192
URL: http://svn.freebsd.org/changeset/base/228192

Log:
   Add a screen that asks if the user would like to enable crash dumps,
   giving them a very brief description of the trade-offs.  Whether the
   user opts in or out add an entry to what will become /etc/rc.conf
   explaining what dumpdev is and how to turn on/off crash dumps.  
The folks
   who handle interacting with users submitting PRs have asked for 
this.


Hmm. Two things I'd like to bring up:

* Not specifically aimed at this commit, but my recommendation
   would be that we keep bsdinstall as simple as possible: installing 
FreeBSD
   should require a minimum amount of keystrokes. I realise this is 
just one

   more screen, but I hope we don't turn bsdinstall into a configuration
   utility where you can disable/enable just about anything in rc.conf.

* Mentioning future system crashes during installation feels awkward. 
Is that
   really what we want? I understand the problem and how this helps 
us with
   debugging, but this is like saying to users that what they are 
installing
   is unstable and that it'll eventuelly crash and die. I know we 
discussed
   ways of making crash dumps smarter in order to not fill up /var, 
which in
   turn would allow us to always have it on. Maybe that is the right 
path?


All non-trivial software has bugs and eventually crashes.  I don't 
expect this to be a surprise to someone installing a UNIX-like 
operating system.  Note that this isn't the PC-BSD installer, but the 
FreeBSD installer.  Also, there have been long discussions about this 
and ample time for other patches to be developed but they haven't.  If 
you want this changed, implement the alternate solution.  Until then 
this is better than not having it at all.




This was also my conclusion when Ken asked me to review this patch. I 
also don't want the installer to become bloated and made the same 
original objection, but (a) this patch existed and (b) the long 
discussion meant that Ken felt it was a particular important decision 
that deserved its own screen. In particular, the explanation of why you 
might or might not want it was larger than could fit in a line in the 
regular services screen and the matter may require some thought.

-Nathan
___
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: r228202 - head/lib/libc/string

[Eitan Adler]

Author: eadler (ports committer)
Date: Fri Dec  2 15:41:09 2011
New Revision: 228202
URL: http://svn.freebsd.org/changeset/base/228202

Log:
  Revert r227812 and r227808 per discussion
  
  Reviewed by:  many
  Approved by:  des

Modified:
  head/lib/libc/string/strcasecmp.c
  head/lib/libc/string/strcmp.c
  head/lib/libc/string/strcoll.c
  head/lib/libc/string/strncmp.c

Modified: head/lib/libc/string/strcasecmp.c
==
--- head/lib/libc/string/strcasecmp.c   Fri Dec  2 15:24:39 2011
(r228201)
+++ head/lib/libc/string/strcasecmp.c   Fri Dec  2 15:41:09 2011
(r228202)
@@ -48,9 +48,6 @@ strcasecmp_l(const char *s1, const char 
const u_char
*us1 = (const u_char *)s1,
*us2 = (const u_char *)s2;
-   if (s1 == s2)
-   return (0);
-
FIX_LOCALE(locale);
 
while (tolower_l(*us1, locale) == tolower_l(*us2++, locale))
@@ -68,22 +65,18 @@ int
 strncasecmp_l(const char *s1, const char *s2, size_t n, locale_t locale)
 {
FIX_LOCALE(locale);
-
-   const u_char
-   *us1 = (const u_char *)s1,
-   *us2 = (const u_char *)s2;
-   
-   /* use a bitwise or to avoid an additional branch instruction */
-   if ((s1 == s2) | (n == 0))
-   return (0);
-
-
-   do {
-   if (tolower_l(*us1, locale) != tolower_l(*us2++, locale))
-   return (tolower_l(*us1, locale) - tolower_l(*--us2, 
locale));
-   if (*us1++ == '\0')
-   break;
-   } while (--n != 0);
+   if (n != 0) {
+   const u_char
+   *us1 = (const u_char *)s1,
+   *us2 = (const u_char *)s2;
+
+   do {
+   if (tolower_l(*us1, locale) != tolower_l(*us2++, 
locale))
+   return (tolower_l(*us1, locale) - 
tolower_l(*--us2, locale));
+   if (*us1++ == '\0')
+   break;
+   } while (--n != 0);
+   }
return (0);
 }
 

Modified: head/lib/libc/string/strcmp.c
==
--- head/lib/libc/string/strcmp.c   Fri Dec  2 15:24:39 2011
(r228201)
+++ head/lib/libc/string/strcmp.c   Fri Dec  2 15:41:09 2011
(r228202)
@@ -44,9 +44,6 @@ __FBSDID("$FreeBSD$");
 int
 strcmp(const char *s1, const char *s2)
 {
-   if (s1 == s2)
-   return (0);
-
while (*s1 == *s2++)
if (*s1++ == '\0')
return (0);

Modified: head/lib/libc/string/strcoll.c
==
--- head/lib/libc/string/strcoll.c  Fri Dec  2 15:24:39 2011
(r228201)
+++ head/lib/libc/string/strcoll.c  Fri Dec  2 15:41:09 2011
(r228202)
@@ -40,7 +40,7 @@ __FBSDID("$FreeBSD$");
 #include 
 
 int
-strcoll_l(const char *s1, const char *s2, locale_t locale)
+strcoll_l(const char *s, const char *s2, locale_t locale)
 {
int len, len2, prim, prim2, sec, sec2, ret, ret2;
const char *t, *t2;
@@ -50,16 +50,16 @@ strcoll_l(const char *s1, const char *s2
(struct xlocale_collate*)locale->components[XLC_COLLATE];
 
if (table->__collate_load_error)
-   return strcmp(s1, s2);
+   return strcmp(s, s2);
 
len = len2 = 1;
ret = ret2 = 0;
if (table->__collate_substitute_nontrivial) {
-   t = tt = __collate_substitute(table, s1);
+   t = tt = __collate_substitute(table, s);
t2 = tt2 = __collate_substitute(table, s2);
} else {
tt = tt2 = NULL;
-   t = s1;
+   t = s;
t2 = s2;
}
while(*t && *t2) {
@@ -95,8 +95,8 @@ strcoll_l(const char *s1, const char *s2
 }
 
 int
-strcoll(const char *s1, const char *s2)
+strcoll(const char *s, const char *s2)
 {
-   return strcoll_l(s1, s2, __get_locale());
+   return strcoll_l(s, s2, __get_locale());
 }
 

Modified: head/lib/libc/string/strncmp.c
==
--- head/lib/libc/string/strncmp.c  Fri Dec  2 15:24:39 2011
(r228201)
+++ head/lib/libc/string/strncmp.c  Fri Dec  2 15:41:09 2011
(r228202)
@@ -39,10 +39,8 @@ int
 strncmp(const char *s1, const char *s2, size_t n)
 {
 
-   /* use a bitwise or to avoid an additional branch instruction */
-   if ((n == 0) | (s1 == s2))
+   if (n == 0)
return (0);
-
do {
if (*s1 != *s2++)
return (*(const unsigned char *)s1 -
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To uns

svn commit: r228203 - head/sbin/camcontrol

[Ed Maste]

Author: emaste
Date: Fri Dec  2 15:47:05 2011
New Revision: 228203
URL: http://svn.freebsd.org/changeset/base/228203

Log:
  Restore comment with credit to Marc Frajola.
  
  fwdownload.c is derived from Andre Albsmeier's fwdownload.c, which was
  written from scratch but was inspired by Marc's original idea and code.

Modified:
  head/sbin/camcontrol/fwdownload.c

Modified: head/sbin/camcontrol/fwdownload.c
==
--- head/sbin/camcontrol/fwdownload.c   Fri Dec  2 15:41:09 2011
(r228202)
+++ head/sbin/camcontrol/fwdownload.c   Fri Dec  2 15:47:05 2011
(r228203)
@@ -26,6 +26,15 @@
  */
 
 /*
+ * This software is derived from Andre Albsmeier's fwprog.c which contained
+ * the following note:
+ *
+ * Many thanks goes to Marc Frajola  from
+ * TeraSolutions for the initial idea and his programme for upgrading
+ * the firmware of I*M DDYS drives.
+ */
+
+/*
  * BEWARE:
  *
  * The fact that you see your favorite vendor listed below does not
___
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: r228204 - head/sys/geom

[Alexander Motin]

Author: mav
Date: Fri Dec  2 17:09:48 2011
New Revision: 228204
URL: http://svn.freebsd.org/changeset/base/228204

Log:
  Close race between geom destruction on g_vfs_close() when softc destroyed
  and g_vfs_orphan() call that tries to access softc, intruced at r227015.
  
  PR:   kern/162997

Modified:
  head/sys/geom/geom_vfs.c

Modified: head/sys/geom/geom_vfs.c
==
--- head/sys/geom/geom_vfs.cFri Dec  2 15:47:05 2011(r228203)
+++ head/sys/geom/geom_vfs.cFri Dec  2 17:09:48 2011(r228204)
@@ -169,8 +169,10 @@ g_vfs_orphan(struct g_consumer *cp)
g_topology_assert();
 
gp = cp->geom;
-   sc = gp->softc;
g_trace(G_T_TOPOLOGY, "g_vfs_orphan(%p(%s))", cp, gp->name);
+   sc = gp->softc;
+   if (sc == NULL)
+   return;
mtx_lock(&sc->sc_mtx);
sc->sc_orphaned = 1;
destroy = (sc->sc_active == 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: r228192 - head/usr.sbin/bsdinstall/scripts

[Bruce Cran]

On 02/12/2011 09:44, Joel Dahl wrote:

* Not specifically aimed at this commit, but my recommendation
   would be that we keep bsdinstall as simple as possible: installing FreeBSD
   should require a minimum amount of keystrokes. I realise this is just one
   more screen, but I hope we don't turn bsdinstall into a configuration
   utility where you can disable/enable just about anything in rc.conf.


This is an argument I see again and again. FreeBSD is a technical OS, 
where I would expect users would appreciate the chance to configure 
their systems without having to look up man pages for every detail (e.g. 
console settings).

For people who want their OS to be as simple as possible there's PC-BSD.

--
Bruce Cran
___
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: r228205 - head/share/man/man4

[Pyun YongHyeon]

Author: yongari
Date: Fri Dec  2 19:01:26 2011
New Revision: 228205
URL: http://svn.freebsd.org/changeset/base/228205

Log:
  Add more controllers that support jumbo frame.

Modified:
  head/share/man/man4/bge.4

Modified: head/share/man/man4/bge.4
==
--- head/share/man/man4/bge.4   Fri Dec  2 17:09:48 2011(r228204)
+++ head/share/man/man4/bge.4   Fri Dec  2 19:01:26 2011(r228205)
@@ -31,7 +31,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd November 23, 2010
+.Dd December 2, 2011
 .Dt BGE 4
 .Os
 .Sh NAME
@@ -82,9 +82,9 @@ copper gigabit transceivers,
 which support autonegotiation of 10, 100 and 1000Mbps modes in
 full or half duplex.
 .Pp
-The BCM5700, BCM5701, BCM5702, BCM5703, BCM5704 and BCM5717 also support
-jumbo frames, which can be configured
-via the interface MTU setting.
+The BCM5700, BCM5701, BCM5702, BCM5703, BCM5704, BCM5714, BCM5717, BCM5719,
+BCM5720, BCM5780 and BCM57765 also support jumbo frames, which can be
+configured via the interface MTU setting.
 Selecting an MTU larger than 1500 bytes with the
 .Xr ifconfig 8
 utility configures the adapter to receive and transmit jumbo frames.
___
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: r228206 - in head/cddl/contrib/opensolaris/cmd: zfs zpool

[Martin Matuska]

Author: mm
Date: Fri Dec  2 19:56:46 2011
New Revision: 228206
URL: http://svn.freebsd.org/changeset/base/228206

Log:
  Remove unnecesary "Ns" macros and add missing command example to zpool(8).
  
  Reported by:  Nobuyuki Koganemaru 
  MFC after:3 days

Modified:
  head/cddl/contrib/opensolaris/cmd/zfs/zfs.8
  head/cddl/contrib/opensolaris/cmd/zpool/zpool.8

Modified: head/cddl/contrib/opensolaris/cmd/zfs/zfs.8
==
--- head/cddl/contrib/opensolaris/cmd/zfs/zfs.8 Fri Dec  2 19:01:26 2011
(r228205)
+++ head/cddl/contrib/opensolaris/cmd/zfs/zfs.8 Fri Dec  2 19:56:46 2011
(r228206)
@@ -2198,7 +2198,7 @@ Generate a stream package that sends all
 .Ar snapshot . For example,
 .Ic -I @a fs@d
 is similar to
-.Ic -i @a fs@b; -i @b fs@c; -i @c fs@d Ns .
+.Ic -i @a fs@b; -i @b fs@c; -i @c fs@d .
 The incremental source snapshot may be specified as with the
 .Fl i
 option.
@@ -2414,7 +2414,7 @@ may be specified as a comma-separated li
 .Tn ZFS
 subcommand and property names. See the property list below. Property set names,
 which begin with an at sign
-.Pq Sy @ Ns ,
+.Pq Sy @ ,
 may be specified. See the
 .Fl s
 form below for details.
@@ -2536,7 +2536,7 @@ commands for the specified file system a
 dynamically, so changes to a set are immediately reflected. Permission sets
 follow the same naming restrictions as ZFS file systems, but the name must
 begin with an "at sign"
-.Pq Sy @ Ns ,
+.Pq Sy @ ,
 and can be no more than 64 characters long.
 .It Xo
 .Nm

Modified: head/cddl/contrib/opensolaris/cmd/zpool/zpool.8
==
--- head/cddl/contrib/opensolaris/cmd/zpool/zpool.8 Fri Dec  2 19:01:26 
2011(r228205)
+++ head/cddl/contrib/opensolaris/cmd/zpool/zpool.8 Fri Dec  2 19:56:46 
2011(r228206)
@@ -195,7 +195,7 @@ are supported:
 .Bl -tag
 .It Sy disk
 A block device, typically located under
-.Pa /dev Ns .
+.Pa /dev .
 .Tn ZFS
 can use individual slices or partitions, though the recommended mode of
 operation is to use whole disks. A disk can be specified by a full path to the
@@ -221,13 +221,14 @@ bytes and can withstand
 .Pq Em N-1
 devices failing before data integrity is compromised.
 .It Sy raidz
-.No ( or Sy raidz1 raidz2 raidz3 Ns ).
+(or
+.Sy raidz1 raidz2 raidz3 ) .
 A variation on
 .Sy RAID-5
 that allows for better distribution of parity and eliminates the
-.Qq Sy RAID-5 No write hole
-(in which data and parity become inconsistent after a power loss). Data and
-parity is striped across all disks within a
+.Qq Sy RAID-5
+write hole (in which data and parity become inconsistent after a power loss).
+Data and parity is striped across all disks within a
 .No raidz
 group.
 .Pp
@@ -251,7 +252,7 @@ type specifies a triple-parity
 group. The
 .Sy raidz No vdev
 type is an alias for
-.Sy raidz1 Ns .
+.Sy raidz1 .
 .Pp
 A
 .No raidz
@@ -322,6 +323,9 @@ are used to distinguish where a group en
 following creates two root
 .No vdev Ns s,
 each a mirror of two disks:
+.Bd -literal -offset 2n
+.Li # Ic zpool create mypool mirror da0 da1 mirror da2 da3
+.Ed
 .Ss Device Failure and Recovery
 .Tn ZFS
 supports a rich set of mechanisms for handling device failure and data
@@ -421,7 +425,7 @@ hardware-dependent and might not be supp
 .Ss Hot Spares
 .Tn ZFS
 allows devices to be associated with pools as
-.Qq hot spares Ns .
+.Qq hot spares .
 These devices are not actively used in the pool, but when an active device
 fails, it is automatically replaced by a hot spare. To create a pool with hot
 spares, specify a 
@@ -537,12 +541,13 @@ Number of blocks within the pool that ar
 A unique identifier for the pool.
 .It Sy health
 The current health of the pool. Health can be
-.Qq Sy ONLINE Ns ,
-.Qq Sy DEGRADED Ns ,
-.Qq Sy FAULTED Ns ,
-.Qq Sy OFFLINE Ns ,
-.Qq Sy REMOVED Ns , or
-.Qq Sy UNAVAIL Ns .
+.Qq Sy ONLINE ,
+.Qq Sy DEGRADED ,
+.Qq Sy FAULTED ,
+.Qq Sy OFFLINE ,
+.Qq Sy REMOVED ,
+or
+.Qq Sy UNAVAIL .
 .It Sy size
 Total size of the storage pool.
 .It Sy used
@@ -577,7 +582,7 @@ is not a persistent property. It is vali
 Setting
 .Sy altroot 
 defaults to using
-.Cm cachefile=none Ns ,
+.Cm cachefile=none ,
 though this may be overridden using an explicit setting.
 .El
 .Pp
@@ -585,7 +590,7 @@ The following property can only be set a
 .Bl -tag -width 2n
 .It Sy readonly Ns = Ns Cm on No | Cm off
 If set to
-.Cm on Ns ,
+.Cm on ,
 pool will be imported in read-only mode with the following restrictions:
 .Bl -bullet -offset 2n
 .It
@@ -606,7 +611,7 @@ command:
 .Bl -tag -width 2n
 .It Sy autoexpand Ns = Ns Cm on No | Cm off
 Controls automatic pool expansion when the underlying LUN is grown. If set to
-.Qq Cm on Ns ,
+.Qq Cm on ,
 the pool will be resized according to the size of the expanded
 device. If the device is part of a mirror or
 .No raidz
@@ -614,20 +619,20 @@ then all devices within that
 .No mirror/ Ns No raidz
 group must be 

svn commit: r228207 - head/sys/kern

[John Baldwin]

Author: jhb
Date: Fri Dec  2 19:59:46 2011
New Revision: 228207
URL: http://svn.freebsd.org/changeset/base/228207

Log:
  When changing the user priority of a thread, change the real priority
  in addition to the user priority for threads whose current real priority
  is equal to the previous user priority or if the new priority is a
  real-time priority.  This allows priority changes of other threads to
  have an immediate effect.
  
  MFC after:2 weeks

Modified:
  head/sys/kern/kern_resource.c

Modified: head/sys/kern/kern_resource.c
==
--- head/sys/kern/kern_resource.c   Fri Dec  2 19:56:46 2011
(r228206)
+++ head/sys/kern/kern_resource.c   Fri Dec  2 19:59:46 2011
(r228207)
@@ -488,8 +488,9 @@ rtp_to_pri(struct rtprio *rtp, struct th
sched_class(td, rtp->type); /* XXX fix */
oldpri = td->td_user_pri;
sched_user_prio(td, newpri);
-   if (curthread == td)
-   sched_prio(curthread, td->td_user_pri); /* XXX dubious */
+   if (td->td_user_pri != oldpri && (td == curthread ||
+   td->td_priority == oldpri || td->td_user_pri >= PRI_MAX_REALTIME))
+   sched_prio(td, td->td_user_pri);
if (TD_ON_UPILOCK(td) && oldpri != newpri) {
critical_enter();
thread_unlock(td);
___
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: r228207 - head/sys/kern

[John Baldwin]

On 12/2/11 2:59 PM, John Baldwin wrote:

Author: jhb
Date: Fri Dec  2 19:59:46 2011
New Revision: 228207
URL: http://svn.freebsd.org/changeset/base/228207

Log:
   When changing the user priority of a thread, change the real priority
   in addition to the user priority for threads whose current real priority
   is equal to the previous user priority or if the new priority is a
   real-time priority.  This allows priority changes of other threads to
   have an immediate effect.


An example that this fixes is suppose you have a CPU bound thread 
running on an dedicated CPU with rtprio and now want to run another 
quick thread on that CPU.  The quick thread doesn't have rtprio and 
won't get a chance to run (by design).  You want to fix this though so
you lower the CPU bound thread to a lower rtprio and set the quick 
thread to rtprio 0.  Without this fix the quick thread still won't get 
to run as the rtprio 0 will never take effect until the thread returns 
from userland.  With this change it will now take effect immediately so 
the quick thread will preempt the CPU bound one.


You can test this by scheduling two CPU bound rtprio threads on a single 
CPU (I used jot ${BIGNUM} >/dev/null) and frob the two processes 
relative realtime priorities.  Before whatever jot started running first 
would never be preempted by the second jot, even if the second jot was 
assigned a higher RT priority.  Now the the jot process with the highest 
RT priority always runs.


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

svn commit: r228209 - head/sys/kern

[Marius Strobl]

Author: marius
Date: Fri Dec  2 21:19:14 2011
New Revision: 228209
URL: http://svn.freebsd.org/changeset/base/228209

Log:
  - In device_probe_child(9) check the return value of device_set_driver(9)
when actually setting a driver as especially ENOMEM is fatal in these
cases.
  - Annotate other calls to device_set_devclass(9) and device_set_driver(9)
without the return value being checked and that are okay to fail.
  
  Reviewed by:  yongari (slightly earlier version)

Modified:
  head/sys/kern/subr_bus.c

Modified: head/sys/kern/subr_bus.c
==
--- head/sys/kern/subr_bus.cFri Dec  2 20:28:45 2011(r228208)
+++ head/sys/kern/subr_bus.cFri Dec  2 21:19:14 2011(r228209)
@@ -1129,7 +1129,7 @@ devclass_driver_deleted(devclass_t buscl
dev->parent->devclass == busclass) {
if ((error = device_detach(dev)) != 0)
return (error);
-   device_set_driver(dev, NULL);
+   (void)device_set_driver(dev, NULL);
BUS_PROBE_NOMATCH(dev->parent, dev);
devnomatch(dev);
dev->flags |= DF_DONENOMATCH;
@@ -2007,19 +2007,22 @@ device_probe_child(device_t dev, device_
for (dl = first_matching_driver(dc, child);
 dl;
 dl = next_matching_driver(dc, child, dl)) {
-
/* If this driver's pass is too high, then ignore it. */
if (dl->pass > bus_current_pass)
continue;
 
PDEBUG(("Trying %s", DRIVERNAME(dl->driver)));
-   device_set_driver(child, dl->driver);
+   result = device_set_driver(child, dl->driver);
+   if (result == ENOMEM)
+   return (result);
+   else if (result != 0)
+   continue;
if (!hasclass) {
if (device_set_devclass(child, 
dl->driver->name)) {
printf("driver bug: Unable to set 
devclass (devname: %s)\n",
(child ? device_get_name(child) :
"no device"));
-   device_set_driver(child, NULL);
+   (void)device_set_driver(child, NULL);
continue;
}
}
@@ -2033,7 +2036,7 @@ device_probe_child(device_t dev, device_
/* Reset flags and devclass before the next probe. */
child->devflags = 0;
if (!hasclass)
-   device_set_devclass(child, NULL);
+   (void)device_set_devclass(child, NULL);
 
/*
 * If the driver returns SUCCESS, there can be
@@ -2050,7 +2053,7 @@ device_probe_child(device_t dev, device_
 * certainly doesn't match.
 */
if (result > 0) {
-   device_set_driver(child, NULL);
+   (void)device_set_driver(child, NULL);
continue;
}
 
@@ -2113,7 +2116,9 @@ device_probe_child(device_t dev, device_
if (result != 0)
return (result);
}
-   device_set_driver(child, best->driver);
+   result = device_set_driver(child, best->driver);
+   if (result != 0)
+   return (result);
resource_int_value(best->driver->name, child->unit,
"flags", &child->devflags);
 
@@ -2722,8 +2727,8 @@ device_attach(device_t dev)
dev->driver->name, dev->unit, error);
/* Unset the class; set in device_probe_child */
if (dev->devclass == NULL)
-   device_set_devclass(dev, NULL);
-   device_set_driver(dev, NULL);
+   (void)device_set_devclass(dev, NULL);
+   (void)device_set_driver(dev, NULL);
device_sysctl_fini(dev);
dev->state = DS_NOTPRESENT;
return (error);
@@ -2776,7 +2781,7 @@ device_detach(device_t dev)
devclass_delete_device(dev->devclass, dev);
 
dev->state = DS_NOTPRESENT;
-   device_set_driver(dev, NULL);
+   (void)device_set_driver(dev, NULL);
device_set_desc(dev, NULL);
device_sysctl_fini(dev);
 
@@ -4613,7 +4618,6 @@ print_driver(driver_t *dri

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

[John Baldwin]

On 12/2/11 4:19 PM, Marius Strobl wrote:

Author: marius
Date: Fri Dec  2 21:19:14 2011
New Revision: 228209
URL: http://svn.freebsd.org/changeset/base/228209

Log:
   - In device_probe_child(9) check the return value of device_set_driver(9)
 when actually setting a driver as especially ENOMEM is fatal in these
 cases.
   - Annotate other calls to device_set_devclass(9) and device_set_driver(9)
 without the return value being checked and that are okay to fail.

   Reviewed by: yongari (slightly earlier version)


Hmm, I've always thought that new-bus overused M_NOWAIT.  There are a 
few places where M_NOWAIT might be appropriate (e.g. from pccbb's 
interrupt routine when a cardbus card is inserted), but the boot time 
probe can certainly use M_WAITOK instead.  Even the pccbb case could use 
M_WAITOK if it were simply deferred to a taskqueue (which it probably 
should be anyway, probing and attaching a driver to a new card is far 
too expensive to do directly from an interrupt handler).


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

[Marius Strobl]

On Fri, Dec 02, 2011 at 04:24:37PM -0500, John Baldwin wrote:
> On 12/2/11 4:19 PM, Marius Strobl wrote:
> >Author: marius
> >Date: Fri Dec  2 21:19:14 2011
> >New Revision: 228209
> >URL: http://svn.freebsd.org/changeset/base/228209
> >
> >Log:
> >   - In device_probe_child(9) check the return value of 
> >   device_set_driver(9)
> > when actually setting a driver as especially ENOMEM is fatal in these
> > cases.
> >   - Annotate other calls to device_set_devclass(9) and 
> >   device_set_driver(9)
> > without the return value being checked and that are okay to fail.
> >
> >   Reviewed by:  yongari (slightly earlier version)
> 
> Hmm, I've always thought that new-bus overused M_NOWAIT.  There are a 
> few places where M_NOWAIT might be appropriate (e.g. from pccbb's 
> interrupt routine when a cardbus card is inserted), but the boot time 
> probe can certainly use M_WAITOK instead.  Even the pccbb case could use 
> M_WAITOK if it were simply deferred to a taskqueue (which it probably 
> should be anyway, probing and attaching a driver to a new card is far 
> too expensive to do directly from an interrupt handler).
> 

In general the use of M_NOWAIT vs. M_WAITOK in bus methods is also very
inconsistent across device drivers. We should decide how to handle the
standard case, document it and fix things up accordingly ...
I agree that M_WAITOK should be used in most of these cases.

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"

svn commit: r228211 - head/sys/kern

[Marius Strobl]

Author: marius
Date: Fri Dec  2 22:03:27 2011
New Revision: 228211
URL: http://svn.freebsd.org/changeset/base/228211

Log:
  It doesn't make much sense to check whether child is NULL after already
  having dereferenced it. We either should generally check the device_t's
  supplied to bus functions before using them (which we seem to virtually
  never do) or just assume that they are not NULL.
  While at it make this code fit 78 columns.
  
  Found with:   Coverity Prevent(tm)
  CID:  4230

Modified:
  head/sys/kern/subr_bus.c

Modified: head/sys/kern/subr_bus.c
==
--- head/sys/kern/subr_bus.cFri Dec  2 21:45:46 2011(r228210)
+++ head/sys/kern/subr_bus.cFri Dec  2 22:03:27 2011(r228211)
@@ -2018,10 +2018,11 @@ device_probe_child(device_t dev, device_
else if (result != 0)
continue;
if (!hasclass) {
-   if (device_set_devclass(child, 
dl->driver->name)) {
-   printf("driver bug: Unable to set 
devclass (devname: %s)\n",
-   (child ? device_get_name(child) :
-   "no device"));
+   if (device_set_devclass(child,
+   dl->driver->name) != 0) {
+   printf("driver bug: Unable to set "
+   "devclass (devname: %s)\n",
+   device_get_name(child));
(void)device_set_driver(child, NULL);
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"

Re: svn commit: r227778 - head/sys/net

[Jung-uk Kim]

On Thursday 01 December 2011 11:43 pm, Lawrence Stewart wrote:
> On 12/02/11 03:43, Jung-uk Kim wrote:
> > On Thursday 01 December 2011 10:11 am, Lawrence Stewart wrote:
> >> On 11/30/11 05:09, Jung-uk Kim wrote:
> >>> On Tuesday 29 November 2011 11:13 am, Lawrence Stewart wrote:
>  On 11/30/11 00:05, Lawrence Stewart wrote:
> > On 11/28/11 14:59, Benjamin Kaduk wrote:
> >> On Wed, 23 Nov 2011, Lawrence Stewart wrote:
> >>> On 11/23/11 17:42, Julien Ridoux wrote:
> 
>  [snip]
> 
>  What is your favourite option?
> >>>
> >>> FreeBSD parlance is to ask what colour you would like to
> >>> paint the bikeshed ;)
> >>>
> >>> As I've never experienced the pain John refers to, I'll
> >>> defer to the wisdom of others on whether the proposed patch
> >>> will create pain down the road. I think it's ok, but if
> >>> consensus is 8bytes per packet isn't going to break the
> >>> bank, I guess we just go for it - but I guess I am cautious
> >>> about this route as we can push a lot of packets per second
> >>> through the stack.
> >>
> >> Since other people seem to be keeping quiet, I'll add that
> >> I'm in favor of just always adding the 8 bytes per packet.
> >
> > Julien and I discussed this at length today, and agree that
> > for head, we'll add the new bh_ffcounter member to the BPF
> > header unconditionally.
> >
> > Thanks to you and John for the input.
> >
> > I'm going to revert r227778 in order to start form a clean
> > slate, and add two separate patches. One will reintegrate
> > FFCLOCK support with BPF without breaking the ABI. A follow
> > up patch will bump the ffclock version and add the
> > bh_ffcounter to the bpf header (after the timestamp member).
> > Then a final patch will bump __FreeBSD_version and add a note
> > to UPDATING about recompiling to get kernel/world in sync,
> > which should seal the deal.
> 
>  Here's the first of the patches:
> 
>  http://people.freebsd.org/~lstewart/patches/misc/ffclock_bpf_i
> nt act abi_10.x.r228130.patch
> >>>
> >>> I only glanced at it but it looks very close to what I wanted
> >>> to suggest.
> >>
> >> Final candidate patch is at:
> >>
> >> http://people.freebsd.org/~lstewart/patches/misc/ffclock_bpf_int
> >>act abi_10.x.r228180.patch
> >>
> >> Assuming it passes the "make tinderbox" build I'm currently
> >> running and no further input is received from interested
> >> parties, I plan to commit it in ~10 hours.
> >>
> >> Changes since the r228130 patch I sent previously:
> >>
> >> - The new flags in bpf.h are added unconditionally so that they
> >> can always be referenced at compile time and a decision made at
> >> runtime as to whether a flag will be set or not. Using one of
> >> the new flags when the kernel doesn't have FFCLOCK compiled in
> >> results in the flag being ignored. An app should check for the
> >> existence of the "ffclock" kernel feature or the "kern.sysclock"
> >> sysctl tree before attempting to use the flags.
> >>
> >> - This patch will hopefully be MFCed at some point, so I added a
> >> CTASSERT to bpf.c to ensure that the ABI of structs bpf_hdr32,
> >> bpf_hdr and bpf_xhdr remains intact when FFCLOCK is enabled and
> >> the union of a ffcounter and struct timeval32/timeval/bpf_ts is
> >> switched in.
> >>
> >> - bpf_gettime() more comprehensively covers all the possible
> >> cases of flag combination and does sensible things for each case
> >> (even though some cases are rather silly).
> >>
> >> - The snippet of code at the beginning of catchpacket() that was
> >> manipulating the struct bintime derived from bpf_gettime() was
> >> gross and has been removed in favour of selecting the right
> >> {get}bin{up}time() function call in bpf_gettime().
> >
> > I did that to reduce branching.  Since you are introducing more
> > branches, it warrants a function pointer now.

There's another reason, BTW.  If mbufs are tagged with timestamps 
(where only monotonic timestamps are allowed), they must be converted 
within the bpf.c.  I forgot all the details. :-(

> I see, thanks for the explanation. Could you elaborate a bit more
> about how you would implement the function pointer idea? I'm also
> curious in the !FFCLOCK case just how much overhead having the
> 2-layer nested if/else adds. I'm not an very optimisation savvy
> person, but I'm wondering if it's actually worth micro-optimising
> this code.
>
> My initial thoughts about your function pointer idea lead to adding
> a function pointer in the bpf_d and setting it to the appropriate
> function to get the timestamp from at bpf_d creation or ioctl time.
> Whilst I like this idea, I can't see how it would work given that
> the various functions involved in time/ffcounter stamp generation
> all have different signatures.
>
> We could have multiple variants of bpf_gettime() which each call
> the appropriate underlying functions 

Re: svn commit: r227778 - head/sys/net

[Jung-uk Kim]

On Friday 02 December 2011 07:27 pm, Jung-uk Kim wrote:
> On Thursday 01 December 2011 11:43 pm, Lawrence Stewart wrote:
> > On 12/02/11 03:43, Jung-uk Kim wrote:
> > > On Thursday 01 December 2011 10:11 am, Lawrence Stewart wrote:
> > >> On 11/30/11 05:09, Jung-uk Kim wrote:
> > >>> On Tuesday 29 November 2011 11:13 am, Lawrence Stewart wrote:
> >  On 11/30/11 00:05, Lawrence Stewart wrote:
> > > On 11/28/11 14:59, Benjamin Kaduk wrote:
> > >> On Wed, 23 Nov 2011, Lawrence Stewart wrote:
> > >>> On 11/23/11 17:42, Julien Ridoux wrote:
> > 
> >  [snip]
> > 
> >  What is your favourite option?
> > >>>
> > >>> FreeBSD parlance is to ask what colour you would like to
> > >>> paint the bikeshed ;)
> > >>>
> > >>> As I've never experienced the pain John refers to, I'll
> > >>> defer to the wisdom of others on whether the proposed
> > >>> patch will create pain down the road. I think it's ok,
> > >>> but if consensus is 8bytes per packet isn't going to
> > >>> break the bank, I guess we just go for it - but I guess I
> > >>> am cautious about this route as we can push a lot of
> > >>> packets per second through the stack.
> > >>
> > >> Since other people seem to be keeping quiet, I'll add that
> > >> I'm in favor of just always adding the 8 bytes per packet.
> > >
> > > Julien and I discussed this at length today, and agree that
> > > for head, we'll add the new bh_ffcounter member to the BPF
> > > header unconditionally.
> > >
> > > Thanks to you and John for the input.
> > >
> > > I'm going to revert r227778 in order to start form a clean
> > > slate, and add two separate patches. One will reintegrate
> > > FFCLOCK support with BPF without breaking the ABI. A follow
> > > up patch will bump the ffclock version and add the
> > > bh_ffcounter to the bpf header (after the timestamp
> > > member). Then a final patch will bump __FreeBSD_version and
> > > add a note to UPDATING about recompiling to get
> > > kernel/world in sync, which should seal the deal.
> > 
> >  Here's the first of the patches:
> > 
> >  http://people.freebsd.org/~lstewart/patches/misc/ffclock_bpf
> > _i nt act abi_10.x.r228130.patch
> > >>>
> > >>> I only glanced at it but it looks very close to what I wanted
> > >>> to suggest.
> > >>
> > >> Final candidate patch is at:
> > >>
> > >> http://people.freebsd.org/~lstewart/patches/misc/ffclock_bpf_i
> > >>nt act abi_10.x.r228180.patch
> > >>
> > >> Assuming it passes the "make tinderbox" build I'm currently
> > >> running and no further input is received from interested
> > >> parties, I plan to commit it in ~10 hours.
> > >>
> > >> Changes since the r228130 patch I sent previously:
> > >>
> > >> - The new flags in bpf.h are added unconditionally so that
> > >> they can always be referenced at compile time and a decision
> > >> made at runtime as to whether a flag will be set or not. Using
> > >> one of the new flags when the kernel doesn't have FFCLOCK
> > >> compiled in results in the flag being ignored. An app should
> > >> check for the existence of the "ffclock" kernel feature or the
> > >> "kern.sysclock" sysctl tree before attempting to use the
> > >> flags.
> > >>
> > >> - This patch will hopefully be MFCed at some point, so I added
> > >> a CTASSERT to bpf.c to ensure that the ABI of structs
> > >> bpf_hdr32, bpf_hdr and bpf_xhdr remains intact when FFCLOCK is
> > >> enabled and the union of a ffcounter and struct
> > >> timeval32/timeval/bpf_ts is switched in.
> > >>
> > >> - bpf_gettime() more comprehensively covers all the possible
> > >> cases of flag combination and does sensible things for each
> > >> case (even though some cases are rather silly).
> > >>
> > >> - The snippet of code at the beginning of catchpacket() that
> > >> was manipulating the struct bintime derived from bpf_gettime()
> > >> was gross and has been removed in favour of selecting the
> > >> right {get}bin{up}time() function call in bpf_gettime().
> > >
> > > I did that to reduce branching.  Since you are introducing more
> > > branches, it warrants a function pointer now.
>
> There's another reason, BTW.  If mbufs are tagged with timestamps
> (where only monotonic timestamps are allowed), they must be
> converted within the bpf.c.  I forgot all the details. :-(
>
> > I see, thanks for the explanation. Could you elaborate a bit more
> > about how you would implement the function pointer idea? I'm also
> > curious in the !FFCLOCK case just how much overhead having the
> > 2-layer nested if/else adds. I'm not an very optimisation savvy
> > person, but I'm wondering if it's actually worth micro-optimising
> > this code.
> >
> > My initial thoughts about your function pointer idea lead to
> > adding a function pointer in the bpf_d and setting it to the
> > appropriate function to get the timestamp from at bpf_d creation
> > or ioctl time. Whilst I

Re: svn commit: r227778 - head/sys/net

[Benjamin Kaduk]

On Fri, 2 Dec 2011, Jung-uk Kim wrote:


On Friday 02 December 2011 07:27 pm, Jung-uk Kim wrote:


Please see my patch:

http://people.freebsd.org/~jkim/bpf_ffclock.diff


I booted up the kernel and found it just crashes because of stupid
typos.  Now a new patch is uploaded in place.  Sorry.

Anyway, I see no regression nor ABI breakage. :-)


I don't have any objections to it; what do Lawrence and Julien think?

-Ben
___
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: r228217 - in head/sys/fs: nfs nfsclient

[Rick Macklem]

Author: rmacklem
Date: Sat Dec  3 02:27:26 2011
New Revision: 228217
URL: http://svn.freebsd.org/changeset/base/228217

Log:
  Post r223774, the NFSv4 client no longer has multiple instances
  of the same lock_owner4 string. As such, the handling of cleanup
  of lock_owners could be simplified. This simplification permitted
  the client to do a ReleaseLockOwner operation when the process that
  the lock_owner4 string represents, has exited. This permits the
  server to release any storage related to the lock_owner4 string
  before the associated open is closed. Without this change, it
  is possible to exhaust a server's storage when a long running
  process opens a file and then many child processes do locking
  on the file, because the open doesn't get closed. A similar patch
  was applied to the Linux NFSv4 client recently so that it wouldn't
  exhaust a server's storage.
  
  Reviewed by:  zack
  MFC after:2 weeks

Modified:
  head/sys/fs/nfs/nfsclstate.h
  head/sys/fs/nfsclient/nfs_clstate.c

Modified: head/sys/fs/nfs/nfsclstate.h
==
--- head/sys/fs/nfs/nfsclstate.hFri Dec  2 23:21:59 2011
(r228216)
+++ head/sys/fs/nfs/nfsclstate.hSat Dec  3 02:27:26 2011
(r228217)
@@ -34,6 +34,7 @@
  */
 LIST_HEAD(nfsclopenhead, nfsclopen);
 LIST_HEAD(nfscllockownerhead, nfscllockowner);
+SLIST_HEAD(nfscllockownerfhhead, nfscllockownerfh);
 LIST_HEAD(nfscllockhead, nfscllock);
 LIST_HEAD(nfsclhead, nfsclclient);
 LIST_HEAD(nfsclownerhead, nfsclowner);
@@ -149,8 +150,8 @@ struct nfscllockowner {
struct nfsclopen*nfsl_open;
NFSPROC_T   *nfsl_inprog;
nfsv4stateid_t  nfsl_stateid;
+   int nfsl_lockflags;
u_int32_t   nfsl_seqid;
-   u_int32_t   nfsl_defunct;
struct nfsv4locknfsl_rwlock;
u_int8_tnfsl_owner[NFSV4CL_LOCKNAMELEN];
u_int8_tnfsl_openowner[NFSV4CL_LOCKNAMELEN];
@@ -166,6 +167,14 @@ struct nfscllock {
short   nfslo_type;
 };
 
+/* This structure is used to collect a list of lockowners to free up. */
+struct nfscllockownerfh {
+   SLIST_ENTRY(nfscllockownerfh)   nfslfh_list;
+   struct nfscllockownerhead   nfslfh_lock;
+   int nfslfh_len;
+   uint8_t nfslfh_fh[NFSX_V4FHMAX];
+};
+
 /*
  * Macro for incrementing the seqid#.
  */

Modified: head/sys/fs/nfsclient/nfs_clstate.c
==
--- head/sys/fs/nfsclient/nfs_clstate.c Fri Dec  2 23:21:59 2011
(r228216)
+++ head/sys/fs/nfsclient/nfs_clstate.c Sat Dec  3 02:27:26 2011
(r228217)
@@ -143,6 +143,8 @@ static void nfscl_freeopenowner(struct n
 static void nfscl_cleandeleg(struct nfscldeleg *);
 static int nfscl_trydelegreturn(struct nfscldeleg *, struct ucred *,
 struct nfsmount *, NFSPROC_T *);
+static void nfscl_emptylockowner(struct nfscllockowner *,
+struct nfscllockownerfhhead *);
 
 static short nfscberr_null[] = {
0,
@@ -1030,7 +1032,7 @@ nfscl_getbytelock(vnode_t vp, u_int64_t 
NFSBCOPY(op->nfso_own->nfsow_owner, nlp->nfsl_openowner,
NFSV4CL_LOCKNAMELEN);
nlp->nfsl_seqid = 0;
-   nlp->nfsl_defunct = 0;
+   nlp->nfsl_lockflags = flags;
nlp->nfsl_inprog = NULL;
nfscl_lockinit(&nlp->nfsl_rwlock);
LIST_INIT(&nlp->nfsl_lock);
@@ -1638,7 +1640,6 @@ static void
 nfscl_cleanup_common(struct nfsclclient *clp, u_int8_t *own)
 {
struct nfsclowner *owp, *nowp;
-   struct nfsclopen *op;
struct nfscllockowner *lp, *nlp;
struct nfscldeleg *dp;
 
@@ -1667,15 +1668,6 @@ nfscl_cleanup_common(struct nfsclclient 
nfscl_freeopenowner(owp, 0);
else
owp->nfsow_defunct = 1;
-   } else {
-   /* look for lockowners on other opens */
-   LIST_FOREACH(op, &owp->nfsow_open, nfso_list) {
-   LIST_FOREACH(lp, &op->nfso_lock, nfsl_list) {
-   if (!NFSBCMP(lp->nfsl_owner, own,
-   NFSV4CL_LOCKNAMELEN))
-   lp->nfsl_defunct = 1;
-   }
-   }
}
owp = nowp;
}
@@ -1685,13 +1677,21 @@ nfscl_cleanup_common(struct nfsclclient 
  * Find open/lock owners for processes that have exited.
  */
 static void
-nfscl_cleanupkext(struct nfsclclient *clp)
+nfscl_cleanupkext(struct nfsclclient *clp, struct nfscllockownerfhhead *lhp)
 {
struct nfsclowner *owp, *nowp;
+   struct nfsclopen *op

Re: svn commit: r227778 - head/sys/net

[Julien Ridoux]

On 03/12/2011, at 12:02 PM, Jung-uk Kim wrote:

[snip]

> 
> - The snippet of code at the beginning of catchpacket() that
> was manipulating the struct bintime derived from bpf_gettime()
> was gross and has been removed in favour of selecting the
> right {get}bin{up}time() function call in bpf_gettime().
 
 I did that to reduce branching.  Since you are introducing more
 branches, it warrants a function pointer now.
>> 
>> There's another reason, BTW.  If mbufs are tagged with timestamps
>> (where only monotonic timestamps are allowed), they must be
>> converted within the bpf.c.  I forgot all the details. :-(
>> 
>>> I see, thanks for the explanation. Could you elaborate a bit more
>>> about how you would implement the function pointer idea? I'm also
>>> curious in the !FFCLOCK case just how much overhead having the
>>> 2-layer nested if/else adds. I'm not an very optimisation savvy
>>> person, but I'm wondering if it's actually worth micro-optimising
>>> this code.
>>> 
>>> My initial thoughts about your function pointer idea lead to
>>> adding a function pointer in the bpf_d and setting it to the
>>> appropriate function to get the timestamp from at bpf_d creation
>>> or ioctl time. Whilst I like this idea, I can't see how it would
>>> work given that the various functions involved in time/ffcounter
>>> stamp generation all have different signatures.
>>> 
>>> We could have multiple variants of bpf_gettime() which each call
>>> the appropriate underlying functions to generate the appropriate
>>> stamp. Would add quite a lot of code but would reduce the
>>> overhead of calling bpf_gettime() to an indirect function call +
>>> the underlying stamp generation function call. This also solves
>>> the problem of multiple function signatures.
>> 
>> Please see my patch:
>> 
>> http://people.freebsd.org/~jkim/bpf_ffclock.diff
> 
> I booted up the kernel and found it just crashes because of stupid 
> typos.  Now a new patch is uploaded in place.  Sorry.
> 
> Anyway, I see no regression nor ABI breakage. :-)
> 
> Jung-uk Kim
> 
>> This is what I originally intended to propose (just
>> compile-tested). I did not use union *intentionally* to make the
>> patch simpler. Instead, I added BPF_FFCOUNT() macro.  It's kinda
>> ugly but I don't see any compelling reason to introduce the union
>> (yet).
>> 
>>> We would also need to add the function pointer to the bpf_d
>>> struct which I guess breaks the ABI, something we're trying to
>>> avoid with this patch as we want to MFC it.
>> 
>> It does not affect user applications as bpfdesc.h is only meant for
>> kernel uses.  Also, it is very hard for me to imagine any
>> third-party drivers that use struct bpf_d directly.
>> 
>> Jung-uk Kim



Jung-uk, thanks for the patch, I understand much better.

Overall I am definitely not against this patch. I see two minor issues that 
require some attention though.

Using the higher level ffclock_[get]binuptime() instead of ffclock_abstime() 
adds an extra function call. My performance metric of interest is timestamping 
latency, because all things being equal, the variablity of the timestamping 
delay gives a less accurate picture of the network and affects the 
synchronisation daemon and the final quality of the clock.
Although I agree that the patch we proposed with Lawrence can be improved on 
this front, adding one more level of abstraction to the timestamping function 
goes the opposite direction. The earlier the timestamp is created, the better 
the results.

I am definitely not very savvy on the code optimisation front, and I need to be 
educated there. I am wondering if removing the branching in the code is worth 
this extra variability in latency. 

Looking ahead to the next patch we want to propose, the BPF header will contain 
a timestamp in seconds from one of the two clocks and a feed-forward counter. 
Here is a very simplistic proof of concept based on Lawrence's patch. I just 
put it there to support the discussion:

http://www.cubinlab.ee.unimelb.edu.au/~jrid/patches/r228191/bpfffcounter_concept_v2.patch

Since it is essential to read the hardware counter only once to produce the 
timestamp in second and the ffcounter, the prototype of the timestamping 
function stored in the bpf_d would have to be changed (if the kernel is 
compiled with the FFCLOCK option, at least).
For example, if BPF_T_FFCLOCK is set, this change will make use of 
ffclock_abstime() and solves the performance issue I raised above.
If the clock flag is not set, then the [get]binuptime() will need a wrapper 
function, and they will bear the timestamping latency cost.

Is this acceptable? If yes, then I don't have any objections to the patch.

Julien

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