Re: make pkg_info -Q work with other flags
On Wed, Aug 02, 2017 at 12:11:13PM -0600, Aaron Bieber wrote: > On Sat, Jul 29, 2017 at 10:40:37AM -0600, Aaron Bieber wrote: > > Hola, > > > > Currently "pkg_info -Q" doesn't respect other flags and the way > > pkg_info(1) reads, it implies that they will work with it. > > > > This diff makes pkg_info function as expected when other flags are > > passed when using -Q. > > Ping. Clue sticks? OKs? Makes sense to me, allows for example to do: $pkg_info -Q firefox -s Information for https://packages.rhaalovely.net/pub/OpenBSD/snapshots/packages/amd64/firefox-55.0rc1.tgz Size: 147593632 or $pkg_info -Q xfwm4 -f (which shows the packing-list for xfwm4 and xfwm4-themes) ok
Re: Kernel compilation fail with clang.
On Wed, Aug 02, 2017 at 09:06:00PM +0200, Olivier Antoine wrote: > Hi, > > I may have missed something but I'm unable to compile the kernel since (I > think) the switch to clang on AMD64. > > I have upgraded to the latest snapshot, updated my /sys tree > > $ cd /sys/arch/$(machine)/compile/GENERIC.MP > $ doas make obj > $ make config > $ make > > Then the compilation fail with this message: > > /sys/netinet/igmp.c:140:22: error: implicit conversion from 'int' to > 'int8_t' > (aka 'signed char') changes value from 148 to -108 > [-Werror,-Wconstant-conversion] > ra->ipopt_list[0] = IPOPT_RA; > ~ ^~~~ > /sys/netinet/ip.h:153:19: note: expanded from macro 'IPOPT_RA' > #define IPOPT_RA148 /* router alert */ > ^~~ > 1 error generated. > *** Error 1 in target 'igmp.o' > *** Error 1 in /sys/arch/amd64/compile/GENERIC.MP (Makefile:961 'igmp.o') > > Is there something wrong with my setup? > If my reading of the code is good, the error seems legitimate. Something is wrong with your checkout. /sys/arch/amd64/conf/Makefile.amd64 has -Wno-constant-conversion. make config creates /sys/arch/amd64/compile/GENERIC.MP/obj/Makefile based on that.
Remove duplicated commented out include in files.i386
Hi tech@, Remove duplicated commented out "dev/rasops/files.rasops" include. Comments? OK? Index: sys/arch/i386/conf/files.i386 === RCS file: /cvs/src/sys/arch/i386/conf/files.i386,v retrieving revision 1.233 diff -u -p -r1.233 files.i386 --- sys/arch/i386/conf/files.i386 31 May 2017 19:18:18 - 1.233 +++ sys/arch/i386/conf/files.i386 2 Aug 2017 19:30:30 - @@ -411,8 +411,6 @@ device esm attach esm at mainbus file arch/i386/i386/esm.cesm needs-flag -#include "dev/rasops/files.rasops" - # quad support is necessary for 32 bit architectures file lib/libkern/adddi3.c file lib/libkern/anddi3.c
Re: less(1) - segmentation fault with '-g'
On Wed, Aug 02, 2017 at 08:46:26PM +0200, Martijn van Duren wrote:
> You're right. Maybe I should read up on my less. :-)
>
> OK martijn@ for the full diff.
Thanks, committed.
> On 08/02/17 19:55, Anton Lindqvist wrote:
> > On Wed, Aug 02, 2017 at 07:19:50PM +0200, Martijn van Duren wrote:
> >> OK martijn@ for the pattern.c part.
> >>
> >> The search.c part seems unneeded.
> >> - If we don't have a match we don't enter hilite_line at all.
> >> - If we have a match and don't negate get sp/ep for every match.
> >> - If we have a match and negate, with your process.c patch we get a
> >> sp/ep with value NULL and return before entering the do/while.
> >>
> >> So with the above there's no situation where we can get in the
> >> do/while loop and still need to check for NULL on every pass, and the
> >> move is thus unnecessary churn.
> >
> > The changes to pattern.c do solve the initial problem as reported by
> > Larry. However, I'm experiencing a crash if I remove the changes to
> > search.c:
> >
> > $ cat in
> > foo
> > bar
> > bar
> > $ less in # type: /!foo
> >
> >>
> >> On 07/28/17 13:32, Anton Lindqvist wrote:
> >>> On Fri, Jul 28, 2017 at 01:19:39PM +0200, Theo Buehler wrote:
> On Thu, Jul 27, 2017 at 10:31:51AM +0100, Larry Hynes wrote:
> > Hi
> >
> > $ env | grep LESS
> > LESSHISTFILE=-
> > LESS="-i -M -R -g -c"
> > LESSCHARSET=utf-8
> >
> > $ unset LESS
> > $ unset LESSCHARSET
> > $ unset LESSHISTFILE
> >
> > $ LESS="-g"
> > $ echo 'foo\nbar\nfoo\nbar\nfoo\nbar' | less
> >
> > While in less, '/' to search, then '^N', or '!', to search for lines
> > which do NOT match the pattern, entering foo results in a seg
> > fault.
> >
> > This is on amd64.
>
> Thanks for the report. Of course you want 'export LESS="-g"' here,
> otherwise less(1) won't use the -g flag which is crucial for this crash.
>
> I don't have a fix, but maybe this helps a bit:
>
> From the command line, without environment variable magic and no
> interactive component you can reproduce as follows:
>
> $ echo 'foo\nbar\nfoo\n' > crash
> $ less -g -p '!foo' crash
>
> It's apparently a use-after-free that happens in search_range():
>
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0 0x09d426e1b35b in create_hilites (linepos=4,
> start_index=-370944992, end_index=-370944989, chpos=0x9d6c5994c80)
> at /usr/src/usr.bin/less/less/../search.c:445
> 445 hl->hl_startpos = linepos + chpos[start_index];
> (gdb) bt
> #0 0x09d426e1b35b in create_hilites (linepos=4,
> start_index=-370944992, end_index=-370944989, chpos=0x9d6c5994c80)
> at /usr/src/usr.bin/less/less/../search.c:445
> #1 0x09d426e1b24c in hilite_line (linepos=4, line=0x9d65872bf70
> "bar", line_len=3, chpos=0x9d6c5994c80,
> sp=0x9d642569390 '\337' , \337>,
> ep=0x9d642569393 '\337' , \337>) at /usr/src/usr.bin/less/less/../search.c:494
> #2 0x09d426e1aa99 in search_range (pos=8, endpos=-1,
> search_type=257, matches=0, maxlines=-1, plinepos=0x7f7e9ff0,
> pendpos=0x0) at /usr/src/usr.bin/less/less/../search.c:804
> #3 0x09d426e1a258 in search (search_type=257, pattern=0x9d42702ca40
> "foo", n=1)
> at /usr/src/usr.bin/less/less/../search.c:925
> #4 0x09d426e0a182 in multi_search (pattern=0x9d42702ca40
> "foo", n=1) at /usr/src/usr.bin/less/less/../command.c:815
> #5 0x09d426e0a779 in exec_mca () at
> /usr/src/usr.bin/less/less/../command.c:189
> #6 0x09d426e09d03 in mca_char (c=10) at
> /usr/src/usr.bin/less/less/../command.c:532
> #7 0x09d426e08865 in commands () at
> /usr/src/usr.bin/less/less/../command.c:971
> #8 0x09d426e010f2 in main (argc=-1, argv=0x7f7ea200) at
> /usr/src/usr.bin/less/less/../main.c:274
>
> >>>
> >>> I took a closer look earlier today. As I see it, {start,end}_index does
> >>> not get updated inside match_pattern() since we're looking for inverted
> >>> matches, i.e. when regexec() indicates no match we actually got a match.
> >>> Instead, the indices could be set to NULL up front and the corresponding
> >>> NULL-check moved into the loop since match_pattern() is called
> >>> repeatedly. Tested with and without `-g` for both normal and inverted
> >>> search.
> >>>
> >>> Index: pattern.c
> >>> ===
> >>> RCS file: /cvs/src/usr.bin/less/pattern.c,v
> >>> retrieving revision 1.9
> >>> diff -u -p -r1.9 pattern.c
> >>> --- pattern.c 21 Nov 2015 13:29:12 - 1.9
> >>> +++ pattern.c 28 Jul 2017 11:31:34 -
> >>> @@ -122,6 +122,8 @@ match_pattern(void *pattern, char *tpatt
> >>> rm.rm_so = 0;
> >>> rm.rm_eo = line_len;
> >>> #endif
> >>> +
Kernel compilation fail with clang.
Hi, I may have missed something but I'm unable to compile the kernel since (I think) the switch to clang on AMD64. I have upgraded to the latest snapshot, updated my /sys tree $ cd /sys/arch/$(machine)/compile/GENERIC.MP $ doas make obj $ make config $ make Then the compilation fail with this message: /sys/netinet/igmp.c:140:22: error: implicit conversion from 'int' to 'int8_t' (aka 'signed char') changes value from 148 to -108 [-Werror,-Wconstant-conversion] ra->ipopt_list[0] = IPOPT_RA; ~ ^~~~ /sys/netinet/ip.h:153:19: note: expanded from macro 'IPOPT_RA' #define IPOPT_RA148 /* router alert */ ^~~ 1 error generated. *** Error 1 in target 'igmp.o' *** Error 1 in /sys/arch/amd64/compile/GENERIC.MP (Makefile:961 'igmp.o') Is there something wrong with my setup? If my reading of the code is good, the error seems legitimate.
Fix libexec/ld.so/dlclose regress
Couldn't convince clang not to inline duplicateFun() into bbTest2().
Splitting things out in a seperate file avoids the issue. Fixes the
regression test.
ok?
Index: regress/libexec/ld.so/dlclose/test1/libbb/Makefile
===
RCS file: /cvs/src/regress/libexec/ld.so/dlclose/test1/libbb/Makefile,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 Makefile
--- regress/libexec/ld.so/dlclose/test1/libbb/Makefile 28 Sep 2005 15:42:32
- 1.1.1.1
+++ regress/libexec/ld.so/dlclose/test1/libbb/Makefile 2 Aug 2017 18:59:14
-
@@ -1,7 +1,7 @@
# $OpenBSD: Makefile,v 1.1.1.1 2005/09/28 15:42:32 kurt Exp $
LIB= bb
-SRCS= bb.c
+SRCS= bb.c bbb.c
regress: all
Index: regress/libexec/ld.so/dlclose/test1/libbb/bb.c
===
RCS file: /cvs/src/regress/libexec/ld.so/dlclose/test1/libbb/bb.c,v
retrieving revision 1.2
diff -u -p -r1.2 bb.c
--- regress/libexec/ld.so/dlclose/test1/libbb/bb.c 30 Sep 2005 14:57:35
- 1.2
+++ regress/libexec/ld.so/dlclose/test1/libbb/bb.c 2 Aug 2017 18:59:14
-
@@ -20,19 +20,10 @@
#include
#include
-int bbSymbol;
-
-void
-bbLazyFun()
-{
+extern void bbLazyFun();
+extern int duplicateFun();
-}
-
-int
-duplicateFun()
-{
- return (1);
-}
+int bbSymbol;
int
bbTest1()
Index: regress/libexec/ld.so/dlclose/test1/libbb/bbb.c
===
RCS file: regress/libexec/ld.so/dlclose/test1/libbb/bbb.c
diff -N regress/libexec/ld.so/dlclose/test1/libbb/bbb.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/libexec/ld.so/dlclose/test1/libbb/bbb.c 2 Aug 2017 18:59:14
-
@@ -0,0 +1,30 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2005 Kurt Miller
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ */
+
+void
+bbLazyFun()
+{
+
+}
+
+int
+duplicateFun()
+{
+ return (1);
+}
Fix libunwind on i386
So apparently Apple messed up the register numbering, or at least
chose to be different:
https://reviews.llvm.org/D22508
Fixes the exception handling regression tests on i386 with clang.
ok?
Index: lib/libunwind/include/libunwind.h
===
RCS file: /cvs/src/lib/libunwind/include/libunwind.h,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 libunwind.h
--- lib/libunwind/include/libunwind.h 3 Sep 2016 18:42:12 - 1.1.1.1
+++ lib/libunwind/include/libunwind.h 2 Aug 2017 18:42:08 -
@@ -151,8 +151,13 @@ enum {
UNW_X86_ECX = 1,
UNW_X86_EDX = 2,
UNW_X86_EBX = 3,
+#ifdef __OpenBSD__
+ UNW_X86_ESP = 4,
+ UNW_X86_EBP = 5,
+#else
UNW_X86_EBP = 4,
UNW_X86_ESP = 5,
+#endif
UNW_X86_ESI = 6,
UNW_X86_EDI = 7
};
Re: less(1) - segmentation fault with '-g'
You're right. Maybe I should read up on my less. :-)
OK martijn@ for the full diff.
On 08/02/17 19:55, Anton Lindqvist wrote:
> On Wed, Aug 02, 2017 at 07:19:50PM +0200, Martijn van Duren wrote:
>> OK martijn@ for the pattern.c part.
>>
>> The search.c part seems unneeded.
>> - If we don't have a match we don't enter hilite_line at all.
>> - If we have a match and don't negate get sp/ep for every match.
>> - If we have a match and negate, with your process.c patch we get a
>> sp/ep with value NULL and return before entering the do/while.
>>
>> So with the above there's no situation where we can get in the
>> do/while loop and still need to check for NULL on every pass, and the
>> move is thus unnecessary churn.
>
> The changes to pattern.c do solve the initial problem as reported by
> Larry. However, I'm experiencing a crash if I remove the changes to
> search.c:
>
> $ cat in
> foo
> bar
> bar
> $ less in # type: /!foo
>
>>
>> On 07/28/17 13:32, Anton Lindqvist wrote:
>>> On Fri, Jul 28, 2017 at 01:19:39PM +0200, Theo Buehler wrote:
On Thu, Jul 27, 2017 at 10:31:51AM +0100, Larry Hynes wrote:
> Hi
>
> $ env | grep LESS
> LESSHISTFILE=-
> LESS="-i -M -R -g -c"
> LESSCHARSET=utf-8
>
> $ unset LESS
> $ unset LESSCHARSET
> $ unset LESSHISTFILE
>
> $ LESS="-g"
> $ echo 'foo\nbar\nfoo\nbar\nfoo\nbar' | less
>
> While in less, '/' to search, then '^N', or '!', to search for lines
> which do NOT match the pattern, entering foo results in a seg
> fault.
>
> This is on amd64.
Thanks for the report. Of course you want 'export LESS="-g"' here,
otherwise less(1) won't use the -g flag which is crucial for this crash.
I don't have a fix, but maybe this helps a bit:
From the command line, without environment variable magic and no
interactive component you can reproduce as follows:
$ echo 'foo\nbar\nfoo\n' > crash
$ less -g -p '!foo' crash
It's apparently a use-after-free that happens in search_range():
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x09d426e1b35b in create_hilites (linepos=4,
start_index=-370944992, end_index=-370944989, chpos=0x9d6c5994c80)
at /usr/src/usr.bin/less/less/../search.c:445
445 hl->hl_startpos = linepos + chpos[start_index];
(gdb) bt
#0 0x09d426e1b35b in create_hilites (linepos=4,
start_index=-370944992, end_index=-370944989, chpos=0x9d6c5994c80)
at /usr/src/usr.bin/less/less/../search.c:445
#1 0x09d426e1b24c in hilite_line (linepos=4, line=0x9d65872bf70
"bar", line_len=3, chpos=0x9d6c5994c80,
sp=0x9d642569390 '\337' , ,
ep=0x9d642569393 '\337' , >>> \337>) at /usr/src/usr.bin/less/less/../search.c:494
#2 0x09d426e1aa99 in search_range (pos=8, endpos=-1, search_type=257,
matches=0, maxlines=-1, plinepos=0x7f7e9ff0,
pendpos=0x0) at /usr/src/usr.bin/less/less/../search.c:804
#3 0x09d426e1a258 in search (search_type=257, pattern=0x9d42702ca40
"foo", n=1)
at /usr/src/usr.bin/less/less/../search.c:925
#4 0x09d426e0a182 in multi_search (pattern=0x9d42702ca40
"foo", n=1) at /usr/src/usr.bin/less/less/../command.c:815
#5 0x09d426e0a779 in exec_mca () at
/usr/src/usr.bin/less/less/../command.c:189
#6 0x09d426e09d03 in mca_char (c=10) at
/usr/src/usr.bin/less/less/../command.c:532
#7 0x09d426e08865 in commands () at
/usr/src/usr.bin/less/less/../command.c:971
#8 0x09d426e010f2 in main (argc=-1, argv=0x7f7ea200) at
/usr/src/usr.bin/less/less/../main.c:274
>>>
>>> I took a closer look earlier today. As I see it, {start,end}_index does
>>> not get updated inside match_pattern() since we're looking for inverted
>>> matches, i.e. when regexec() indicates no match we actually got a match.
>>> Instead, the indices could be set to NULL up front and the corresponding
>>> NULL-check moved into the loop since match_pattern() is called
>>> repeatedly. Tested with and without `-g` for both normal and inverted
>>> search.
>>>
>>> Index: pattern.c
>>> ===
>>> RCS file: /cvs/src/usr.bin/less/pattern.c,v
>>> retrieving revision 1.9
>>> diff -u -p -r1.9 pattern.c
>>> --- pattern.c 21 Nov 2015 13:29:12 - 1.9
>>> +++ pattern.c 28 Jul 2017 11:31:34 -
>>> @@ -122,6 +122,8 @@ match_pattern(void *pattern, char *tpatt
>>> rm.rm_so = 0;
>>> rm.rm_eo = line_len;
>>> #endif
>>> + *sp = NULL;
>>> + *ep = NULL;
>>> matched = !regexec(spattern, line, 1, &rm, flags);
>>> if (matched) {
>>> *sp = line + rm.rm_so;
>>> Index: search.c
>>> ===
>>> RCS file: /cvs/src/usr.bin/less/
Re: make pkg_info -Q work with other flags
On Sat, Jul 29, 2017 at 10:40:37AM -0600, Aaron Bieber wrote:
> Hola,
>
> Currently "pkg_info -Q" doesn't respect other flags and the way
> pkg_info(1) reads, it implies that they will work with it.
>
> This diff makes pkg_info function as expected when other flags are
> passed when using -Q.
Ping. Clue sticks? OKs?
>
> Cheers,
> Aaron
>
> Index: OpenBSD/PkgInfo.pm
> ===
> RCS file: /cvs/src/usr.sbin/pkg_add/OpenBSD/PkgInfo.pm,v
> retrieving revision 1.44
> diff -u -p -r1.44 PkgInfo.pm
> --- OpenBSD/PkgInfo.pm25 Jan 2017 14:10:46 - 1.44
> +++ OpenBSD/PkgInfo.pm29 Jul 2017 16:32:54 -
> @@ -406,13 +406,18 @@ sub print_info
> }
> $state->say("#1", $compose);
> } elsif ($state->opt('I')) {
> - if ($state->opt('q')) {
> - $state->say("#1", $pkg);
> + if ($state->opt('Q')) {
> + $state->say(
> + is_installed($pkg) ? "#1 (installed)" : "#1", $pkg);
> } else {
> - my $l = 20 - length($pkg);
> - $l = 1 if $l <= 0;
> - $state->say("#1#2#3", $pkg, " "x$l,
> - get_comment($handle->info));
> + if ($state->opt('q')) {
> + $state->say("#1", $pkg);
> + } else {
> + my $l = 20 - length($pkg);
> + $l = 1 if $l <= 0;
> + $state->say("#1#2#3", $pkg, " "x$l,
> + get_comment($handle->info));
> + }
> }
> } else {
> if ($state->opt('c')) {
> @@ -468,7 +473,7 @@ sub print_info
>
> OpenBSD::x509::print_certificate_info($plist);
> } elsif ($sig->{key} eq 'signify' ||
> $sig->{key} eq 'signify2') {
> - $state->say("reportedly signed by #1",
> + $state->say("reportedly signed by #1",
> $plist->get('signer')->name);
> }
> } else {
> @@ -601,8 +606,10 @@ sub parse_and_run
> my $r = $state->repo->match_locations($partial);
>
> for my $p (sort map {$_->name} @$r) {
> - $state->say(
> - is_installed($p) ? "#1 (installed)" : "#1", $p);
> + $self->find_pkg($state, $p,
> + sub {
> + $self->print_info($state, @_);
> + });
> }
>
> return 0;
>
>
> --
> PGP: 0x1F81112D62A9ADCE / 3586 3350 BFEA C101 DB1A 4AF0 1F81 112D 62A9 ADCE
>
--
PGP: 0x1F81112D62A9ADCE / 3586 3350 BFEA C101 DB1A 4AF0 1F81 112D 62A9 ADCE
Re: less(1) - segmentation fault with '-g'
On Wed, Aug 02, 2017 at 07:19:50PM +0200, Martijn van Duren wrote:
> OK martijn@ for the pattern.c part.
>
> The search.c part seems unneeded.
> - If we don't have a match we don't enter hilite_line at all.
> - If we have a match and don't negate get sp/ep for every match.
> - If we have a match and negate, with your process.c patch we get a
> sp/ep with value NULL and return before entering the do/while.
>
> So with the above there's no situation where we can get in the
> do/while loop and still need to check for NULL on every pass, and the
> move is thus unnecessary churn.
The changes to pattern.c do solve the initial problem as reported by
Larry. However, I'm experiencing a crash if I remove the changes to
search.c:
$ cat in
foo
bar
bar
$ less in # type: /!foo
>
> On 07/28/17 13:32, Anton Lindqvist wrote:
> > On Fri, Jul 28, 2017 at 01:19:39PM +0200, Theo Buehler wrote:
> >> On Thu, Jul 27, 2017 at 10:31:51AM +0100, Larry Hynes wrote:
> >>> Hi
> >>>
> >>> $ env | grep LESS
> >>> LESSHISTFILE=-
> >>> LESS="-i -M -R -g -c"
> >>> LESSCHARSET=utf-8
> >>>
> >>> $ unset LESS
> >>> $ unset LESSCHARSET
> >>> $ unset LESSHISTFILE
> >>>
> >>> $ LESS="-g"
> >>> $ echo 'foo\nbar\nfoo\nbar\nfoo\nbar' | less
> >>>
> >>> While in less, '/' to search, then '^N', or '!', to search for lines
> >>> which do NOT match the pattern, entering foo results in a seg
> >>> fault.
> >>>
> >>> This is on amd64.
> >>
> >> Thanks for the report. Of course you want 'export LESS="-g"' here,
> >> otherwise less(1) won't use the -g flag which is crucial for this crash.
> >>
> >> I don't have a fix, but maybe this helps a bit:
> >>
> >> From the command line, without environment variable magic and no
> >> interactive component you can reproduce as follows:
> >>
> >> $ echo 'foo\nbar\nfoo\n' > crash
> >> $ less -g -p '!foo' crash
> >>
> >> It's apparently a use-after-free that happens in search_range():
> >>
> >> Program terminated with signal SIGSEGV, Segmentation fault.
> >> #0 0x09d426e1b35b in create_hilites (linepos=4,
> >> start_index=-370944992, end_index=-370944989, chpos=0x9d6c5994c80)
> >> at /usr/src/usr.bin/less/less/../search.c:445
> >> 445 hl->hl_startpos = linepos + chpos[start_index];
> >> (gdb) bt
> >> #0 0x09d426e1b35b in create_hilites (linepos=4,
> >> start_index=-370944992, end_index=-370944989, chpos=0x9d6c5994c80)
> >> at /usr/src/usr.bin/less/less/../search.c:445
> >> #1 0x09d426e1b24c in hilite_line (linepos=4, line=0x9d65872bf70
> >> "bar", line_len=3, chpos=0x9d6c5994c80,
> >> sp=0x9d642569390 '\337' , ,
> >> ep=0x9d642569393 '\337' , >> \337>) at /usr/src/usr.bin/less/less/../search.c:494
> >> #2 0x09d426e1aa99 in search_range (pos=8, endpos=-1, search_type=257,
> >> matches=0, maxlines=-1, plinepos=0x7f7e9ff0,
> >> pendpos=0x0) at /usr/src/usr.bin/less/less/../search.c:804
> >> #3 0x09d426e1a258 in search (search_type=257, pattern=0x9d42702ca40
> >> "foo", n=1)
> >> at /usr/src/usr.bin/less/less/../search.c:925
> >> #4 0x09d426e0a182 in multi_search (pattern=0x9d42702ca40
> >> "foo", n=1) at /usr/src/usr.bin/less/less/../command.c:815
> >> #5 0x09d426e0a779 in exec_mca () at
> >> /usr/src/usr.bin/less/less/../command.c:189
> >> #6 0x09d426e09d03 in mca_char (c=10) at
> >> /usr/src/usr.bin/less/less/../command.c:532
> >> #7 0x09d426e08865 in commands () at
> >> /usr/src/usr.bin/less/less/../command.c:971
> >> #8 0x09d426e010f2 in main (argc=-1, argv=0x7f7ea200) at
> >> /usr/src/usr.bin/less/less/../main.c:274
> >>
> >
> > I took a closer look earlier today. As I see it, {start,end}_index does
> > not get updated inside match_pattern() since we're looking for inverted
> > matches, i.e. when regexec() indicates no match we actually got a match.
> > Instead, the indices could be set to NULL up front and the corresponding
> > NULL-check moved into the loop since match_pattern() is called
> > repeatedly. Tested with and without `-g` for both normal and inverted
> > search.
> >
> > Index: pattern.c
> > ===
> > RCS file: /cvs/src/usr.bin/less/pattern.c,v
> > retrieving revision 1.9
> > diff -u -p -r1.9 pattern.c
> > --- pattern.c 21 Nov 2015 13:29:12 - 1.9
> > +++ pattern.c 28 Jul 2017 11:31:34 -
> > @@ -122,6 +122,8 @@ match_pattern(void *pattern, char *tpatt
> > rm.rm_so = 0;
> > rm.rm_eo = line_len;
> > #endif
> > + *sp = NULL;
> > + *ep = NULL;
> > matched = !regexec(spattern, line, 1, &rm, flags);
> > if (matched) {
> > *sp = line + rm.rm_so;
> > Index: search.c
> > ===
> > RCS file: /cvs/src/usr.bin/less/search.c,v
> > retrieving revision 1.18
> > diff -u -p -r1.18 search.c
> > --- search.c17 Sep 2016 15:06:41 - 1.18
> > +++ search.c28 J
Re: less(1) - segmentation fault with '-g'
OK martijn@ for the pattern.c part.
The search.c part seems unneeded.
- If we don't have a match we don't enter hilite_line at all.
- If we have a match and don't negate get sp/ep for every match.
- If we have a match and negate, with your process.c patch we get a
sp/ep with value NULL and return before entering the do/while.
So with the above there's no situation where we can get in the
do/while loop and still need to check for NULL on every pass, and the
move is thus unnecessary churn.
On 07/28/17 13:32, Anton Lindqvist wrote:
> On Fri, Jul 28, 2017 at 01:19:39PM +0200, Theo Buehler wrote:
>> On Thu, Jul 27, 2017 at 10:31:51AM +0100, Larry Hynes wrote:
>>> Hi
>>>
>>> $ env | grep LESS
>>> LESSHISTFILE=-
>>> LESS="-i -M -R -g -c"
>>> LESSCHARSET=utf-8
>>>
>>> $ unset LESS
>>> $ unset LESSCHARSET
>>> $ unset LESSHISTFILE
>>>
>>> $ LESS="-g"
>>> $ echo 'foo\nbar\nfoo\nbar\nfoo\nbar' | less
>>>
>>> While in less, '/' to search, then '^N', or '!', to search for lines
>>> which do NOT match the pattern, entering foo results in a seg
>>> fault.
>>>
>>> This is on amd64.
>>
>> Thanks for the report. Of course you want 'export LESS="-g"' here,
>> otherwise less(1) won't use the -g flag which is crucial for this crash.
>>
>> I don't have a fix, but maybe this helps a bit:
>>
>> From the command line, without environment variable magic and no
>> interactive component you can reproduce as follows:
>>
>> $ echo 'foo\nbar\nfoo\n' > crash
>> $ less -g -p '!foo' crash
>>
>> It's apparently a use-after-free that happens in search_range():
>>
>> Program terminated with signal SIGSEGV, Segmentation fault.
>> #0 0x09d426e1b35b in create_hilites (linepos=4, start_index=-370944992,
>> end_index=-370944989, chpos=0x9d6c5994c80)
>> at /usr/src/usr.bin/less/less/../search.c:445
>> 445 hl->hl_startpos = linepos + chpos[start_index];
>> (gdb) bt
>> #0 0x09d426e1b35b in create_hilites (linepos=4, start_index=-370944992,
>> end_index=-370944989, chpos=0x9d6c5994c80)
>> at /usr/src/usr.bin/less/less/../search.c:445
>> #1 0x09d426e1b24c in hilite_line (linepos=4, line=0x9d65872bf70 "bar",
>> line_len=3, chpos=0x9d6c5994c80,
>> sp=0x9d642569390 '\337' , ,
>> ep=0x9d642569393 '\337' , )
>> at /usr/src/usr.bin/less/less/../search.c:494
>> #2 0x09d426e1aa99 in search_range (pos=8, endpos=-1, search_type=257,
>> matches=0, maxlines=-1, plinepos=0x7f7e9ff0,
>> pendpos=0x0) at /usr/src/usr.bin/less/less/../search.c:804
>> #3 0x09d426e1a258 in search (search_type=257, pattern=0x9d42702ca40
>> "foo", n=1)
>> at /usr/src/usr.bin/less/less/../search.c:925
>> #4 0x09d426e0a182 in multi_search (pattern=0x9d42702ca40
>> "foo", n=1) at /usr/src/usr.bin/less/less/../command.c:815
>> #5 0x09d426e0a779 in exec_mca () at
>> /usr/src/usr.bin/less/less/../command.c:189
>> #6 0x09d426e09d03 in mca_char (c=10) at
>> /usr/src/usr.bin/less/less/../command.c:532
>> #7 0x09d426e08865 in commands () at
>> /usr/src/usr.bin/less/less/../command.c:971
>> #8 0x09d426e010f2 in main (argc=-1, argv=0x7f7ea200) at
>> /usr/src/usr.bin/less/less/../main.c:274
>>
>
> I took a closer look earlier today. As I see it, {start,end}_index does
> not get updated inside match_pattern() since we're looking for inverted
> matches, i.e. when regexec() indicates no match we actually got a match.
> Instead, the indices could be set to NULL up front and the corresponding
> NULL-check moved into the loop since match_pattern() is called
> repeatedly. Tested with and without `-g` for both normal and inverted
> search.
>
> Index: pattern.c
> ===
> RCS file: /cvs/src/usr.bin/less/pattern.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 pattern.c
> --- pattern.c 21 Nov 2015 13:29:12 - 1.9
> +++ pattern.c 28 Jul 2017 11:31:34 -
> @@ -122,6 +122,8 @@ match_pattern(void *pattern, char *tpatt
> rm.rm_so = 0;
> rm.rm_eo = line_len;
> #endif
> + *sp = NULL;
> + *ep = NULL;
> matched = !regexec(spattern, line, 1, &rm, flags);
> if (matched) {
> *sp = line + rm.rm_so;
> Index: search.c
> ===
> RCS file: /cvs/src/usr.bin/less/search.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 search.c
> --- search.c 17 Sep 2016 15:06:41 - 1.18
> +++ search.c 28 Jul 2017 11:31:34 -
> @@ -477,8 +477,6 @@ hilite_line(off_t linepos, char *line, i
> char *searchp;
> char *line_end = line + line_len;
>
> - if (sp == NULL || ep == NULL)
> - return;
> /*
>* sp and ep delimit the first match in the line.
>* Mark the corresponding file positions, then
> @@ -491,6 +489,9 @@ hilite_line(off_t linepos, char *line, i
>*/
> searchp = line;
> do {
> + if (sp == NULL || ep == NULL
Re: fstat output
On Wed, 02 Aug 2017 11:01:38 +0200, Alexander Bluhm wrote: > On Wed, Aug 02, 2017 at 09:56:50AM +0200, Martin Pieuchot wrote: > > Simple diff to improved readability. Before: > > There are situations where you want a full match on the command. > For testing and scripting with long program names this is necessary. > With a -v switch that shows the long name I would accept the patch. > netstat has that, too. And it would help in my use case. Which long name do you mean? ps_comm is limited to 16 characters. You won't really get the full process name without fetching argv and even then the program is free to modify argv[0]. - todd
Re: clang ld.so regress failures
On Wed, Aug 02, 2017 at 12:09:13AM -0400, Ted Unangst wrote: > Mark Kettenis wrote: > > FAIL libexec/ld.so/dlclose/test1/prog3/prog3 > > > > This fails because clang doesn't respect ELF interposition: > > > > http://lists.llvm.org/pipermail/llvm-dev/2016-November/107625.html > > > > We generally frown upon interposition so I can have some sympathy > > for their position here. Probably not a huge issue in practice. > > But we still want to test the ld.so functionality. > > would this affect, for example, a program's ability to override > malloc/free/etc? i'm trying to understand what's not working (or what will not > work). it sounds like i can override malloc in my program, but clang/llvm will > generate "tight" (not sure of the word) bindings to libc malloc within libc? > although this would probably only manifest within translation units with > inlining? Correct, functions in the same TU can be inlined, even when -fPIC is active. It primarily becomes a problem when using LTO, but I don't think you are that far :) Joerg
nd6 address expiration & NET_LOCK() contention
Florian killed the default router and prefix lists from the kernel. So
nd6_timer() is now only doing address expiration based on pltime/vltime.
Diff below renames the function and kill outdated comments to reflect
the reality.
Now, this timer wakes a task every second. And even if there's nothing
to do the task tries to grab the NET_LOCK(). Being scheduled on `systq'
this task creates contention with the `softnettq' and any userland
program trying to grab the lock. So this diff also moves this task to
the `softnettq'.
ok?
Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.211
diff -u -p -r1.211 nd6.c
--- netinet6/nd6.c 12 Jul 2017 16:53:58 - 1.211
+++ netinet6/nd6.c 2 Aug 2017 09:18:13 -
@@ -90,15 +90,15 @@ int nd6_inuse, nd6_allocated;
int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
void nd6_slowtimo(void *);
-void nd6_timer_work(void *);
-void nd6_timer(void *);
+void nd6_expire(void *);
+void nd6_expire_timer(void *);
void nd6_invalidate(struct rtentry *);
struct llinfo_nd6 *nd6_free(struct rtentry *, int);
void nd6_llinfo_timer(void *);
struct timeout nd6_slowtimo_ch;
-struct timeout nd6_timer_ch;
-struct task nd6_timer_task;
+struct timeout nd6_expire_timeout;
+struct task nd6_expire_task;
void
nd6_init(void)
@@ -114,17 +114,15 @@ nd6_init(void)
pool_init(&nd6_pool, sizeof(struct llinfo_nd6), 0,
IPL_SOFTNET, 0, "nd6", NULL);
- /* initialization of the default router list */
-
- task_set(&nd6_timer_task, nd6_timer_work, NULL);
+ task_set(&nd6_expire_task, nd6_expire, NULL);
nd6_init_done = 1;
/* start timer */
timeout_set_proc(&nd6_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(&nd6_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
- timeout_set(&nd6_timer_ch, nd6_timer, NULL);
- timeout_add_sec(&nd6_timer_ch, nd6_prune);
+ timeout_set(&nd6_expire_timeout, nd6_expire_timer, NULL);
+ timeout_add_sec(&nd6_expire_timeout, nd6_prune);
}
@@ -420,24 +418,19 @@ nd6_llinfo_timer(void *arg)
}
/*
- * ND6 timer routine to expire default route list and prefix list
+ * Expire interface addresses.
*/
void
-nd6_timer_work(void *null)
+nd6_expire(void *unused)
{
struct ifnet *ifp;
int s;
+ KERNEL_LOCK();
NET_LOCK(s);
- timeout_add_sec(&nd6_timer_ch, nd6_prune);
+ timeout_add_sec(&nd6_expire_timeout, nd6_prune);
- /*
-* expire interface addresses.
-* in the past the loop was inside prefix expiry processing.
-* However, from a stricter spec-conformance standpoint, we should
-* rather separate address lifetimes and prefix lifetimes.
-*/
TAILQ_FOREACH(ifp, &ifnet, if_list) {
struct ifaddr *ifa, *nifa;
struct in6_ifaddr *ia6;
@@ -462,12 +455,13 @@ nd6_timer_work(void *null)
}
NET_UNLOCK(s);
+ KERNEL_UNLOCK();
}
void
-nd6_timer(void *ignored_arg)
+nd6_expire_timer(void *unused)
{
- task_add(systq, &nd6_timer_task);
+ task_add(softnettq, &nd6_expire_task);
}
/*
Re: clang ld.so regress failures
> From: "Ted Unangst" > Date: Wed, 02 Aug 2017 00:09:13 -0400 > > Mark Kettenis wrote: > > FAIL libexec/ld.so/dlclose/test1/prog3/prog3 > > > > This fails because clang doesn't respect ELF interposition: > > > > http://lists.llvm.org/pipermail/llvm-dev/2016-November/107625.html > > > > We generally frown upon interposition so I can have some sympathy > > for their position here. Probably not a huge issue in practice. > > But we still want to test the ld.so functionality. > > would this affect, for example, a program's ability to override > malloc/free/etc? i'm trying to understand what's not working (or > what will not work). it sounds like i can override malloc in my > program, but clang/llvm will generate "tight" (not sure of the word) > bindings to libc malloc within libc? although this would probably > only manifest within translation units with inlining? Right. It seems that this only happens for inlining for now, so malloc should be safe for now. And that's the only case where we really support interposing. So I don't think this is a big issue.
Re: Reducing NET_LOCK() contention
On 2.8.2017. 11:00, Alexandr Nedvedicky wrote: > Hello, > > On Wed, Aug 02, 2017 at 10:10:51AM +0200, Martin Pieuchot wrote: >> On 18/07/17(Tue) 15:55, Martin Pieuchot wrote: >>> When forwarding a lot of traffic with 10G interfaces contention on the >>> NET_LOCK() is "visible". Each time you type "ifconfig" you can go grab >>> a coffee... >>> >>> The problem has a name: pf_purge_thread(). This thread is created by >>> default and run even if you don't have PF enabled. Every `hz' it wakes >>> up, grab the lock and go to sleep. >>> >>> Since the execution of this thread is serialized with the `softnet' task, >>> it makes more sense to execute it in the same context. This reduce the >>> NET_LOCK() contention and implicitly preempt the packet processing. >>> >>> Diff below improves the situation with PF disabled, I didn't test with >>> PF enabled. >> >> Updated diff that includes sashan@ suggestions and do not stop the purge >> task when PF is disabled. Otherwise some states are not purged until PF >> is re-enabled. This can be optimized later. >> > > thank Hrvoje for spotting the glitch. i should have spotted it with the first diff, but then i haven't disabled pf while generating traffic ...
Re: Reducing NET_LOCK() contention
On 2.8.2017. 10:10, Martin Pieuchot wrote: > On 18/07/17(Tue) 15:55, Martin Pieuchot wrote: >> When forwarding a lot of traffic with 10G interfaces contention on the >> NET_LOCK() is "visible". Each time you type "ifconfig" you can go grab >> a coffee... >> >> The problem has a name: pf_purge_thread(). This thread is created by >> default and run even if you don't have PF enabled. Every `hz' it wakes >> up, grab the lock and go to sleep. >> >> Since the execution of this thread is serialized with the `softnet' task, >> it makes more sense to execute it in the same context. This reduce the >> NET_LOCK() contention and implicitly preempt the packet processing. >> >> Diff below improves the situation with PF disabled, I didn't test with >> PF enabled. > Updated diff that includes sashan@ suggestions and do not stop the purge > task when PF is disabled. Otherwise some states are not purged until PF > is re-enabled. This can be optimized later. Hi all, with this diff states are purged as expected and ifconfig, netstat and arp outputs are little faster then before.
so{s,g}etopt() & solock
Diff below moves the socket lock "above" sosetopt(), sogetopt() and
sosplice(). While this adds a lot of lock/unlock dances in NFS, they
will be merge in a later diff.
sosetopt() modifies a socket fields so it needs the lock. sogetopt()
do not always need it, but it makes the code simpler to always grab
it and the code section is really small anyway.
ok?
Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.198
diff -u -p -r1.198 uipc_socket.c
--- kern/uipc_socket.c 27 Jul 2017 12:05:36 - 1.198
+++ kern/uipc_socket.c 2 Aug 2017 08:21:45 -
@@ -1073,7 +1073,9 @@ sosplice(struct socket *so, int fd, off_
struct file *fp;
struct socket *sosp;
struct sosplice *sp;
- int s, error = 0;
+ int error = 0;
+
+ soassertlocked(so);
if (sosplice_taskq == NULL)
sosplice_taskq = taskq_create("sosplice", 1, IPL_SOFTNET, 0);
@@ -1097,17 +1099,14 @@ sosplice(struct socket *so, int fd, off_
/* If no fd is given, unsplice by removing existing link. */
if (fd < 0) {
- s = solock(so);
/* Lock receive buffer. */
if ((error = sblock(so, &so->so_rcv,
(so->so_state & SS_NBIO) ? M_NOWAIT : M_WAITOK)) != 0) {
- sounlock(s);
return (error);
}
if (so->so_sp->ssp_socket)
sounsplice(so, so->so_sp->ssp_socket, 1);
sbunlock(&so->so_rcv);
- sounlock(s);
return (0);
}
@@ -1129,17 +1128,14 @@ sosplice(struct socket *so, int fd, off_
pool_put(&sosplice_pool, sp);
}
- s = solock(so);
/* Lock both receive and send buffer. */
if ((error = sblock(so, &so->so_rcv,
(so->so_state & SS_NBIO) ? M_NOWAIT : M_WAITOK)) != 0) {
- sounlock(s);
FRELE(fp, curproc);
return (error);
}
if ((error = sblock(so, &sosp->so_snd, M_WAITOK)) != 0) {
sbunlock(&so->so_rcv);
- sounlock(s);
FRELE(fp, curproc);
return (error);
}
@@ -1185,7 +1181,6 @@ sosplice(struct socket *so, int fd, off_
release:
sbunlock(&sosp->so_snd);
sbunlock(&so->so_rcv);
- sounlock(s);
FRELE(fp, curproc);
return (error);
}
@@ -1565,15 +1560,15 @@ sowwakeup(struct socket *so)
int
sosetopt(struct socket *so, int level, int optname, struct mbuf *m0)
{
- int s, error = 0;
+ int error = 0;
struct mbuf *m = m0;
+ soassertlocked(so);
+
if (level != SOL_SOCKET) {
if (so->so_proto && so->so_proto->pr_ctloutput) {
- s = solock(so);
error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
level, optname, m0);
- sounlock(s);
return (error);
}
error = ENOPROTOOPT;
@@ -1647,14 +1642,11 @@ sosetopt(struct socket *so, int level, i
error = EINVAL;
goto bad;
}
- s = solock(so);
if (sbcheckreserve(cnt, so->so_snd.sb_wat) ||
sbreserve(so, &so->so_snd, cnt)) {
- sounlock(s);
error = ENOBUFS;
goto bad;
}
- sounlock(s);
so->so_snd.sb_wat = cnt;
break;
@@ -1663,14 +1655,11 @@ sosetopt(struct socket *so, int level, i
error = EINVAL;
goto bad;
}
- s = solock(so);
if (sbcheckreserve(cnt, so->so_rcv.sb_wat) ||
sbreserve(so, &so->so_rcv, cnt)) {
- sounlock(s);
error = ENOBUFS;
goto bad;
}
- sounlock(s);
so->so_rcv.sb_wat = cnt;
break;
@@ -1724,10 +1713,8 @@ sosetopt(struct socket *so, int level, i
struct domain *dom = so->so_proto->pr_domain;
level = dom->dom_protosw->pr_protocol;
- s = solock(so);
error = (*so->
Re: Reducing NET_LOCK() contention
Hello, On Wed, Aug 02, 2017 at 10:10:51AM +0200, Martin Pieuchot wrote: > On 18/07/17(Tue) 15:55, Martin Pieuchot wrote: > > When forwarding a lot of traffic with 10G interfaces contention on the > > NET_LOCK() is "visible". Each time you type "ifconfig" you can go grab > > a coffee... > > > > The problem has a name: pf_purge_thread(). This thread is created by > > default and run even if you don't have PF enabled. Every `hz' it wakes > > up, grab the lock and go to sleep. > > > > Since the execution of this thread is serialized with the `softnet' task, > > it makes more sense to execute it in the same context. This reduce the > > NET_LOCK() contention and implicitly preempt the packet processing. > > > > Diff below improves the situation with PF disabled, I didn't test with > > PF enabled. > > Updated diff that includes sashan@ suggestions and do not stop the purge > task when PF is disabled. Otherwise some states are not purged until PF > is re-enabled. This can be optimized later. > thank Hrvoje for spotting the glitch. change looks good (actually better) to me. OK sashan@
Re: fstat output
On Wed, Aug 02, 2017 at 09:56:50AM +0200, Martin Pieuchot wrote:
> Simple diff to improved readability. Before:
There are situations where you want a full match on the command.
For testing and scripting with long program names this is necessary.
With a -v switch that shows the long name I would accept the patch.
netstat has that, too. And it would help in my use case.
But note that connected sockets don't fit anyway.
_ntp ntpd 190376* internet dgram udp 10.188.2.17:22899 <->
213.209.109.45:123
bluhm
>
> $ fstat -f /tmp
> USER CMD PID FD MOUNTINUM MODE R/WSZ|DV
> mpi fstat 964791 /tmp 19 -rw-r--r-- w0
> mpi ksh466571 /tmp 19 -rw-r--r-- w0
> mpi gnome-terminal-s 59648 16 /tmp 24* -rw---rw
> 262144
> mpi gnome-terminal-s 59648 20 /tmp 25* -rw---rw
> 131072
> mpi python3.6 771783 /tmp 18* -rw--- rwe 4096
> mpi seahorse-sharing 21270 wd /tmp2 drwxrwxrwt r
> 1024
> mpi gnome-shell 26656 19 /tmp 12* -rw--- rwe12288
>
>
> After:
>
> $ fstat -f /tmp
> USER CMDPID FD MOUNT INUM MODE R/WSZ|DV
> mpi fstat258001 /tmp 21 -rw-r--r-- w0
> mpi ksh 466571 /tmp 21 -rw-r--r-- w0
> mpi gnome-termin 59648 16 /tmp 24* -rw---rw 262144
> mpi gnome-termin 59648 20 /tmp 25* -rw---rw 131072
> mpi python3.6771783 /tmp 18* -rw--- rwe 4096
> mpi seahorse-sha 21270 wd /tmp2 drwxrwxrwt r 1024
> mpi gnome-shell 26656 19 /tmp 12* -rw--- rwe12288
>
>
>
> ok?
>
> Index: fstat.c
> ===
> RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
> retrieving revision 1.90
> diff -u -p -r1.90 fstat.c
> --- fstat.c 21 Jan 2017 12:21:57 - 1.90
> +++ fstat.c 2 Aug 2017 07:54:17 -
> @@ -318,10 +318,10 @@ fstat_header(void)
> {
> if (nflg)
> printf("%s",
> -"USER CMD PID FD DEV INUM MODE R/WSZ|DV");
> +"USER CMDPID FD DEV INUM MODE R/W
> SZ|DV");
> else
> printf("%s",
> -"USER CMD PID FD MOUNTINUM MODE R/W
> SZ|DV");
> +"USER CMDPID FD MOUNT INUM MODE R/W
> SZ|DV");
> if (oflg)
> printf("%s", ":OFFSET ");
> if (checkfile && fsflg == 0)
> @@ -336,7 +336,7 @@ uid_t *procuid;
> pid_tPid;
>
> #define PREFIX(i) do { \
> - printf("%-8.8s %-10s %5ld", Uname, Comm, (long)Pid); \
> + printf("%-8.8s %-12.12s %5ld", Uname, Comm, (long)Pid); \
> switch (i) { \
> case KERN_FILE_TEXT: \
> printf(" text"); \
Re: em: workaround for ultra-low-power mode on i219V
On Tue, 1 Aug 2017, Stefan Fritsch wrote: > For consumer NICs, the patch only does something on I218_LM_3/I218_V_3 or > I219* or newer. Sorry, misread the code there. There are more i218 variants that are affected by the patch.
Re: Reducing NET_LOCK() contention
On 18/07/17(Tue) 15:55, Martin Pieuchot wrote:
> When forwarding a lot of traffic with 10G interfaces contention on the
> NET_LOCK() is "visible". Each time you type "ifconfig" you can go grab
> a coffee...
>
> The problem has a name: pf_purge_thread(). This thread is created by
> default and run even if you don't have PF enabled. Every `hz' it wakes
> up, grab the lock and go to sleep.
>
> Since the execution of this thread is serialized with the `softnet' task,
> it makes more sense to execute it in the same context. This reduce the
> NET_LOCK() contention and implicitly preempt the packet processing.
>
> Diff below improves the situation with PF disabled, I didn't test with
> PF enabled.
Updated diff that includes sashan@ suggestions and do not stop the purge
task when PF is disabled. Otherwise some states are not purged until PF
is re-enabled. This can be optimized later.
ok?
Index: net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1037
diff -u -p -r1.1037 pf.c
--- net/pf.c4 Jul 2017 14:10:15 - 1.1037
+++ net/pf.c2 Aug 2017 08:08:47 -
@@ -121,6 +121,10 @@ u_char pf_tcp_secret[16];
int pf_tcp_secret_init;
int pf_tcp_iss_off;
+int pf_npurge;
+struct task pf_purge_task = TASK_INITIALIZER(pf_purge, &pf_npurge);
+struct timeout pf_purge_to = TIMEOUT_INITIALIZER(pf_purge_timeout, NULL);
+
enum pf_test_status {
PF_TEST_FAIL = -1,
PF_TEST_OK,
@@ -1200,36 +1204,43 @@ pf_purge_expired_rules(void)
}
void
-pf_purge_thread(void *v)
+pf_purge_timeout(void *unused)
{
- int nloops = 0, s;
+ task_add(softnettq, &pf_purge_task);
+}
- for (;;) {
- tsleep(pf_purge_thread, PWAIT, "pftm", 1 * hz);
+void
+pf_purge(void *xnloops)
+{
+ int *nloops = xnloops;
+ int s;
- NET_LOCK(s);
+ KERNEL_LOCK();
+ NET_LOCK(s);
- PF_LOCK();
- /* process a fraction of the state table every second */
- pf_purge_expired_states(1 + (pf_status.states
- / pf_default_rule.timeout[PFTM_INTERVAL]));
+ PF_LOCK();
+ /* process a fraction of the state table every second */
+ pf_purge_expired_states(1 + (pf_status.states
+ / pf_default_rule.timeout[PFTM_INTERVAL]));
- /* purge other expired types every PFTM_INTERVAL seconds */
- if (++nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
- pf_purge_expired_src_nodes(0);
- pf_purge_expired_rules();
- }
- PF_UNLOCK();
- /*
-* Fragments don't require PF_LOCK(), they use their own lock.
-*/
- if (nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
- pf_purge_expired_fragments();
- nloops = 0;
- }
+ /* purge other expired types every PFTM_INTERVAL seconds */
+ if (++(*nloops) >= pf_default_rule.timeout[PFTM_INTERVAL]) {
+ pf_purge_expired_src_nodes(0);
+ pf_purge_expired_rules();
+ }
+ PF_UNLOCK();
- NET_UNLOCK(s);
+ /*
+* Fragments don't require PF_LOCK(), they use their own lock.
+*/
+ if ((*nloops) >= pf_default_rule.timeout[PFTM_INTERVAL]) {
+ pf_purge_expired_fragments();
+ *nloops = 0;
}
+ NET_UNLOCK(s);
+ KERNEL_UNLOCK();
+
+ timeout_add(&pf_purge_to, 1 * hz);
}
int32_t
Index: net/pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.320
diff -u -p -r1.320 pf_ioctl.c
--- net/pf_ioctl.c 27 Jul 2017 12:09:51 - 1.320
+++ net/pf_ioctl.c 2 Aug 2017 08:01:53 -
@@ -231,16 +231,6 @@ pfattach(int num)
/* XXX do our best to avoid a conflict */
pf_status.hostid = arc4random();
-
- /* require process context to purge states, so perform in a thread */
- kthread_create_deferred(pf_thread_create, NULL);
-}
-
-void
-pf_thread_create(void *v)
-{
- if (kthread_create(pf_purge_thread, NULL, NULL, "pfpurge"))
- panic("pfpurge thread");
}
int
@@ -1027,6 +1017,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pf_status.stateid = time_second;
pf_status.stateid = pf_status.stateid << 32;
}
+ timeout_add(&pf_purge_to, 1 * hz);
pf_create_queues();
DPFPRINTF(LOG_NOTICE, "pf: started");
}
@@ -2320,7 +2311,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pf_default_rule_new.timeout[i];
if (pf_default_rule.timeou
fstat output
Simple diff to improved readability. Before:
$ fstat -f /tmp
USER CMD PID FD MOUNTINUM MODE R/WSZ|DV
mpi fstat 964791 /tmp 19 -rw-r--r-- w0
mpi ksh466571 /tmp 19 -rw-r--r-- w0
mpi gnome-terminal-s 59648 16 /tmp 24* -rw---rw
262144
mpi gnome-terminal-s 59648 20 /tmp 25* -rw---rw
131072
mpi python3.6 771783 /tmp 18* -rw--- rwe 4096
mpi seahorse-sharing 21270 wd /tmp2 drwxrwxrwt r
1024
mpi gnome-shell 26656 19 /tmp 12* -rw--- rwe12288
After:
$ fstat -f /tmp
USER CMDPID FD MOUNT INUM MODE R/WSZ|DV
mpi fstat258001 /tmp 21 -rw-r--r-- w0
mpi ksh 466571 /tmp 21 -rw-r--r-- w0
mpi gnome-termin 59648 16 /tmp 24* -rw---rw 262144
mpi gnome-termin 59648 20 /tmp 25* -rw---rw 131072
mpi python3.6771783 /tmp 18* -rw--- rwe 4096
mpi seahorse-sha 21270 wd /tmp2 drwxrwxrwt r 1024
mpi gnome-shell 26656 19 /tmp 12* -rw--- rwe12288
ok?
Index: fstat.c
===
RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
retrieving revision 1.90
diff -u -p -r1.90 fstat.c
--- fstat.c 21 Jan 2017 12:21:57 - 1.90
+++ fstat.c 2 Aug 2017 07:54:17 -
@@ -318,10 +318,10 @@ fstat_header(void)
{
if (nflg)
printf("%s",
-"USER CMD PID FD DEV INUM MODE R/WSZ|DV");
+"USER CMDPID FD DEV INUM MODE R/WSZ|DV");
else
printf("%s",
-"USER CMD PID FD MOUNTINUM MODE R/WSZ|DV");
+"USER CMDPID FD MOUNT INUM MODE R/W
SZ|DV");
if (oflg)
printf("%s", ":OFFSET ");
if (checkfile && fsflg == 0)
@@ -336,7 +336,7 @@ uid_t *procuid;
pid_t Pid;
#define PREFIX(i) do { \
- printf("%-8.8s %-10s %5ld", Uname, Comm, (long)Pid); \
+ printf("%-8.8s %-12.12s %5ld", Uname, Comm, (long)Pid); \
switch (i) { \
case KERN_FILE_TEXT: \
printf(" text"); \
