Re: svn commit: r365787 - head/sys/fs/tmpfs

[Alexey Dokuchaev]

On Tue, Jan 05, 2021 at 04:13:16PM +0100, Hans Petter Selasky wrote:
> On 1/1/21 7:44 PM, Alexey Dokuchaev wrote:
> >__mtx_lock_sleep()
> >tmpfs_free_node()
> >tmpfs_fo_close()
> >_fdrop()
> >closef()
> >fdescfree_fds()
> >fdescfree()
> >exit1()
> >sigexit()
> >postsig()
> >ast()
> >doreit_ast()
> 
> I'm seeing the same and I have a clue what is going on.
> 
> Are you absolutely sure doreit_ast() is the bottom of your stacktrace,

You can see for yourself (x3.txz for recent -current, minidump version 3,
or x4.txz for r365977 if you are confined to minidump version 2 for some
reason; the panic is the same).  Both files are at my $home at freefall.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r365787 - head/sys/fs/tmpfs

[Alexey Dokuchaev]

On Sat, Jan 02, 2021 at 06:47:14PM +0200, Konstantin Belousov wrote:
> On Sat, Jan 02, 2021 at 04:35:22PM +0000, Alexey Dokuchaev wrote:
> > On Sat, Jan 02, 2021 at 04:52:33PM +0200, Konstantin Belousov wrote:
> > > ...
> > > Ok.  So two questions:
> > > 1. Do you have core dump?
> > 
> > I have vmcore.0 and matching kernel.debug, I'll gladly obtain and provide
> > anything else you might wanna need.
> ...
> > I can upload the kernel core+debug files to freefall so you could have
> > a closer look yourself.
> 
> Lets try this.

ff:/home/danfe/x.rar

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r365787 - head/sys/fs/tmpfs

[Alexey Dokuchaev]

On Sat, Jan 02, 2021 at 04:52:33PM +0200, Konstantin Belousov wrote:
> ...
> Ok.  So two questions:
> 1. Do you have core dump?

I have vmcore.0 and matching kernel.debug, I'll gladly obtain and provide
anything else you might wanna need.

> 2. Can you look more closely what you do from the user PoV there, and
> provide a recipe to reproduce without involving 'tc tinderbiuild',
> whatever it is?

Yeah, I know that would be nice; the naive way (from the previous email
of yours) indeed doesn't trigger it, I haven't found one which does yet.

> In particular, I want to see the *node content.

I can upload the kernel core+debug files to freefall so you could have
a closer look yourself.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r365787 - head/sys/fs/tmpfs

[Alexey Dokuchaev]

On Sat, Jan 02, 2021 at 12:02:23AM +0200, Konstantin Belousov wrote:
> On Fri, Jan 01, 2021 at 06:44:00PM +0000, Alexey Dokuchaev wrote:
> > On Tue, Sep 15, 2020 at 10:19:16PM +, Konstantin Belousov wrote:
> > > New Revision: 365787
> > > URL: https://svnweb.freebsd.org/changeset/base/365787
> > > 
> > > Log:
> > >   Add tmpfs page cache read support.
> > >   
> > >   Or it could be explained as lockless (for vnode lock) reads.  Reads
> > >   are performed from the node tn_obj object.  Tmpfs regular vnode object
> > >   lifecycle is significantly different from the normal OBJT_VNODE: it is
> > >   alive as far as ref_count > 0.
> > 
> > This causes panics for me when building ports in the tmpfs-backed tinderbox.
> > Easily reproducible:
> > 
> > 1) ./tc tinderbuild ... -b "$@"
> > 2) tail -f .../tmp/make.log4 # on the adjacent console
> > 3) wait until the build job finishes
> > 4) ^C in the "tail" window -> crash
> 
> What exactly 'crash' is?

The usual "Fatal trap 12: page fault while in kernel mode" panic.

> Provide literal transcription of the kernel messages and not your
> interpretation of them.

Sorry, I've just quickly copied function names off the screen since my
debug symbols did not match the running kernel at that moment and I've
thought maybe it's a known bug that was fixed after r368820.  I've now
made things in sync so can provide full context (see below).

> >  ...
> >   __mtx_lock_sleep() at __mtx_lock_sleep+0xd2/frame 0xfe0060636490
> >   tmpfs_free_node() at tmpfs_free_node+0xc7/frame 0xfe00606364c0
> 
> What is the source line for tmpfs_free_node() frame?

/usr/src/crash-dec/sys/fs/tmpfs/tmpfs_subr.c:373

(kgdb) list *(tmpfs_free_node+0xc7)
0x815246d7 is in tmpfs_free_node 
(/usr/src/crash-dec/sys/fs/tmpfs/tmpfs_subr.c:374).
369 {
370 if (refcount_release_if_not_last(>tn_refcount))
371 return;
372 
373 TMPFS_LOCK(tmp);
374 TMPFS_NODE_LOCK(node);
375 if (!tmpfs_free_node_locked(tmp, node, false)) {
376 TMPFS_NODE_UNLOCK(node);
377 TMPFS_UNLOCK(tmp);
378 }
(kgdb)

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r365787 - head/sys/fs/tmpfs

[Alexey Dokuchaev]

On Tue, Sep 15, 2020 at 10:19:16PM +, Konstantin Belousov wrote:
> New Revision: 365787
> URL: https://svnweb.freebsd.org/changeset/base/365787
> 
> Log:
>   Add tmpfs page cache read support.
>   
>   Or it could be explained as lockless (for vnode lock) reads.  Reads
>   are performed from the node tn_obj object.  Tmpfs regular vnode object
>   lifecycle is significantly different from the normal OBJT_VNODE: it is
>   alive as far as ref_count > 0.

This causes panics for me when building ports in the tmpfs-backed tinderbox.
Easily reproducible:

1) ./tc tinderbuild ... -b "$@"
2) tail -f .../tmp/make.log4 # on the adjacent console
3) wait until the build job finishes
4) ^C in the "tail" window -> crash
 ...
  __mtx_lock_sleep()
  tmpfs_free_node()
  tmpfs_fo_close()
  _fdrop()
  closef()
  fdescfree_fds()
  fdescfree()
  exit1()
  sigexit()
  postsig()
  ast()
  doreit_ast()

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r367343 - in head/sys/contrib/openzfs/include/os: freebsd/zfs/sys linux/zfs/sys

[Alexey Dokuchaev]

On Wed, Nov 04, 2020 at 09:18:52PM +, Mateusz Guzik wrote:
> New Revision: 367343
> URL: https://svnweb.freebsd.org/changeset/base/367343
> 
> Log:
>   zfs: add branch prediction to ZFS_ENTER and ZFS_VERIFY_ZP macros
>   
>   They are expected to fail only in corner cases.
> 
> Modified:
>   head/sys/contrib/openzfs/include/os/freebsd/zfs/sys/zfs_znode_impl.h
>   head/sys/contrib/openzfs/include/os/linux/zfs/sys/zfs_znode_impl.h

Since my earlier email was ignored, I'd take this chance to ask again:
why Linux-specific bits present in the head and had to be updated like
in this commit?  They shouldn't have made past the vendor branch, no?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

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

[Alexey Dokuchaev]

On Sat, Oct 24, 2020 at 01:07:50PM +, Hans Petter Selasky wrote:
> New Revision: 366995
> URL: https://svnweb.freebsd.org/changeset/base/366995
> 
> Log:
>   More style fixes (partial revert of r366994).

Thank you Hans Petter, I appreciate it.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

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

[Alexey Dokuchaev]

On Sat, Oct 24, 2020 at 10:23:22AM +, Hans Petter Selasky wrote:
> New Revision: 366993
> URL: https://svnweb.freebsd.org/changeset/base/366993
> 
> Log:
>   Run code through "clang-format -style=file" with some additional fixes.
>   No functional change.
>   
> ...
> @@ -99,8 +97,8 @@ infiniband_ipv4_multicast_map(uint32_t addr,
>  
>  #ifdef INET6
>  static inline void
> -infiniband_ipv6_multicast_map(const struct in6_addr *addr,
> -const uint8_t *broadcast, uint8_t *buf)
> +infiniband_ipv6_multicast_map(
> +const struct in6_addr *addr, const uint8_t *broadcast, uint8_t *buf)
>  {

This is not how we format these in FreeBSD, please revert.  It was correct
before and no "fix" is need here.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r366662 - head/sbin/devmatch

[Alexey Dokuchaev]

On Tue, Oct 13, 2020 at 12:16:23AM -0600, Warner Losh wrote:
> On Mon, Oct 12, 2020, 11:50 PM Alexey Dokuchaev wrote:
> > On Tue, Oct 13, 2020 at 05:32:00AM +, Warner Losh wrote:
> > > New Revision: 32
> > > URL: https://svnweb.freebsd.org/changeset/base/32
> > >
> > > Log:
> > >   devmatch: First appeared in 12.0
> > >
> > >   Document that devmatch first appeared in FreeBSD 12.0.
> > >   Also can't -> can not. But it doesn't help the sentence much.
> > >
> > > ...
> > > @@ -62,7 +62,7 @@ Parse and use a standard NOMATCH event from
> > >  for matching instead of searching the device tree.
> > >  .It Fl u Fl -unbound
> > >  Attempt to produce a list of those drivers with PNP info whose driver
> > > -tables with that PNP info can't be found.
> > > +tables with that PNP info can not be found.
> >
> > Are you sure you've really meant "can not" and not "cannot" here?
> 
> They are variants of the same word (though it seems the one word
> variationis more common here)

No, they're not*, examples of "can not" usage:

- We can not only build the house, we can also sell it for a profit.
- Maybe I can not worry about this for a few days.

> the whole sentence is a trainwreck, though, and I should have just
> completely rewritten it.

I do agree. :-)

./danfe

*) 
https://grammar.yourdictionary.com/vs/cannot-vs-can-not-vs-cant-differences-explained.html
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r366662 - head/sbin/devmatch

[Alexey Dokuchaev]

On Tue, Oct 13, 2020 at 05:32:00AM +, Warner Losh wrote:
> New Revision: 32
> URL: https://svnweb.freebsd.org/changeset/base/32
> 
> Log:
>   devmatch: First appeared in 12.0
>   
>   Document that devmatch first appeared in FreeBSD 12.0.
>   Also can't -> can not. But it doesn't help the sentence much.
>   
> ...
> @@ -62,7 +62,7 @@ Parse and use a standard NOMATCH event from
>  for matching instead of searching the device tree.
>  .It Fl u Fl -unbound
>  Attempt to produce a list of those drivers with PNP info whose driver
> -tables with that PNP info can't be found.
> +tables with that PNP info can not be found.

Are you sure you've really meant "can not" and not "cannot" here?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r366626 - head/sbin/reboot

[Alexey Dokuchaev]

On Sun, Oct 11, 2020 at 09:17:42PM -0600, Warner Losh wrote:
> On Sun, Oct 11, 2020, 8:13 PM Alexey Dokuchaev wrote:
> > ...
> > I still don't understand how could rm be better than graceful disabling
> > alternative configuration with nextboot_enable="NO".  I most certainly
> > do *not* like when my custom config files are being removed, especially
> > silently.  When I see nextboot_enable="NO" I know that the file
> > had been processed, and processed by the machine, not me (since I would
> > never add trailing space).  When I don't see the file, I'd be questioning
> > myself if I've ever added it here, or maybe I put in the wrong location.
> 
> Nextboot.conf is special. It will be deleted. It doesn't belong to you, it
> belongs to nextboot(8).

OK, I see your point.

> > Personally I find them quite useful, except when they contradict the
> > reality (like this time).  In these cases, I'd fix them.
> 
> For now, it's fine.

Fair enough; I guess we can now wrap this discussion up, thanks!

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r366626 - head/sbin/reboot

[Alexey Dokuchaev]

On Sun, Oct 11, 2020 at 09:12:43AM -0600, Warner Losh wrote:
> ...
> There were cases that were discussed when the feature went in that
> required it to be removed in some failure modes for full functionality.
> I don't recall if they were in the rc thread or somewhere else.

You mean, literally delete the file, that is, nextboot_enable="NO" can
not be enough?

> And honestly, nextboot.conf is special in so many ways. We have no
> unlink in the loader for UFS and no write for ZFS or MSDOS. In those

What's the problem with in-place overwrite in the FAT case?

> cases, the rm from rc is what you want

I still don't understand how could rm be better than graceful disabling
alternative configuration with nextboot_enable="NO".  I most certainly
do *not* like when my custom config files are being removed, especially
silently.  When I see nextboot_enable="NO" I know that the file
had been processed, and processed by the machine, not me (since I would
never add trailing space).  When I don't see the file, I'd be questioning
myself if I've ever added it here, or maybe I put in the wrong location.

> I'm not likely to remove it, but if UFS grows unlink in the future,
> this man page will need to change.

Just because it's easier to implemented unlink for UFS then (over)write
for ZFS?

> Then again, all the loser [loader?] man pages need a complete rewrite,
> or close to it.

Personally I find them quite useful, except when they contradict the
reality (like this time).  In these cases, I'd fix them.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r366626 - head/sbin/reboot

[Alexey Dokuchaev]

On Sun, Oct 11, 2020 at 04:08:09PM +0300, Toomas Soome wrote:
> > On 11. Oct 2020, at 16:01, Alexey Dokuchaev wrote:
> >> ...
> >> Also nextboot.conf not generic configuration file (such as loader.conf
> >> or loader.conf.local), but the implementation specific file, part of
> >> special feature.
> >> 
> >> That is, one should not assume the presence of nextboot.conf file, make
> >> assumptions about its content, or perform manual edits on it.
> > 
> > Do we want it to be the second-class citizen like this?  Would it make
> > better sense by documenting it more completely instead?
> 
> It is not really about being second-class citizen, it really is about if
> and how we can implement the feature. With UFS there is a limited write
> (write to existing, allocated disk blocks), with ZFS there is no write to
> file system at all.

I see; that would explain why loader(8) replaces the "YES" -> "NO",
but I guess I'd have to read the discussion on -rc@ which lead to r177062,
because I don't see the reason for it to be removed (twice) if it's being
disabled by the loader(8) earlier anyway.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r366626 - head/sbin/reboot

[Alexey Dokuchaev]

On Sun, Oct 11, 2020 at 03:20:16PM +0300, Toomas Soome wrote:
> Please note, the remove is done by rc script during the boot.

But not by the loader(8) as the page used to claim.  It confused me how to
avoid the remove, and only later I've discovered with some relief that it
is in fact not being removed, but only disabled (which IMHO is a lot more
graceful and thus correct behavior).

> Also nextboot.conf not generic configuration file (such as loader.conf
> or loader.conf.local), but the implementation specific file, part of
> special feature.
>
> That is, one should not assume the presence of nextboot.conf file, make
> assumptions about its content, or perform manual edits on it.

Do we want it to be the second-class citizen like this?  Would it make
better sense by documenting it more completely instead?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

svn commit: r366626 - head/sbin/reboot

[Alexey Dokuchaev]

Author: danfe (ports committer)
Date: Sun Oct 11 10:40:11 2020
New Revision: 366626
URL: https://svnweb.freebsd.org/changeset/base/366626

Log:
  The nextboot(8) manual page currently says that the loader(8) would delete
  the /boot/nextboot.conf file or its contents which is 1) not the most user-
  friendly way of working with custom configurations, and 2) simply not true
  for both Forth and Lua implementations: they would not delete it, but just
  change the setting to "NO", that is, disable it.
  
  While at it, add one missing serial (Oxford) comma and fix some bogus line
  wraps along the way.
  
  Approved by:  bcr (manpages)
  Differential Revision:https://reviews.freebsd.org/D25971

Modified:
  head/sbin/reboot/nextboot.8

Modified: head/sbin/reboot/nextboot.8
==
--- head/sbin/reboot/nextboot.8 Sun Oct 11 09:05:13 2020(r366625)
+++ head/sbin/reboot/nextboot.8 Sun Oct 11 10:40:11 2020(r366626)
@@ -24,7 +24,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd September 19, 2020
+.Dd October 11, 2020
 .Dt NEXTBOOT 8
 .Os
 .Sh NAME
@@ -41,14 +41,14 @@
 .Sh DESCRIPTION
 The
 .Nm
-utility allows specifying some combination of an alternate kernel, boot flags
-and kernel environment for the
-next time the machine is booted.
+utility allows specifying some combination of an alternate kernel, boot
+flags, and kernel environment for the next time the machine is booted.
 Once the
 .Xr loader 8
-loads in the new kernel
-information, it is deleted so in case the new kernel hangs the machine,
-once it is rebooted, the machine will automatically revert to its previous
+loads in the new kernel information from the
+.Pa /boot/nextboot.conf
+file, it is disabled so in case the new kernel hangs the machine, once
+it is rebooted, the machine will automatically revert to its previous
 configuration.
 .Pp
 The options are as follows:
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r366466 - head/usr.sbin/crunch/crunchgen

[Alexey Dokuchaev]

On Mon, Oct 05, 2020 at 08:57:44PM +, Kyle Evans wrote:
> New Revision: 366466
> URL: https://svnweb.freebsd.org/changeset/base/366466
> 
> Log:
>   crunchgen: fix MK_AUTO_OBJ logic after r364166
>   
>   r364166 converted echo -n `/bin/pwd` to a raw pwd invocation, leaving a
>   trailing newline at the end of path.  This caused a later stat() of it to
>   erroneously fail and the fallback to MK_AUTO_OBJ=no logic proceeded as
>   unexpected.

[...]
@@ -648,8 +653,7 @@
 
/* Determine the actual srcdir (maybe symlinked). */
if (p->srcdir) {
-   snprintf(line, MAXLINELEN, "cd %s && echo -n `/bin/pwd`",
-   p->srcdir);
+   snprintf(line, MAXLINELEN, "cd %s && pwd", p->srcdir);
f = popen(line,"r");

Calling popen("cd somedir && pwd") in a C program to resolve symlinks,
seriously?  Why not simply call realpath(3) instead? :-/

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r365378 - head/usr.sbin/traceroute6

[Alexey Dokuchaev]

On Sun, Sep 06, 2020 at 08:32:46PM +0200, Oliver Pinter wrote:
> On Sunday, September 6, 2020, Mariusz Zaborski  wrote:
> > New Revision: 365378
> > URL: https://svnweb.freebsd.org/changeset/base/365378
> >
> > Log:
> >   traceroute6: capsicumize it
> >
> > ...
> > @@ -13,6 +13,10 @@
> >  # A PARTICULAR PURPOSE.
> >  # $FreeBSD$
> >
> > +.include 
> > +
> > +.include 
> 
> Dup

1) It's usually a good idea to read all svn-mail in your inbox before
replying to a particular commit, the problem could already be fixed;

2) Please don't overquote, i.e. do *not* include the rest of the diff
if you only wanted to comment on the first lines it.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r365071 - in head/sys: net net/altq net/route net80211 netgraph netgraph/atm netgraph/atm/ccatm netgraph/atm/sscfu netgraph/atm/sscop netgraph/atm/uni netgraph/bluetooth/common netgrap

[Alexey Dokuchaev]

On Wed, Sep 02, 2020 at 11:39:51PM +0200, Michael Tuexen wrote:
> > On 2. Sep 2020, at 20:28, Pedro Giffuni  wrote:
> > On 02/09/2020 13:06, Alexey Dokuchaev wrote:
> >> On Wed, Sep 02, 2020 at 10:18:15AM -0500, Pedro Giffuni wrote:
> >>> On 01/09/2020 21:05, Alexey Dokuchaev wrote:
> >>>> ...
> >>>> This is common sense.  I can't count how often I wanted to hack on
> >>>> something in the base/kernel and was turned away by this atrocious
> >>>> excessive whitespace mess.
> >>>> 
> >>>> Thank you Mateusz for cleaning this up.
> >>> I honestly don't care much, but spaces do no harm and can make the code
> >>> more readable. Sort of a silent comment, or what you do in written
> >>> language when you start a new paragraph.
> >> Right, but that's the example of appropriate usage of whitespace.  I was
> >> talking about *excessive* whitespace, that is, more than two \n's in a
> >> row if we speak of newlines (subject of these commits).
> > 
> > But how much space is rather subjective so Michael is right in asking
> > what rule has been violated.
> > 
> > No one is asking for the change to be reverted: the damage, if any,
> > is already done.
> 
> Just to be clear: I have NOT asked for reverting, I did not mentioned it.

Wow wow, come down people.  Indeed, no one was asking for reverting and
I don't understand why we're talking about it. :-)

> The why was explained: Some developers don't work on files which
> violate whitespace rules.

Usually they still will, but would often prefer to fix the style issues
first so they don't get in the way.  This is a bit frustrating because,
per style(9), stylistic changes are hard on the source repository (e.g.
they pessimize "svn blame") and should be avoided without good reason,
but we cannot just seat and watch how style bugs accumulate and make the
code harder and harder to work with.

> I want to understand which rules have to be followed (and why).

In general, FreeBSD code we write should follow style(9); it specifically
mentions "do not add whitespace at the end of a line" and "... followed by
one blank line" but doesn't go as far as explicitly forbidding multiple
consecutive newlines.  To me it's pretty obvious, and while others might
have different sens esthe'tique, usually it is lack thereof (no offense)
or mere ignorance.

./danfe

P.S.  Old-school tools like indent(1) or `uncrustify' were never widely
popular, I guess, because they did not possess enough knowledge of the
language to always produce correct results.  Perhaps new era tools, like
clang-format, could bring this to a whole new level.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r365071 - in head/sys: net net/altq net/route net80211 netgraph netgraph/atm netgraph/atm/ccatm netgraph/atm/sscfu netgraph/atm/sscop netgraph/atm/uni netgraph/bluetooth/common netgrap

[Alexey Dokuchaev]

On Wed, Sep 02, 2020 at 10:18:15AM -0500, Pedro Giffuni wrote:
> On 01/09/2020 21:05, Alexey Dokuchaev wrote:
> > ...
> > This is common sense.  I can't count how often I wanted to hack on
> > something in the base/kernel and was turned away by this atrocious
> > excessive whitespace mess.
> >
> > Thank you Mateusz for cleaning this up.
> 
> I honestly don't care much, but spaces do no harm and can make the code
> more readable. Sort of a silent comment, or what you do in written
> language when you start a new paragraph.

Right, but that's the example of appropriate usage of whitespace.  I was
talking about *excessive* whitespace, that is, more than two \n's in a row
if we speak of newlines (subject of these commits).

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r365071 - in head/sys: net net/altq net/route net80211 netgraph netgraph/atm netgraph/atm/ccatm netgraph/atm/sscfu netgraph/atm/sscop netgraph/atm/uni netgraph/bluetooth/common netgrap

[Alexey Dokuchaev]

On Wed, Sep 02, 2020 at 12:41:43AM +0200, Michael Tuexen wrote:
> > On 1. Sep 2020, at 23:19, Mateusz Guzik  wrote:
> > Author: mjg
> > Date: Tue Sep  1 21:19:14 2020
> > New Revision: 365071
> > URL: https://svnweb.freebsd.org/changeset/base/365071
> > 
> > Log:
> >  net: clean up empty lines in .c and .h files
> 
> Hi Mateusz,
> 
> which rules are enforced? Why?

This is common sense.  I can't count how often I wanted to hack on
something in the base/kernel and was turned away by this atrocious
excessive whitespace mess.

Thank you Mateusz for cleaning this up.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r364746 - in head: . cddl/compat/opensolaris/include cddl/contrib/opensolaris/cmd/lockstat cddl/contrib/opensolaris/cmd/zdb cddl/contrib/opensolaris/cmd/zfs cddl/contrib/opensolaris/cm

[Alexey Dokuchaev]

On Tue, Aug 25, 2020 at 02:21:27AM +, Matt Macy wrote:
> New Revision: 364746
> URL: https://svnweb.freebsd.org/changeset/base/364746
> 
> Log:
>   Merge OpenZFS support in to HEAD.
>   
>   The primary benefit is maintaining a completely shared
>   code base with the community allowing FreeBSD to receive
>   new features sooner and with less effort.

This commit raises several questions.  To start, I see a lot of things
checked in under `sys/contrib/openzfs' which do not belong to the kernel
(e.g. userland programs, libraries, manpages, bash completions, test
suite, etc/sudoers.d) or FreeBSD at all (init.d, initramfs, systemd).
Why those bits were not cleaned up, or at least kept on the vendor
branch only?

>   Improvements include:
> project quotas, encrypted datasets,
> allocation classes, vectorized raidz,
> vectorized checksums, various command line
> improvements, zstd compression.

And what about regressions?  Would illumos (opensolaris) ZFS be also
offered, maybe from ports, for those of us who prefer stable and highly
reliable implementation?

> Added: head/cddl/lib/libicp/Makefile
> ...
> +core/kcf_prov_lib.c \
> +core/kcf_callprov.c \
> +core/kcf_mech_tabs.c \
> +core/kcf_prov_tabs.c \
> +$(ASM_SOURCES_C)
> +
> +
> +
> +
> +
> +
> +SRCS= $(ASM_SOURCES_AS) $(KERNEL_C)

There are many excessive whitespace bugs, this is the most prominent.
How did they pass the review?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

svn commit: r364062 - head/usr.bin/uname

[Alexey Dokuchaev]

Author: danfe (ports committer)
Date: Mon Aug 10 09:03:29 2020
New Revision: 364062
URL: https://svnweb.freebsd.org/changeset/base/364062

Log:
  Document the order in which the kernel and the user environment versions
  are printed when both -K and -U options are passed on the command line.
  
  Approved by:  0mp
  Differential Revision:https://reviews.freebsd.org/D25970

Modified:
  head/usr.bin/uname/uname.1

Modified: head/usr.bin/uname/uname.1
==
--- head/usr.bin/uname/uname.1  Mon Aug 10 06:19:33 2020(r364061)
+++ head/usr.bin/uname/uname.1  Mon Aug 10 09:03:29 2020(r364062)
@@ -28,7 +28,7 @@
 .\"@(#)uname.1 8.3 (Berkeley) 4/8/94
 .\" $FreeBSD$
 .\"
-.Dd June 27, 2019
+.Dd August 10, 2020
 .Dt UNAME 1
 .Os
 .Sh NAME
@@ -107,6 +107,9 @@ and
 flags are intended to be used for fine grain differentiation of incremental
 .Fx
 development and user visible changes.
+Note that when both of these two options are specified, regardless of their
+order, the kernel version would be printed first, followed by the user
+environment version.
 .Sh ENVIRONMENT
 An environment variable composed of the string
 .Ev UNAME_
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r363091 - in head/contrib/bc: . include manuals src tests tests/bc

[Alexey Dokuchaev]

On Thu, Jul 30, 2020 at 06:51:58PM +0200, Stefan E??er wrote:
> ...
> My guess is that he'll add a compile time switch to his version
> that will be added to our usr.bin Makefile and will suppress the
> start-up message in our base system version. (I'll keep the port
> version as is unless there is a strong preference to suppress it
> also in the port).

Yes, this is the correct approach, thank you Stefan.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362998 - in head: stand/defaults sys/amd64/amd64 sys/powerpc/aim sys/vm

[Alexey Dokuchaev]

On Tue, Jul 07, 2020 at 08:33:12PM +, Scott Long wrote:
> New Revision: 362998
> URL: https://svnweb.freebsd.org/changeset/base/362998
> 
> Log:
>   Migrate the feature of excluding RAM pages to use "excludelist"
>   as its nomenclature.

Did I miss any prior discussion of this change?  Commit references no
DR or something.  I thought we had agreed that this black/master type
of commits should not just go in like one pleases.

To me this change is pessimization, it is technically less accurate.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362809 - head/contrib/mandoc

[Alexey Dokuchaev]

On Wed, Jul 01, 2020 at 05:01:00PM -0700, Rodney W. Grimes wrote:
> Thats good, but realize the page already contains history that
> reads like:
> 
> HISTORY
>  Part of the functionality of whatis was already provided by the former
>  manwhere utility in 1BSD. The apropos and whatis utilities first ap-
>  peared in 2BSD.  They were rewritten from scratch for OpenBSD 5.6.
> 
>  The -M option and the MANPATH variable first appeared in 4.3BSD; -m in
>  4.3BSD-Reno; -C in 4.4BSD Lite1; and -S and -s in OpenBSD 4.5 for apropos
>  and in OpenBSD 5.6 for whatis.  The options -acfhIKklOTWw appeared in
>  OpenBSD 5.7.
> 
> And further contains:
> 
> AUTHORS
>  Bill Joy wrote manwhere in 1977 and the original BSD apropos and whatis
>  in February 1979. The current version was written by Kristaps Dzonsons
>   and Ingo Schwarze .
> 
> So the history is rich and complete, do we really need to say when we
> incorporated this into FreeBSD from OpenBSD's mandoc in the manual page?

Ah, in this case, the only thing lacking from the current version is mention
of FreeBSD 11.1.  Sorry for not checking with that before writing my reply.
My main point, however, was that reverse chronological order looks strange.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362809 - head/contrib/mandoc

[Alexey Dokuchaev]

On Wed, Jul 01, 2020 at 11:28:47PM +0200, Gordon Bergling wrote:
> On Wed, Jul 01, 2020 at 11:58:05AM -0600, Warner Losh wrote:
> > On Wed, Jul 1, 2020 at 8:51 AM Rodney W. Grimes wrote:
> > > ...
> > > We often have "An ls command appeared in Version 1 AT UNIX."  Our source
> > > code and man page is not from that, but that is the history of ls.
> > >
> > > This *could* be amended and *should* be amended to reflect that apropos,
> > > and makewhatis got *updated* by a switch to the mandoc versions, but it
> > > is misleading to say it was intergrated with the switch to mandoc as that
> > > implies it did not exist before this action.
> > 
> > I tend to agree with Rod here. These appeared in X the first time, but
> > noting they were replaced in version X with Y is the best way to address
> > the current provenance of the code...
> 
> OK, I see your arguments. How about the following addition for HISTORY 
> section,
> 
> The apropos utility was integrated into FreeBSD 11.1 as part of the
> switch to mandoc. Before the switch to mandoc apropos was available since
> FreeBSD 1.0.

It should be the other way around:

  "The apropos utility appeared in FreeBSD 1.0.  Since FreeBSD 11.1
   it is based on mandoc implementation."

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362667 - head/sys/cddl/contrib/opensolaris/uts/common/dtrace

[Alexey Dokuchaev]

On Sat, Jun 27, 2020 at 12:57:48AM +, Matt Macy wrote:
> New Revision: 362667
> URL: https://svnweb.freebsd.org/changeset/base/362667
> 
> Log:
>   Fix "current" variable name conflict with openzfs
>   
>   The variable "current" is an alias for curthread
>   in openzfs. Rename all variable uses of current
>   in dtrace.c to curstate.

Shouldn't OpenZFS be fixed instead?  The fact that we need to fix our
local variables suggests that they're abusing too generic name...

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362488 - in head: contrib/file/magic/Magdir contrib/tcpdump lib/geom/part stand/efi/include stand/i386/boot0 sys/dev/hptmv sys/geom/part usr.bin/fortune/datfiles usr.bin/mkimg usr.sbi

[Alexey Dokuchaev]

On Mon, Jun 22, 2020 at 07:46:25AM +, Baptiste Daroussin wrote:
> New Revision: 362488
> URL: https://svnweb.freebsd.org/changeset/base/362488
> 
> Log:
>   Revert r362466
>   
>   Such change should not have happen without prior discussion and review.

Thank you Baptiste.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362466 - in head: contrib/file/magic/Magdir contrib/tcpdump lib/geom/part stand/efi/include stand/i386/boot0 sys/dev/hptmv sys/geom/part usr.bin/fortune/datfiles usr.bin/mkimg usr.sbi

[Alexey Dokuchaev]

On Sun, Jun 21, 2020 at 06:01:40PM +0200, Hans Petter Selasky wrote:
> On 2020-06-21 17:53, Alexandr Kovalenko wrote:
> > I wonder should we also rename Master's Degree? Would you throw
> > away yours?
> > https://en.wiktionary.org/wiki/master
> 
> I don't see a problem if you have a main computer degree.

*facepalm*

> Actually you have a very good point. Didn't think of that yet. I
> should be more proud of my Bachelor's than my Masters's apparently.

Hans, if you wanted to make reductio ad absurdum, that point is seen.
Now, please revert the commit.  We are *already losing developers*
because of all this shit and it must be stopped.  This is definitely
not the right time to make such changes in such manner, leaving alone
the need for the changes per se.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362466 - in head: contrib/file/magic/Magdir contrib/tcpdump lib/geom/part stand/efi/include stand/i386/boot0 sys/dev/hptmv sys/geom/part usr.bin/fortune/datfiles usr.bin/mkimg usr.sbi

[Alexey Dokuchaev]

On Sun, Jun 21, 2020 at 04:09:15PM +0200, Hans Petter Selasky wrote:
> On 2020-06-21 16:02, Alexey Dokuchaev wrote:
> > Last edited by Hselasky (talk | contribs) 0 seconds ago.
> > 
> > Ah, so you first make changes to FreeBSD and then attempt to edit the
> > Wikipedia to look like it supports your views?  Seriously Hans?!
> 
> This is a chicken an egg problem. It was easier to start with FreeBSD
> and the follow up on Wikipedia.

There is no chicken and egg problem here Hans.  You're inventing bogus
and unsound terminology that does no good and only confuses our users,
code readers, and makes us as a project look like idiots.

Several people had asked you to revert the commit.  It was not properly
reviewed, it solves no existing problem, but creates one instead.

Please don't "fix" what ain't broken.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362466 - in head: contrib/file/magic/Magdir contrib/tcpdump lib/geom/part stand/efi/include stand/i386/boot0 sys/dev/hptmv sys/geom/part usr.bin/fortune/datfiles usr.bin/mkimg usr.sbi

[Alexey Dokuchaev]

On Sun, Jun 21, 2020 at 01:59:12PM +, Alexey Dokuchaev wrote:
> On Sun, Jun 21, 2020 at 03:48:33PM +0200, Emmanuel Vadot wrote:
> > On Sun, 21 Jun 2020 15:46:56 +0200
> > Hans Petter Selasky  wrote:
> > > On 2020-06-21 15:45, Alexey Dokuchaev wrote:
> > > >> ...
> > > >> -#https://en.wikipedia.org/wiki/Master_boot_record#PTE
> > > >> +#https://en.wikipedia.org/wiki/Main_boot_record#PTE
> > > > Wikipedia does not have an article with this name.
> > > 
> > > Yes, it does:
> > > https://en.wikipedia.org/wiki/Main_boot_record
> > > 
> > > No, need to revert.
> > 
> > So because some random person on the net created a redirection MBR is
> > now defined at Main Boot Record ? WFT
> 
> There's no redirection; this article simply does not exist (well, did
> not exist several minutes ago as apparently someone had added a draft
> for it just now).

Last edited by Hselasky (talk | contribs) 0 seconds ago.

Ah, so you first make changes to FreeBSD and then attempt to edit the
Wikipedia to look like it supports your views?  Seriously Hans?!

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362466 - in head: contrib/file/magic/Magdir contrib/tcpdump lib/geom/part stand/efi/include stand/i386/boot0 sys/dev/hptmv sys/geom/part usr.bin/fortune/datfiles usr.bin/mkimg usr.sbi

[Alexey Dokuchaev]

On Sun, Jun 21, 2020 at 03:48:33PM +0200, Emmanuel Vadot wrote:
> On Sun, 21 Jun 2020 15:46:56 +0200
> Hans Petter Selasky  wrote:
> > On 2020-06-21 15:45, Alexey Dokuchaev wrote:
> > >> ...
> > >> -#https://en.wikipedia.org/wiki/Master_boot_record#PTE
> > >> +#https://en.wikipedia.org/wiki/Main_boot_record#PTE
> > > Wikipedia does not have an article with this name.
> > 
> > Yes, it does:
> > https://en.wikipedia.org/wiki/Main_boot_record
> > 
> > No, need to revert.
> 
> So because some random person on the net created a redirection MBR is
> now defined at Main Boot Record ? WFT

There's no redirection; this article simply does not exist (well, did
not exist several minutes ago as apparently someone had added a draft
for it just now).

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362466 - in head: contrib/file/magic/Magdir contrib/tcpdump lib/geom/part stand/efi/include stand/i386/boot0 sys/dev/hptmv sys/geom/part usr.bin/fortune/datfiles usr.bin/mkimg usr.sbi

[Alexey Dokuchaev]

On Sun, Jun 21, 2020 at 03:46:56PM +0200, Hans Petter Selasky wrote:
> On 2020-06-21 15:45, Alexey Dokuchaev wrote:
> >> ...
> >> -#https://en.wikipedia.org/wiki/Master_boot_record#PTE
> >> +#https://en.wikipedia.org/wiki/Main_boot_record#PTE
> > Wikipedia does not have an article with this name.
> 
> Yes, it does:
> https://en.wikipedia.org/wiki/Main_boot_record

No, it doesn't.  The fast that it does not return 404 does not
mean it exists, this is how wikis work.  Please read the contents.

> No, need to revert.

Just as we managed to cool down this shitstorm of controversial
commits hitting the tree undiscussed, you're pouring more oil to
the flames.  This has to stop.  Again, this commit must be reverted
and DR opened with proposed changes if you feel they're warranted,
because IMHO they are not: as with "master volume", MBR has its
well defined meaning, there's no "slave boot record" and hence this
change was totally uncalled for.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362466 - in head: contrib/file/magic/Magdir contrib/tcpdump lib/geom/part stand/efi/include stand/i386/boot0 sys/dev/hptmv sys/geom/part usr.bin/fortune/datfiles usr.bin/mkimg usr.sbi

[Alexey Dokuchaev]

On Sun, Jun 21, 2020 at 01:34:09PM +, Hans Petter Selasky wrote:
> New Revision: 362466
> URL: https://svnweb.freebsd.org/changeset/base/362466
> 
> Log:
>   Improve wording to be more precise and clear.
>   No functional change intended.

Please revert.

> ...
> -# https://en.wikipedia.org/wiki/Master_boot_record#PTE
> +# https://en.wikipedia.org/wiki/Main_boot_record#PTE

Wikipedia does not have an article with this name.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362444 - head/sbin/dump

[Alexey Dokuchaev]

On Sat, Jun 20, 2020 at 03:03:15PM -0600, Warner Losh wrote:
> On Sat, Jun 20, 2020 at 2:19 PM Alexey Dokuchaev wrote:
> > ...
> > Please, just open a DR for that so all interested parties can participate
> > and fine-tune particular grammar and language choices.  Also, r362447
> > should be reverted on the same grounds as r362422.
> 
> I'd have rather r362447 go through review as well, but really, it's fine
> enough for now that it's not worth the churn to back it out.

Agreed; I've basically said the same in the follow-up email.  If we could
fix the "enworker" then it should be as technically correct yet neutral as
possible, but even now it's an improvement.  So is r362449 (notice how no
one had objected to it).

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362444 - head/sbin/dump

[Alexey Dokuchaev]

On Sat, Jun 20, 2020 at 08:19:35PM +, Alexey Dokuchaev wrote:
> ...
> Please, just open a DR for that so all interested parties can participate
> and fine-tune particular grammar and language choices.  Also, r362447
> should be reverted on the same grounds as r362422.

Perhaps full backout is not necessary as r362447 looks mostly okay, but
it would be nice to do something about enworker().

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362444 - head/sbin/dump

[Alexey Dokuchaev]

On Sat, Jun 20, 2020 at 08:12:40PM +, Colin Percival wrote:
> Thanks for backing this out, Warner.

I also appreciate it.

> I'd like to change "slave" to "worker" here (which I think is a reasonably
> neutral and entirely inoffensive term), and in the process perhaps make some
> associated grammatical changes (since "enworker" is dubious at best).
> 
> To avoid causing any further issues: If anyone objects to the word "worker"
> please let me know in the next ~48 hours.  I think there's enough people
> reading svn-src-all that I can anticipate feedback now if anyone will care
> deeply about that word.

Please, just open a DR for that so all interested parties can participate
and fine-tune particular grammar and language choices.  Also, r362447
should be reverted on the same grounds as r362422.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362420 - head/share/misc

[Alexey Dokuchaev]

On Sat, Jun 20, 2020 at 04:07:44AM +, Warner Losh wrote:
> New Revision: 362420
> URL: https://svnweb.freebsd.org/changeset/base/362420
> 
> Log:
>   Correct 1BSD release date.
>   
>   The Quarter Century of Unix book said that 1BSD was released March 1979.
>   However, the 1BSD tape image that's on Kirk's historical unix collection
>   has an earlier date.
>   
>   It was common practice, at the time, to create a new copy of the tape
>   from the master system
 ^^
Ouch! :-)

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362399 - head/sys/dev/dwc

[Alexey Dokuchaev]

On Fri, Jun 19, 2020 at 06:04:41PM +, Michal Meloun wrote:
> New Revision: 362399
> URL: https://svnweb.freebsd.org/changeset/base/362399
> 
> Log:
>   Use naming nomenclature used in DesignWare TRM.
>   Use naming nomenclature used in DesignWare TRM.

Please trim repeated lines from the commit logs.  It breaks reading
prefetch and thus makes it rather annoying.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r361791 - head/etc/mtree

[Alexey Dokuchaev]

On Thu, Jun 04, 2020 at 09:19:35AM -0700, Cy Schubert wrote:
> In message <202006041604.054g4kab098...@repo.freebsd.org>, Conrad Meyer
> writes:
> > New Revision: 361791
> > URL: https://svnweb.freebsd.org/changeset/base/361791
> >
> > Log:
> >   Restrict default /root permissions
> >   
> > ...
> > @@ -117,7 +117,7 @@
> >  ..
> >  rescue
> >  ..
> > -root
> > +rootmode=0750
> >  ..
> 
> Recent CIS benchmarks recommend 0700.

Please, let's keep a reasonable balance between security and usability.
I often visit /root as a regular user (wheel'ed), and 0700 would make
it real PITA.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r360025 - head/sys/dev/acpica

[Alexey Dokuchaev]

On Fri, Apr 17, 2020 at 08:54:52AM -0700, Colin Percival wrote:
> On 2020-04-16 23:26, Alexey Dokuchaev wrote:
> > On Thu, Apr 16, 2020 at 03:12:15PM -0700, Colin Percival wrote:
> >> ...
> >> I considered that, but so far I don't have any evidence that other
> >> systems exist with the same half-working ACPI.  Unless you're aware of
> >> people who are seeing the same "hw.apci.video.lcd0.brightness changes
> >> but thebacklight doesn't" syndrome?
> > 
> > So this is more like a work-around for the problem which lies elsewhere:
> > something is missing in acpi_video(4) or perhaps we need a (nonexistent)
> > acpi_dell(4)?  In this case, shouldn't it be disabled by default, or at
> > least be toggable since as you say, evidently just a minority of systems
> > exist with the same half-working ACPI?
> 
> Having occasional extra messages coming through devctl(4) is harmless; by
> default devd will just drop them on the floor since there isn't anything
> configured to do with them.
> 
> In an upcoming change to the graphics/intel-backlight port (if/when someone
> merges my patch into their github repo...) I'll be providing a configuration
> file which can be copied into /usr/local/etc/devd/ in order to hook these
> messages up to the intel-backlight utility.
> 
> So... yes, for practical purposes this is disabled by default.

Understood, thanks Colin!

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r360025 - head/sys/dev/acpica

[Alexey Dokuchaev]

On Thu, Apr 16, 2020 at 03:12:15PM -0700, Colin Percival wrote:
> On 2020-04-16 15:05, Oliver Pinter wrote:
> > On Thursday, April 16, 2020, Colin Percival  > > wrote:
> > Log:
> >   Alert devd when acpi_video brightness changes
> > 
> > Please add this to release notes!
> 
> I considered that, but so far I don't have any evidence that other
> systems exist with the same half-working ACPI.  Unless you're aware of
> people who are seeing the same "hw.apci.video.lcd0.brightness changes
> but thebacklight doesn't" syndrome?

So this is more like a work-around for the problem which lies elsewhere:
something is missing in acpi_video(4) or perhaps we need a (nonexistent)
acpi_dell(4)?  In this case, shouldn't it be disabled by default, or at
least be toggable since as you say, evidently just a minority of systems
exist with the same half-working ACPI?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r359374 - in head: . share/man/man4 share/man/man7 share/man/man9 sys/crypto/aesni sys/crypto/armv8 sys/crypto/blake2 sys/crypto/ccp sys/crypto/via sys/dev/cesa sys/dev/cxgbe sys/dev/c

[Alexey Dokuchaev]

On Sun, Apr 12, 2020 at 01:08:49PM -0700, Xin Li via svn-src-all wrote:
> On 3/27/20 11:25 AM, John Baldwin wrote:
> >   - Drivers no longer register a list of supported algorithms. [...]
> 
> For user-visible interface, it seems like we are essentially treating
> "accelerated software" like AES-NI the same way of plain software.  For
> example, geom_eli would now say:
> 
> GEOM_ELI: Encryption: AES-XTS 128
> GEOM_ELI: Crypto: software
> 
> Instead of:
> 
> GEOM_ELI: Encryption: AES-XTS 128
> GEOM_ELI: Crypto: hardware
> 
> [...] and it's much easier for system administrators if we expose the
> fact that they are using some kind of acceleration than asking them to
> run DTrace etc. to find out.  Personally, I think it's probably better
> to change the notion to either "accelerated" (by either hardware or
> software) and "software"...

+1 for "accelerated" vs. "software".  For most users "accelerated" would
mean AES-NI (dedicated crypto cards are not as common), and since most
modern CPUs have those, not being able to easily distinguish between that
and "pure software" (unaccelerated) mode is rather frustrating.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r359459 - head/share/man/man5

[Alexey Dokuchaev]

On Mon, Mar 30, 2020 at 07:39:39PM -0400, Ed Maste wrote:
> On Mon, 30 Mar 2020 at 19:03, John Baldwin  wrote:
> >
> > and I haven't seen the commit to
> > actually remove GDB_LIBEXEC yet?
> 
> I haven't seen the commit mail for r359457 yet either, but the change
> is committed.

As of recently, commit mail gets significantly delayed for some reason,
per what I've also observed.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r358989 - in head/stand/efi: libefi loader loader/arch/arm loader/arch/arm64

[Alexey Dokuchaev]

On Sat, Mar 14, 2020 at 06:36:03AM +, Toomas Soome wrote:
> New Revision: 358989
> URL: https://svnweb.freebsd.org/changeset/base/358989
> 
> Log:
>   loader: add comconsole implementation on top of SIO protocol
> 
> [...]
> --- /dev/null 00:00:00 1970   (empty, because file is newly added)
> +++ head/stand/efi/loader/efiserialio.c   Sat Mar 14 06:36:03 2020
> (r358989)
> @@ -0,0 +1,518 @@
> +/*-
> + * Copyright (c) 1998 Michael Smith (msm...@freebsd.org)

If this is the new code, then copyright date and maybe attribution
seem wrong.  If this is the old code, why wasn't it repocopied?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r359260 - in head: contrib/kyua etc/mtree lib lib/kyua lib/kyua/cli lib/kyua/drivers lib/kyua/engine lib/kyua/model lib/kyua/store lib/kyua/utils share/mk tools/build/mk usr.bin usr.bi

[Alexey Dokuchaev]

On Mon, Mar 23, 2020 at 07:01:24PM +, Brooks Davis wrote:
> New Revision: 359260
> URL: https://svnweb.freebsd.org/changeset/base/359260
> 
> Log:
>   Import the kyua test framework.
>   
>   Having kyua in the base system will simplify automated testing in CI
>   and eliminates bootstrapping issues on new platforms.
>   
>   The build of kyua is controlled by WITH(OUT)_TESTS_SUPPORT.

What's WITH(OUT)_TESTS_SUPPORT relationship with MK_TESTS?  I often put
MK_TESTS=no in /etc/make.conf on space-limited machines, do I also have
to add WITHOUT_TESTS_SUPPORT there now?  Perhaps it makes sense to fold
two knobs into one?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

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

[Alexey Dokuchaev]

On Thu, Jan 30, 2020 at 08:05:05PM +, Mateusz Guzik wrote:
> New Revision: 357312
> URL: https://svnweb.freebsd.org/changeset/base/357312
> 
> Log:
>   Remove duplicated empty lines from kern/*.c

Thank you.  Every once in a while when I want/need to hack on src code base,
these superfluous newlines drive me away. :-(

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r357028 - head/sys/vm

[Alexey Dokuchaev]

On Wed, Jan 22, 2020 at 07:33:00PM -1000, Jeff Roberson wrote:
> You may be asking yourself, why did jeff just squish a bunch of code
> around with little functional change.  I could not justify adding more
> complexity to a 900 line function with a handful of gotos.  It is ~300
> lines now.  I tested the whole set together.  I apologize if there are
> bugs.

I'm quite certain folks do appreciate the good work Jeff.  "Reviewed by:
dougm, kib, markj" lines leave very little room for doubt. :-)

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

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

[Alexey Dokuchaev]

On Tue, Jan 21, 2020 at 12:33:11AM +0700, Eugene Grosbein wrote:
> 21.01.2020 0:22, Alexey Dokuchaev wrote:
> > On Fri, Jan 17, 2020 at 08:17:11PM +0700, Eugene Grosbein wrote:
> >> ...
> >> It was fine before clang-[78]. Buy nowadays, yes, I'm forced to use
> >> multiple knobs for src.conf
> >>
> >> WITHOUT_LLVM_TARGET_ALL=
> >> WITH_LLVM_TARGET_X86=
> >> WITHOUT_CLANG_FULL=
> > 
> > Thanks, I've added them (with =yes part) to my /etc/src.conf as well.
> > I've noticed I also have WITHOUT_CLANG_EXTRAS=yes there, is this one
> > still needed/does any good?
> 
> Take a look at src/usr.bin/clang/Makefile, it seems WITH_CLANG_EXTRAS
> enables building multiple utilities like llvm versions of ar, as, nm,
> objcopy, objdump etc.

Hmm, I do have those (under /usr/bin) and they don't seem like being
*extras* but rather essential part of the toolchain.  I'll take a closer
look, perhaps I've added WITHOUT_CLANG_EXTRAS=yes *after* my last build
of the world, thanks.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

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

[Alexey Dokuchaev]

On Fri, Jan 17, 2020 at 08:17:11PM +0700, Eugene Grosbein wrote:
> ...
> It was fine before clang-[78]. Buy nowadays, yes, I'm forced to use
> multiple knobs for src.conf
> 
> WITHOUT_LLVM_TARGET_ALL=
> WITH_LLVM_TARGET_X86=
> WITHOUT_CLANG_FULL=

Thanks, I've added them (with =yes part) to my /etc/src.conf as well.
I've noticed I also have WITHOUT_CLANG_EXTRAS=yes there, is this one
still needed/does any good?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r355978 - head/usr.bin/top

[Alexey Dokuchaev]

On Wed, Jan 15, 2020 at 07:09:58PM +0100, T??l Coosemans wrote:
> On Sat, 21 Dec 2019 05:03:21 + (UTC) Philip Paeps wrote:
> > New Revision: 355978
> > URL: https://svnweb.freebsd.org/changeset/base/355978
> > 
> > Log:
> >   top: display battery capacity remaining
> >   
> >   Submitted by: Antranig Vartanian 
> >   Reviewed by:  imp, philip
> >   Differential Revision:https://reviews.freebsd.org/D22871
> > 
> > @@ -1322,6 +1322,15 @@ i_uptime(struct timeval *bt, time_t *tod)
> >  }
> >  }
> >  
> > +void
> > +i_battery(int nbat, int batt)
> > +{
> > +
> > +   if (nbat > 0) {
> > +   printf("; battery: %d%%", batt);
> 
> It doesn't fit.  There's only room for "; b":
> 
> last pid: 33047;  load averages:  1.17,  1.25,  1.34; b up 3+07:35:37 ...

TBH, I don't think it belongs in that line in the first place (leaving
aside the argument whether it belongs to the top(1) at all).  At least
it should probably be printed on its own line, with potential other
power/green-related metrics, and hidden by default.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r356185 - in head: lib/geom lib/geom/sched sys/geom sys/geom/sched sys/modules/geom sys/modules/geom/geom_sched sys/sys

[Alexey Dokuchaev]

On Mon, Dec 30, 2019 at 02:55:16PM -0500, Alexander Motin wrote:
> On 30.12.2019 12:02, Alexey Dokuchaev wrote:
> > ...
> > Well, hard drives essentially didn't change since then, still being
> > the same rotation media. :)
> 
> At least some papers about gsched I read mention adX devices, which
> means old ATA stack and no NCQ.  It can be quite a significant change to
> let HDD to do its own scheduling.  Also about a year ago in r335066
> Warner added sysctl debug.bioq_batchsize, which if set to non-zero value
> may, I think, improve fairness between several processes, just not sure
> why it was never enabled.

Thanks for the pointer, I'll remember to play with it.

> > Admittedly, I've only did some tests no later than in 8.4 times when I
> > first started using it.  Fair point, though, I should redo them again.
> 
> I'm sorry to create a regression for you, if there is really one.  As I
> have written I don't have so much against the scheduler part itself, as
> against the accumulated technical debt and the way integration is done,
> such as mechanism of live insertion, etc.  Without unmapped I/O and
> direct dispatch I bet it must be quite slow on bigger systems, that is
> why I doubted anybody really use it.

It's OK, no need to be sorry, I understand the reasons behind the move
and it certainly looks justified enough.

> > Is there a planned replacement, or I'd better create a port for the
> > GEOM_SCHED class and gsched(8) tool?
> 
> I wasn't planning replacement.  And moving it to ports would be a
> problem, since in process I removed few capabilities critical for it:
> nstart/nend for live insertion and BIO classification for scheduling.
> But the last I don't mind to return if there appear to be a need.  It
> is only the first I am strongly against.  But if somebody would like to
> reimplement it, may be it would be better to consider merging it with
> CAM I/O scheduler by Warner?  The one at least knows about device queue
> depth, etc.  We could return the BIO classification to be used by CAM
> scheduler instead, if needed.

Make sense; thanks everyone who replied in the thread, it was quite
fruitful.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r356185 - in head: lib/geom lib/geom/sched sys/geom sys/geom/sched sys/modules/geom sys/modules/geom/geom_sched sys/sys

[Alexey Dokuchaev]

On Mon, Dec 30, 2019 at 08:55:14AM -0700, Warner Losh wrote:
> On Mon, Dec 30, 2019, 5:32 AM Alexey Dokuchaev wrote:
> > On Sun, Dec 29, 2019 at 09:16:04PM +, Alexander Motin wrote:
> > > New Revision: 356185
> > > URL: https://svnweb.freebsd.org/changeset/base/356185
> > >
> > > Log:
> > >   Remove GEOM_SCHED class and gsched tool.
> > >   [...]
> >
> > Wow, that was unexpected, I use it on all my machines' HDD drives.
> > Is there a planned replacement, or I'd better create a port for the
> > GEOM_SCHED class and gsched(8) tool?
> 
> How much of a performance improvement do you see with it?
> 
> There has been no tweaks to this geom in years and years. It was tuned
> to 10 year old hard drives and never retuned for anything newer.

Well, hard drives essentially didn't change since then, still being the
same roration media. :)

> And when I played with it a few years ago, I saw no improvements...

Admittedly, I've only did some tests no later than in 8.4 times when I
first started using it.  Fair point, though, I should redo them again.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r356185 - in head: lib/geom lib/geom/sched sys/geom sys/geom/sched sys/modules/geom sys/modules/geom/geom_sched sys/sys

[Alexey Dokuchaev]

On Sun, Dec 29, 2019 at 09:16:04PM +, Alexander Motin wrote:
> New Revision: 356185
> URL: https://svnweb.freebsd.org/changeset/base/356185
> 
> Log:
>   Remove GEOM_SCHED class and gsched tool.
>   
>   This code was not actively maintained since it was introduced 10 years ago.
>   It lacks support for many later GEOM features, such as direct dispatch,
>   unmapped I/O, stripesize/stripeoffset, resize, etc.  Plus it is the only
>   remaining use of GEOM nstart/nend request counters, used there to implement
>   live insertion/removal, questionable by itself.

Wow, that was unexpected, I use it on all my machines' HDD drives.
Is there a planned replacement, or I'd better create a port for the
GEOM_SCHED class and gsched(8) tool?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r355600 - in head: share/man/man9 sys/kern sys/sys

[Alexey Dokuchaev]

On Tue, Dec 10, 2019 at 11:09:41PM +0100, Hans Petter Selasky wrote:
> On 2019-12-10 22:58, John Baldwin wrote:
> >   While here, add  to the manpage.
> 
> FYI:
> 
> Linux guys eliminated the "void *c_arg" in their timer implementation by
> using container_of() to get callback argument. We could possibly do the
> same!

#define container_of(ptr, type, member) ({  \
void *__mptr = (void *)(ptr);   \
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
 !__same_type(*(ptr), void),\
 "pointer type mismatch in container_of()");\
((type *)(__mptr - offsetof(type, member))); })

That's one of those things that make Linux so unpleasant to work with.
Frankly, I don't think we want that for FreeBSD.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r354922 - in head: etc libexec/rc libexec/rc/rc.d share/man/man5 sys/sys

[Alexey Dokuchaev]

On Wed, Nov 20, 2019 at 11:45:31PM +, Warner Losh wrote:
> New Revision: 354922
> URL: https://svnweb.freebsd.org/changeset/base/354922
> 
> Log:
>   Create /etc/os-release file.
>   
>   Each boot, regenerate /var/run/os-release based on the currently
>   running system. Create a /etc/os-release symlink pointing to this
>   file (so that this doesn't create a new reason /etc can not be
>   mounted read-only).
>   
>   This is compatible with what other systems do and is what the
>   sysutil/os-release port attempted to do, but in an incomplete way.

Thanks!  Hopefully, `sysutil/os-release' can be wiped out soon: not
just it was placed under wrong category, but it was badly and quite
unreadably coded.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r354824 - head/lib/geom/part

[Alexey Dokuchaev]

On Mon, Nov 18, 2019 at 01:09:19PM -0700, Alan Somers wrote:
> On Mon, Nov 18, 2019 at 12:48 PM Mark Johnston  wrote:
> > On Mon, Nov 18, 2019 at 12:43:48PM -0700, Alan Somers wrote:
> > > On Mon, Nov 18, 2019 at 12:06 PM Mark Johnston 
> > wrote:
> > >
> > > > Author: markj
> > > > Date: Mon Nov 18 19:05:52 2019
> > > > New Revision: 354824
> > > > URL: https://svnweb.freebsd.org/changeset/base/354824
> > > >
> > > > Log:
> > > >   Fix grammar in gpart.8.
> > > >
> > > >   PR:   241973
> > > >   MFC after:3 days
> > > >
> > > > Modified:
> > > >   head/lib/geom/part/gpart.8
> > >
> > > Don't forget to bump the .Dd date.
> >
> > AFAIK we do not bump .Dd for such changes, only for content changes.
> 
> Isn't that a content change?  I thought anything displayed to the user
> counts as a content change?

I presume that "content" means more like semantic, functional changes
that affect some particular meaning.  Grammar fixes are just, well,
grammar fixes and thus do not warrant .Dd bump.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r353937 - in head/share: man/man5 mk

[Alexey Dokuchaev]

On Tue, Oct 29, 2019 at 08:04:07AM +0100, Dimitry Andric wrote:
> On 27 Oct 2019, at 17:59, Alexey Dokuchaev  wrote:
> > 
> > On Sat, Oct 26, 2019 at 04:34:14PM +0200, Dimitry Andric wrote:
> ...
> >> I only tested -j24 on a 32-core system, but I could probably repeat the
> >> experiment with lower and higher -j values: [...]
> >> 
> >> So ~2.3% difference in real time, which is not too bad I think.
> > 
> > Well, I'd say it's acceptable. :-/
> 
> I also tested at low (-j4) and high (-j32) levels.  It turns out that at
> low -j levels, the difference is less pronounced, just ~1.1% in real
> time.  And at high -j levels, it is more pronounced, ~4.3% in real time.
> 
> Note also that at high -j levels, the difference in system time seems
> to get more influence, e.g. at low -j the difference is ~4.5%, while at
> high -j the difference is ~7.2%.  I guess it is because dynamic linking
> uses more syscalls.

Yeah, it would be definitely nice if we could optimize runtime linker.
Thanks for conducting these tests by the way, much appreciated, Dimitry!

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r353937 - in head/share: man/man5 mk

[Alexey Dokuchaev]

On Sat, Oct 26, 2019 at 04:34:14PM +0200, Dimitry Andric wrote:
> > On 24 Oct 2019, at 14:49, Alexey Dokuchaev  wrote:
> > What are the benefits of the new order?
> 
> The advantages and disadvantages of dynamic linking are a contentious
> and almost religious issue, so I hope you don't mind that I will not go
> into this.

OK. :-)

> > What about those of us who cannot use BEs, VMs, and other "cloudy"
> > tech because, well, they might not work as well and reliably as one
> > might think?
> 
> There are many possibilities, such as making backups, using
> WITHOUT_SHARED_TOOLCHAIN (and hoping that you can compile/link your way
> out of a botched installation), or even using NO_SHARED.

WITHOUT_SHARED_TOOLCHAIN sounds good, I hope it won't go away one day.

> > Very good point. [about regressed performance]
> 
> But if you take this point to its logical conclusion, then you should
> link everything statically, and never use dynamic linking at all. :)

Toolchain is special: many people prefer (or have to) build their ports
and stuff; even those who prefer binary packages may need to test their
ports in a tinderbox or p*re.  In other words, I don't mind Firefox being
dynalinked because I launch it once a month, contrary to the compiler.

> I only tested -j24 on a 32-core system, but I could probably repeat the
> experiment with lower and higher -j values: [...]
> 
> So ~2.3% difference in real time, which is not too bad I think.

Well, I'd say it's acceptable. :-/

> There are probably opportunities to improve the performance of the
> dynamic linker, which would be beneficial to every program in the
> system.

Now that's a good point; I look forward to it!  Thanks for replying,

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r353936 - head/contrib/llvm/tools/clang/lib/Driver/ToolChains/Arch

[Alexey Dokuchaev]

On Thu, Oct 24, 2019 at 09:14:07AM -0600, Warner Losh wrote:
> On Thu, Oct 24, 2019 at 2:26 AM Alexey Dokuchaev  wrote:
> > On Wed, Oct 23, 2019 at 04:57:12PM +, Dimitry Andric wrote:
> > > New Revision: 353936
> > > URL: https://svnweb.freebsd.org/changeset/base/353936
> > >
> > > Log:
> > >   Bump clang's default target CPU for the i386 architecture (aka
> > >   "x86") to i686, as per the discussion on the freebsd-arch mailing
> > >   list.
> >
> > Why i686, not i586?  i486 lacking 64-bit atomics is a sound and valid
> > reason, but I don't understand why i586 wasn't chosen, and quick review
> > of that -arch thread did not help.
> 
> There were several notions at play here. First, the rest of the i386
> ecosystem has defaulted to i686 for a long time. [...]
> 
> i686 support by default allows better code generation and increased
> performance. The biggest thing being using CMOVxx instructions to avoid
> a pipeline miss due to branching, though there's likely others.
> 
> By moving to i686 by default, we have only one bump instead of two. [...]
> 
> So that's where we are: a mix of technical and political reasons were why
> we bumped up to i686 by default

Understood.  I appreciate long and elaborate reply Warner.

> Chances are this will be the last minimum bump as well before i386 is
> removed from the tree as irrelevant (some years from now, but the day
> will come).

I hope the day never comes, as I don't want to be forced from FreeBSD to
something else on vast majority of my hardware.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r353937 - in head/share: man/man5 mk

[Alexey Dokuchaev]

On Wed, Oct 23, 2019 at 11:31:41AM -0700, Enji Cooper wrote:
> > On Oct 23, 2019, at 10:02, Dimitry Andric  wrote:
> > New Revision: 353937
> > URL: https://svnweb.freebsd.org/changeset/base/353937
> > 
> > Log:
> >  Build toolchain components as dynamically linked executables by default

What are the benefits of the new order?

> >  Summary:
> >  Historically, we have built toolchain components such as cc, ld, etc as
> >  statically linked executables.  One of the reasons being that you could
> >  sometimes save yourself from botched upgrades, by e.g. recompiling a
> >  "known good" libc and reinstalling it.

> >  In this day and age, we have boot environments, virtual machine
> >  snapshots, cloud backups, and other much more reliable methods to
> >  restore systems to working order.  So I think the time is ripe to flip

What about those of us who cannot use BEs, VMs, and other "cloudy" tech
because, well, they might not work as well and reliably as one might think?

> Using dynamic binaries instead of static binaries might actually regress
> performance in a way you don't expect, depending on -j values, etc. Static
> binaries avoid the dynamic linker, which (obviously) results in a perf hit
> at scale [...]

Very good point.

> Did you calculate the perf trade offs for the static binaries at low -j vs
> high -j, system and user time, etc?

I'd like to know the answer to this question (and the results) as well.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r353936 - head/contrib/llvm/tools/clang/lib/Driver/ToolChains/Arch

[Alexey Dokuchaev]

On Wed, Oct 23, 2019 at 04:57:12PM +, Dimitry Andric wrote:
> New Revision: 353936
> URL: https://svnweb.freebsd.org/changeset/base/353936
> 
> Log:
>   Bump clang's default target CPU for the i386 architecture (aka "x86") to
>   i686, as per the discussion on the freebsd-arch mailing list.  Earlier
>   in r352030, I had already bumped it to i586, to work around missing
>   atomic 64 bit functions for the i386 architecture.

Why i686, not i586?  i486 lacking 64-bit atomics is a sound and valid
reason, but I don't understand why i586 wasn't chosen, and quick review
of that -arch thread did not help.  Could you shed some more light here?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r351379 - head/usr.bin/w

[Alexey Dokuchaev]

On Thu, Aug 22, 2019 at 03:28:31AM +, Mike Karels wrote:
> New Revision: 351379
> URL: https://svnweb.freebsd.org/changeset/base/351379
> 
> Log:
>   Change w(1) to compute FROM (host) field size dynamically
>   
>   It's nice to be able to display a full IPv6 host address if
>   needed, but it's also nice to display more than 3 characters of a command
>   line. Compute the needed size for the FROM column in an earlier pass,
>   and determine the maximum, then print what fits for the command.

Thank you Mike, this had been bugging me for quite a while!  Now I only
need one-line W_DISPUSERSIZE=8 patch to get my perfect w(1). :-)

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r353565 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Alexey Dokuchaev]

On Tue, Oct 15, 2019 at 10:00:51AM -0600, Ian Lepore wrote:
> On Tue, 2019-10-15 at 18:56 +0300, Andriy Gapon wrote:
> > On 15/10/2019 18:53, O. Hartmann wrote:
> > > -BEGIN PGP SIGNED MESSAGE-
> > > Hash: SHA256
> > > 
> > > Am Tue, 15 Oct 2019 15:09:36 + (UTC)
> > > Andriy Gapon  schrieb:
> > 
> > Sorry, I got tired before I could scroll to your reply.
> 
> You break the build, a user reports it, and a smartass reply is the
> best you can do?

In Andriy's defense, O. Hartmann could have trimmed the log down to the
relevant build error lines rather than (uselessly) quoting entire diff.
It really takes a while to scroll down these emails of his, especially
on a slow connections.

Overquoting in general is a big problem on FreeBSD lists, and I don't
know exactly why.  It was not like that 15-20 years ago.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r346588 - head/lib/libc/powerpc64/string

[Alexey Dokuchaev]

On Tue, Apr 23, 2019 at 02:53:53AM +, Justin Hibbits wrote:
> New Revision: 346588
> URL: https://svnweb.freebsd.org/changeset/base/346588
> 
> Log:
>   powerpc64: Rewrite strcmp in asm to take advantage of word size
> ...
>   Some performance gain rates between the current and the optimized
>   solution:
> 
>   String size (bytes) Gain rate
>   <=8 0.59%
>   <=161.92%
>   32  3.02%
>   64  5.60%
>   128 10.16%
>   256 18.05%
>   512 30.18%
>   102442.82%

Nice!  This should help to speed up buildworld quite a bit.  Would it
be feasible to patch ppc32 in a similar fashion?  Thanks,

./danfe


___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r346318 - head/lib/libcasper/services/cap_fileargs

[Alexey Dokuchaev]

On Wed, Apr 17, 2019 at 04:31:30PM +, Ed Maste wrote:
> New Revision: 346318
> URL: https://svnweb.freebsd.org/changeset/base/346318
> 
> Log:
>   cap_fileargs: fix GCC build, don't shadow 'stat'
>   
> @@ -566,7 +566,7 @@ static int
>  fileargs_command_lstat(const nvlist_t *limits, nvlist_t *nvlin,
>  nvlist_t *nvlout)
>  {
> - int stat;
> + int error;
>   const char *name;
>   struct stat sb;
>  
> @@ -578,7 +578,7 @@ fileargs_command_lstat(const nvlist_t *limits, nvlist_
>  
>   name = nvlist_get_string(nvlin, "name");
>  
> - stat = lstat(name, );
> + error = lstat(name, );
>   if (stat < 0)
>   return (errno);

Looks like "if (stat < 0)" is wrong now.

./danfe


___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r344648 - in head: . sys/kern sys/sys

[Alexey Dokuchaev]

On Sun, Mar 10, 2019 at 03:11:40PM +, Alexey Dokuchaev wrote:
> On Thu, Feb 28, 2019 at 07:47:51AM +0000, Alexey Dokuchaev wrote:
> > On Wed, Feb 27, 2019 at 10:56:55PM +, Mateusz Guzik wrote:
> > > New Revision: 344648
> > > URL: https://svnweb.freebsd.org/changeset/base/344648
> > > 
> > > Log:
> > >   Rename seq to seqc to avoid namespace clashes with Linux
> > >   
> > > ...
> > > Added:
> > >   head/sys/sys/seqc.h   (contents, props changed)
> > > Deleted:
> > >   head/sys/sys/seq.h
> > 
> > Why it was deleted and added as new file instead of being repocopied?
> 
> Retransmit.

Retransmit.

./danfe


___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r345680 - head/sys/conf

[Alexey Dokuchaev]

On Fri, Mar 29, 2019 at 05:40:05PM +1100, Bruce Evans wrote:
> >> ...
> >> -options   BOOT_TAG=\"---<>---\"
> >> +options   BOOT_TAG=\"\"
> 
> Useful variations are not always easy to invent.  I usually just add 1 for
> numeric values.  This example is useful.  It kills the boot tag.  It gives
> an empty tag, and the kernel does extra work to avoid printing a newline
> after an empty tag.

That's a nice idea, thanks Bruce.  I always had to revert commits that had
added this stupid boot tag; if empty tag does not imply a newline, then it
makes things much easier.

./danfe

P.S.  Boot tag of a healthy human is "Copyright (c) 1992-2018 The FreeBSD
Project".


___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r349890 - head/contrib/telnet/telnet

[Alexey Dokuchaev]

On Wed, Jul 10, 2019 at 05:42:04PM +, Philip Paeps wrote:
> New Revision: 349890
> URL: https://svnweb.freebsd.org/changeset/base/349890
> 
> Log:
>   telnet: fix a couple of snprintf() buffer overflows
>   
> Modified: head/contrib/telnet/telnet/commands.c
> @@ -1655,10 +1655,11 @@ env_init(void)
>   char hbuf[256+1];
>   char *cp2 = strchr((char *)ep->value, ':');
>  
> - gethostname(hbuf, 256);
> - hbuf[256] = '\0';
> - cp = (char *)malloc(strlen(hbuf) + strlen(cp2) + 1);
> - sprintf((char *)cp, "%s%s", hbuf, cp2);

Would it make sense to add something like __attribute__ ((deprecated))
to those unsafe functions like gets(), sprintf(), etc.?  Or it would
cause too much PITA?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r349890 - head/contrib/telnet/telnet

[Alexey Dokuchaev]

On Wed, Jul 10, 2019 at 04:45:06PM -0600, Warner Losh wrote:
> On Wed, Jul 10, 2019 at 4:43 PM Shawn Webb 
> > ...
> > Because the existing code already used malloc + snprintf. And this is
> > contrib/telnet/telnet, which arguably should be `rm -rf`ed. ;)
> >
> > The bike shed is now glow-in-the-dark neon green.
> 
> I'm not in the rm-rf telnet camp... I use it far too often to connect
> to a port with good terminal support to want to do that...

Seconded.  Having telnet(1) in the base is so fucking convenient compared
to GNU/Linux or Windoze where they've removed it from the default install.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r248514 - head/sys/vm

[Alexey Dokuchaev]

On Tue, Mar 19, 2013 at 02:39:27PM +, Konstantin Belousov wrote:
> New Revision: 248514
> URL: http://svnweb.freebsd.org/changeset/base/248514
> 
> Log:
>   Do not map the swap i/o pbufs if the geom provider for the swap
>   partition accepts unmapped requests.
>   
> Modified: head/sys/vm/swap_pager.c
> ...
> @@ -2180,6 +2190,7 @@ swaponsomething(struct vnode *vp, void *
>   sp->sw_flags = 0;
>   sp->sw_nblks = nblks;
>   sp->sw_used = 0;
>   sp->sw_strategy = strategy;
>   sp->sw_close = close;
> + sp->sw_flags = flags;

PVS Studio complains here: /usr/src/sys/vm/swap_pager.c:2238:1: warning:
V519 The 'sp->sw_flags' variable is assigned values twice successively.
Perhaps this is a mistake. Check lines: 2233, 2238.

Looks like "sp->sw_flags = 0" should've been removed, can you confirm?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

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

[Alexey Dokuchaev]

On Mon, Mar 07, 2016 at 06:44:07PM +, Konstantin Belousov wrote:
> New Revision: 296467
> URL: https://svnweb.freebsd.org/changeset/base/296467
> 
> Log:
>   Convert all panics from the link_elf_obj kernel linker for object
>   files format into printfs and errors to caller.  Some leaks of
>   resources are there, but the same leaks are present in other error
>   pathes.  With the change, the kernel at least boots even when module
>   with unexpected or corrupted ELF structure is preloaded.
>   
> Modified: head/sys/kern/link_elf_obj.c
> ...
> @@ -634,8 +645,11 @@ link_elf_load_file(linker_class_t cls, c
>   ef->relatab = malloc(ef->nrelatab * sizeof(*ef->relatab),
>   M_LINKER, M_WAITOK | M_ZERO);
>  
> - if (symtabindex == -1)
> - panic("lost symbol table index");
> + if (symtabindex == -1) {
> + link_elf_error(filename, "lost symbol table index");
> + error = ENOEXEC;
> + goto out;
> + }
>   /* Allocate space for and load the symbol table */
>   ef->ddbsymcnt = shdr[symtabindex].sh_size / sizeof(Elf_Sym);
>   ef->ddbsymtab = malloc(shdr[symtabindex].sh_size, M_LINKER, M_WAITOK);
> @@ -650,8 +664,11 @@ link_elf_load_file(linker_class_t cls, c
>   goto out;
>   }
>  
> - if (symstrindex == -1)
> - panic("lost symbol string index");
> + if (symstrindex == -1) {
> + link_elf_error(filename, "lost symbol string index");
> + error = ENOEXEC;
> + goto out;
> + }

PVS Studio reports: /usr/src/sys/kern/link_elf_obj.c:717:1: warning: V547
Expression 'symstrindex == - 1' is always false.

Original panic()s were added by peter@ in r129362 (CC'ed).  Could one of
you guys take a look?  Thanks,

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r344648 - in head: . sys/kern sys/sys

[Alexey Dokuchaev]

On Fri, May 31, 2019 at 03:20:50PM +, Brooks Davis wrote:
> On Fri, May 24, 2019 at 09:11:36AM +0000, Alexey Dokuchaev wrote:
> > On Sun, Mar 10, 2019 at 03:11:40PM +0000, Alexey Dokuchaev wrote:
> > > On Thu, Feb 28, 2019 at 07:47:51AM +, Alexey Dokuchaev wrote:
> > > > On Wed, Feb 27, 2019 at 10:56:55PM +, Mateusz Guzik wrote:
> > > > > New Revision: 344648
> > > > > URL: https://svnweb.freebsd.org/changeset/base/344648
> > > > > 
> > > > > Log:
> > > > >   Rename seq to seqc to avoid namespace clashes with Linux
> > > > >   
> > > > > ...
> > > > > Added:
> > > > >   head/sys/sys/seqc.h   (contents, props changed)
> > > > > Deleted:
> > > > >   head/sys/sys/seq.h
> > > > 
> > > > Why it was deleted and added as new file instead of being repocopied?
> > > 
> > > Retransmit.
> > 
> > Ping!
> 
> Do you have nothing better to do?

Oh, I have a lot on my plate, thanks for asking.  Now, back to the subject:

- I think that this commit was executed badly (not via repocopy)
- I've been trying to contact mjg@ for several weeks, to no avail :(
- All I get in return is an email from a fellow committer asking me
  if I have anything better to do

Sorry, I don't understand this.  I don't understand why we ignore email
from our own community.  I don't understand why we do not correct or at
least explain why what looks like a mistake is actually not(?).  I don't
understand why I'm being ridiculed, not those who make dubious commits
and fail to defend them.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r232071 - head/sys/vm

[Alexey Dokuchaev]

On Mon, May 27, 2019 at 02:34:09PM -0700, Rodney W. Grimes wrote:
> I do not at all mean to discourage what you are doing, it is good to go
> over static analysis reports, the problem is that there are often many
> false positives,

There are tons of them indeed, but that's normal for any static analysis
tool.  I've attempted to clean up the original log by ignoring the most
noisy and dubious diagnostics and maintaining false positives list,
starting with the ones pointed out by kib@.

> It would be nice if we had a "team" that looked at all the Coverity
> data, and any other data like what you have offered up here.  Part of
> the problem is that few want to do that work, or those that do want to
> think it is low hanging fruit that anyone can do.

That's exactly right, generating the check log is easy, finding people
motivated enough to triage it (or finding the motivation within oneself)
is tough.  Maybe this ~2.4k line log would attract more interest than
the first raw, full >16k line one:

freefall:/home/danfe/pvs-kernel-filtered-2019-05-28.log.xz

I'd appreciate if experts in their areas (cam/scsi, dev drivers, geom,
kern, netinet, nfs, etc.) glanced over and see if they might help with
anything there.  If you find a false positive, please tell me so I
can add it to the ignore list.  Thanks!
 
./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r232071 - head/sys/vm

[Alexey Dokuchaev]

On Mon, May 27, 2019 at 07:21:24AM -0700, Rodney W. Grimes wrote:
> ...
> We as a project look bad by not having this minimal set of code
> reviews in place for things that are coming from a source that is
> frought with introducing bad change.

We do *not* look bad because none of those warnings and errors suggested
by (any) static analysis tools were ever passed without all the due checks
by either the original committer, the last committer who touched the files,
or opening a PR in less straight-forward cases.

I have several potential issues in my queue from the last run which I might
submit for the further analysis (via email or PR), but for now since I've
published the full log (very noisy), let's see if it gets any traction on
its own.  Some people had contacted me off-the-list and expressed their
interest.  I'll revisit it in a few weeks to see if nagging developers and
opening PRs is still needed, or the bugs are being voluntarily squished by
responsible (and having proper domain-knowledge) parties.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r232071 - head/sys/vm

[Alexey Dokuchaev]

On Mon, May 27, 2019 at 07:21:24AM -0700, Rodney W. Grimes wrote:
> I wish to assert again that all changes based on static
> analysis tools require a formal code review by at minimum:
> a)  An expert in static analysis tools or a group of people
> selected who we consider language experts.
> b)  An area expert from the area that is being affected
> 
> We as a project look bad by not having this minimal
> set of code reviews in place for things that are coming
> from a source that is frought with introducing bad
> change.

Perhaps I should've just uploaded the log to let anyone interested
have a look themselves.  I've been only checking the kernel so far,
but plan to cover userland as well later.

freefall:/home/danfe/pvs-studio-2019-05-27.log.xz

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r232071 - head/sys/vm

[Alexey Dokuchaev]

On Mon, May 27, 2019 at 02:21:55PM +0300, Konstantin Belousov wrote:
> On Mon, May 27, 2019 at 09:52:56AM +0000, Alexey Dokuchaev wrote:
> > On Thu, Feb 23, 2012 at 09:07:16PM +, Konstantin Belousov wrote:
> > > ...
> > > + /*
> > > +  * Decrement the object's writemappings, by swapping the start
> > > +  * and end arguments for vnode_pager_update_writecount().  If
> > > +  * there was not a race with vnode reclaimation, then the
> > > +  * vnode's v_writecount is decremented.
> > > +  */
> > > + vnode_pager_update_writecount(object, end, start);
> > 
> > ... here, first `end' is passed, then `start'.  Is this intentional?
> Did you read the comment right before the call ?

I did, as I thought that explanation might be there, but it evaded me
(also, I was subconsciously expecting it near the end of the comment),
thanks for bring it to my attention and sorry for the noise.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r232071 - head/sys/vm

[Alexey Dokuchaev]

On Thu, Feb 23, 2012 at 09:07:16PM +, Konstantin Belousov wrote:
> New Revision: 232071
> URL: http://svn.freebsd.org/changeset/base/232071
> 
> Log:
>   Account the writeable shared mappings backed by file in the vnode
>   v_writecount.  Keep the amount of the virtual address space used by
>   the mappings in the new vm_object un_pager.vnp.writemappings
>   counter. The vnode v_writecount is incremented when writemappings gets
>   non-zero value, and decremented when writemappings is returned to
>   zero.
>   
>   Writeable shared vnode-backed mappings are accounted for in vm_mmap(),
>   and vm_map_insert() is instructed to set MAP_ENTRY_VN_WRITECNT flag on
>   the created map entry.  During deferred map entry deallocation,
>   vm_map_process_deferred() checks for MAP_ENTRY_VN_WRITECOUNT and
>   decrements writemappings for the vm object.
>   
>   Now, the writeable mount cannot be demoted to read-only while
>   writeable shared mappings of the vnodes from the mount point
>   exist. Also, execve(2) fails for such files with ETXTBUSY, as it
>   should be.
>   
> ...
> Modified: head/sys/vm/vnode_pager.c
> ==
> --- head/sys/vm/vnode_pager.c Thu Feb 23 20:58:52 2012(r232070)
> +++ head/sys/vm/vnode_pager.c Thu Feb 23 21:07:16 2012(r232071)
> @@ -1215,3 +1222,81 @@ vnode_pager_undirty_pages(vm_page_t *ma,
>   }
>   VM_OBJECT_UNLOCK(obj);
>  }
> +
> +void
> +vnode_pager_update_writecount(vm_object_t object, vm_offset_t start,
> +vm_offset_t end)

So, it is first `start, then `end', but below...

> +{
> + struct vnode *vp;
> + vm_ooffset_t old_wm;
> +
> + ...
> +}
> +
> +void
> +vnode_pager_release_writecount(vm_object_t object, vm_offset_t start,
> +vm_offset_t end)
> +{
> + struct vnode *vp;
> + struct mount *mp;
> + vm_offset_t inc;
> + int vfslocked;
> +
> + VM_OBJECT_LOCK(object);
> +
> + /*
> +  * First, recheck the object type to account for the race when
> +  * the vnode is reclaimed.
> +  */
> + if (object->type != OBJT_VNODE) {
> + VM_OBJECT_UNLOCK(object);
> + return;
> + }
> +
> + /*
> +  * Optimize for the case when writemappings is not going to
> +  * zero.
> +  */
> + inc = end - start;
> + if (object->un_pager.vnp.writemappings != inc) {
> + object->un_pager.vnp.writemappings -= inc;
> + VM_OBJECT_UNLOCK(object);
> + return;
> + }
> +
> + vp = object->handle;
> + vhold(vp);
> + VM_OBJECT_UNLOCK(object);
> + vfslocked = VFS_LOCK_GIANT(vp->v_mount);
> + mp = NULL;
> + vn_start_write(vp, , V_WAIT);
> + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
> +
> + /*
> +  * Decrement the object's writemappings, by swapping the start
> +  * and end arguments for vnode_pager_update_writecount().  If
> +  * there was not a race with vnode reclaimation, then the
> +  * vnode's v_writecount is decremented.
> +  */
> + vnode_pager_update_writecount(object, end, start);

... here, first `end' is passed, then `start'.  Is this intentional?

PVS Studio complains:

/usr/src/sys/vm/vnode_pager.c:1584:1: warning: V764 Possible
incorrect order of arguments passed to 'vnode_pager_update_writecount'
function: 'end' and 'start'.

> + VOP_UNLOCK(vp, 0);
> + vdrop(vp);
> + if (mp != NULL)
> + vn_finished_write(mp);
> + VFS_UNLOCK_GIANT(vfslocked);
> +}
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r348286 - in head/sys/dev: ctau cxgbe

[Alexey Dokuchaev]

On Sun, May 26, 2019 at 07:05:20AM -0700, Enji Cooper wrote:
> > On May 26, 2019, at 05:41, Alexey Dokuchaev  wrote:
> > New Revision: 348286
> > URL: https://svnweb.freebsd.org/changeset/base/348286
> > 
> > Log:
> >  Fix two errors reported by PVS Studio: V646 Consider inspecting the
> >  application's logic.  It's possible that 'else' keyword is missing.
> > 
> > ...
> > Modified: head/sys/dev/ctau/ctddk.c
> > --- head/sys/dev/ctau/ctddk.cSun May 26 03:52:35 2019(r348285)
> > +++ head/sys/dev/ctau/ctddk.cSun May 26 12:41:03 2019(r348286)
> > @@ -237,7 +237,7 @@ int ct_set_clk (ct_chan_t *c, int clk)
> >if (c->mode == M_E1) {
> >ct_setup_e1 (c->board);
> >return 0;
> > -} if (c->mode == M_G703) {
> > +} else if (c->mode == M_G703) {
> >ct_setup_g703 (c->board);
> >return 0;
> >} else
> 
> This seems like a good candidate for a switch-case logical block.

Perhaps.  I'm currently doing some PVS Studio runs over our kernel
and these were just some low-hanging fruit I wanted to fix.  Several
others had been forwarded to commit authors, and some already fixed.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

svn commit: r348286 - in head/sys/dev: ctau cxgbe

[Alexey Dokuchaev]

Author: danfe (ports committer)
Date: Sun May 26 12:41:03 2019
New Revision: 348286
URL: https://svnweb.freebsd.org/changeset/base/348286

Log:
  Fix two errors reported by PVS Studio: V646 Consider inspecting the
  application's logic.  It's possible that 'else' keyword is missing.
  
  Reviewed by:  gallatin, np, pfg
  Approved by:  pfg
  Differential Revision:https://reviews.freebsd.org/D20396

Modified:
  head/sys/dev/ctau/ctddk.c
  head/sys/dev/cxgbe/t4_main.c

Modified: head/sys/dev/ctau/ctddk.c
==
--- head/sys/dev/ctau/ctddk.c   Sun May 26 03:52:35 2019(r348285)
+++ head/sys/dev/ctau/ctddk.c   Sun May 26 12:41:03 2019(r348286)
@@ -237,7 +237,7 @@ int ct_set_clk (ct_chan_t *c, int clk)
if (c->mode == M_E1) {
ct_setup_e1 (c->board);
return 0;
-   } if (c->mode == M_G703) {
+   } else if (c->mode == M_G703) {
ct_setup_g703 (c->board);
return 0;
} else

Modified: head/sys/dev/cxgbe/t4_main.c
==
--- head/sys/dev/cxgbe/t4_main.cSun May 26 03:52:35 2019
(r348285)
+++ head/sys/dev/cxgbe/t4_main.cSun May 26 12:41:03 2019
(r348286)
@@ -9709,7 +9709,7 @@ set_offload_policy(struct adapter *sc, struct t4_offlo
/* Delete installed policies. */
op = NULL;
goto set_policy;
-   } if (uop->nrules > 256) { /* arbitrary */
+   } else if (uop->nrules > 256) { /* arbitrary */
return (E2BIG);
}
 
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r348250 - head/sys/powerpc/conf

[Alexey Dokuchaev]

On Fri, May 24, 2019 at 03:16:51PM -0500, Justin Hibbits wrote:
> On Fri, 24 May 2019 20:01:59 + (UTC) Piotr Kubaj wrote:
> > New Revision: 348250
> > URL: https://svnweb.freebsd.org/changeset/base/348250
> > 
> > Log:
> >   Add snd_hda(4) to GENERIC64 used by powerpc64.
> >   
> >   amd64 also has snd_hda(4) in GENERIC.
> >   
> > Modified:
> >   head/sys/powerpc/conf/GENERIC64
> > ...
> 
> To note: This was done because there's a strange bug in the snd_hda
> module, with the hdaa component.  For some reason it either doesn't
> find all the internal components it needs, or something, because
> there's a NULL dereference when trying to call a kobj method in
> hdaa_attach().

So this commit essentially masks the real bug somewhere rather than
fixing it, is this what you're saying?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r344648 - in head: . sys/kern sys/sys

[Alexey Dokuchaev]

On Sun, Mar 10, 2019 at 03:11:40PM +, Alexey Dokuchaev wrote:
> On Thu, Feb 28, 2019 at 07:47:51AM +0000, Alexey Dokuchaev wrote:
> > On Wed, Feb 27, 2019 at 10:56:55PM +, Mateusz Guzik wrote:
> > > New Revision: 344648
> > > URL: https://svnweb.freebsd.org/changeset/base/344648
> > > 
> > > Log:
> > >   Rename seq to seqc to avoid namespace clashes with Linux
> > >   
> > > ...
> > > Added:
> > >   head/sys/sys/seqc.h   (contents, props changed)
> > > Deleted:
> > >   head/sys/sys/seq.h
> > 
> > Why it was deleted and added as new file instead of being repocopied?
> 
> Retransmit.

Ping!

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r250609 - head/sys/dev/ath

[Alexey Dokuchaev]

On Wed, May 22, 2019 at 10:34:35AM -0600, Ian Lepore wrote:
> On Wed, 2019-05-22 at 16:20 +0000, Alexey Dokuchaev wrote:
> > On Mon, May 13, 2013 at 07:03:13PM +, Adrian Chadd wrote:
> > > New Revision: 250609
> > > URL: http://svnweb.freebsd.org/changeset/base/250609
> > > 
> > > Log:
> > >   Since the node state is 100% back under the TX lock, just kill
> > > the use
> > >   of atomics.
> > >   
> > > Modified:
> > >   head/sys/dev/ath/if_ath.c
> > > 
> > > @@ -6140,13 +6133,13 @@ ath_tx_update_tim(struct ath_softc *sc,
> > >   /*
> > >* Don't bother grabbing the lock unless the queue is
> > > empty.
> > >*/
> > > - if (atomic_load_acq_int(>an_swq_depth) != 0)
> > > + if (>an_swq_depth != 0)
> > >   return;
> > >  
> > >   if (an->an_is_powersave &&
> > >   an->an_stack_psq == 0 &&
> > >   an->an_tim_set == 1 &&
> > > - atomic_load_acq_int(>an_swq_depth) == 0) {
> > > + an->an_swq_depth == 0) {
> > 
> > PVS Studio complains here: warning: V560 A part of conditional
> > expression
> > is always true: an->an_swq_depth == 0.  Which probably makes sense
> > since
> > you return earlier if it's != 0.
> 
> You're replying to a six year old commit?

Yeah, anything wrong with it?

> It doesn't check earlier whether that value is 0, it checks whether the
> address of that value is 0.  That's probably a bug.

No, the & is a vestige from the atomic_* conversion, it is not present in
current code.  The check is still there, however.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r244943 - in head/sys/dev/ath/ath_hal: ar5416 ar9002

[Alexey Dokuchaev]

On Wed, Jan 02, 2013 at 12:38:01AM +, Adrian Chadd wrote:
> New Revision: 244943
> URL: http://svnweb.freebsd.org/changeset/base/244943
> 
> Log:
>   Bring over the basic spectral scan framework code from Qualcomm Atheros.
>   
>   This includes the HAL routines to setup, enable/activate/disable spectral
>   scan and configure the relevant registers.
>   
>   Obtained from:  Qualcomm Atheros
> 
> Added:
>   head/sys/dev/ath/ath_hal/ar5416/ar5416_spectral.c
> 
> ...
> + if (ss->ss_period != HAL_SPECTRAL_PARAM_NOVAL) {
> + val &= ~AR_PHY_SPECTRAL_SCAN_PERIOD;
> + val |= SM(ss->ss_period, AR_PHY_SPECTRAL_SCAN_PERIOD);
> + }
> +
> + if (ss->ss_period != HAL_SPECTRAL_PARAM_NOVAL) {
> + val &= ~AR_PHY_SPECTRAL_SCAN_PERIOD;
> + val |= SM(ss->ss_period, AR_PHY_SPECTRAL_SCAN_PERIOD);
> + }

PVS Studio complains here: warning: V581 The conditional expressions of
the 'if' operators situated alongside each other are identical.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r250609 - head/sys/dev/ath

[Alexey Dokuchaev]

On Mon, May 13, 2013 at 07:03:13PM +, Adrian Chadd wrote:
> New Revision: 250609
> URL: http://svnweb.freebsd.org/changeset/base/250609
> 
> Log:
>   Since the node state is 100% back under the TX lock, just kill the use
>   of atomics.
>   
> Modified:
>   head/sys/dev/ath/if_ath.c
> 
> @@ -6140,13 +6133,13 @@ ath_tx_update_tim(struct ath_softc *sc,
>   /*
>* Don't bother grabbing the lock unless the queue is empty.
>*/
> - if (atomic_load_acq_int(>an_swq_depth) != 0)
> + if (>an_swq_depth != 0)
>   return;
>  
>   if (an->an_is_powersave &&
>   an->an_stack_psq == 0 &&
>   an->an_tim_set == 1 &&
> - atomic_load_acq_int(>an_swq_depth) == 0) {
> + an->an_swq_depth == 0) {

PVS Studio complains here: warning: V560 A part of conditional expression
is always true: an->an_swq_depth == 0.  Which probably makes sense since
you return earlier if it's != 0.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310524 - in head/sys/cam: ctl scsi

[Alexey Dokuchaev]

On Wed, May 22, 2019 at 11:40:42AM -0400, Alexander Motin wrote:
> On 22.05.2019 11:21, Alexey Dokuchaev wrote:
> > On Sat, Dec 24, 2016 at 05:42:34PM +, Alexander Motin wrote:
> >> New Revision: 310524
> >> URL: https://svnweb.freebsd.org/changeset/base/310524
> >>
> >> Modified: head/sys/cam/scsi/scsi_all.c
> >> ...
> >> +  }
> >> +  if (len > sizeof(sense->cmd_spec_info)) {
> >> +  data += len - sizeof(sense->cmd_spec_info);
> >> +  len -= len - sizeof(sense->cmd_spec_info);
> > 
> > PVS Studio complains here: error: V765 A compound assignment expression 'x
> > -= x - ...' is suspicious. Consider inspecting it for a possible error.
> > 
> > This "len -= len - sizeof(...)" indeed looks weird, can you take a look?
> 
> It may be weird, but not incorrect.  Fixed though.

Wow that was fast!  Thanks (for both commits),

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310524 - in head/sys/cam: ctl scsi

[Alexey Dokuchaev]

On Sat, Dec 24, 2016 at 05:42:34PM +, Alexander Motin wrote:
> New Revision: 310524
> URL: https://svnweb.freebsd.org/changeset/base/310524
> 
> Log:
>   Improve length handling when writing sense data.
>   
>- Allow maximal sense size limitation via Control Extension mode page.
>- When sense size limited, include descriptors atomically: whole or none.
>- Set new SDAT_OVFL bit if some descriptors don't fit the limit.
>- Report real written sense length instead of static maximal 252 bytes.
>   
> Modified: head/sys/cam/scsi/scsi_all.c
> ...
>   break;
> - case SSD_ELEM_DESC:
> - default:
> -
> - /*
> -  * If the user passes in descriptor sense,
> -  * we can't handle that in fixed format.
> -  * So just skip it, and any unknown argument
> -  * types.
> -  */
> + }
> + if (len > sizeof(sense->cmd_spec_info)) {
> + data += len - sizeof(sense->cmd_spec_info);
> + len -= len - sizeof(sense->cmd_spec_info);

PVS Studio complains here: error: V765 A compound assignment expression 'x
-= x - ...' is suspicious. Consider inspecting it for a possible error.

This "len -= len - sizeof(...)" indeed looks weird, can you take a look?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r345815 - in head: lib/libcam sys/cam sys/cam/nvme

[Alexey Dokuchaev]

On Tue, Apr 02, 2019 at 07:37:53PM +, Alexander Motin wrote:
> New Revision: 345815
> URL: https://svnweb.freebsd.org/changeset/base/345815
> 
> Log:
>   Make cam_error_print() decode NVMe commands.
> 
> Modified: head/sys/cam/nvme/nvme_xpt.c
> ...
> @@ -769,11 +769,13 @@ nvme_proto_debug_out(union ccb *ccb)
>  {
>   char cdb_str[(sizeof(struct nvme_command) * 3) + 1];
>  
> - if (ccb->ccb_h.func_code != XPT_NVME_IO)
> + if (ccb->ccb_h.func_code != XPT_NVME_IO ||
> + ccb->ccb_h.func_code != XPT_NVME_ADMIN)
>   return;

PVS Studio complains that "Expression is always true. Probably the '&&'
operator should be used here".  Can you take a look?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r347961 - head

[Alexey Dokuchaev]

On Wed, May 22, 2019 at 09:42:29AM -0300, Renato Botelho wrote:
> On 18/05/19 16:36, Brad Davis wrote:
> > New Revision: 347961
> > URL: https://svnweb.freebsd.org/changeset/base/347961
> > 
> > Log:
> >   Add note to UPDATING for users of mergemaster after the move of
> >   master.passwd and group in r347638.
> >   
> >   Approved by:  allanjude (mentor)
> > 
> > Modified:
> >   head/UPDATING
> > 
> > +20190517:
> > +   For users of mergemaster, after recent changes to locations of
> > +   master.passwd and group in the source tree mergemaster needs to be
> > +   updated.  Before running `mergemaster -p', cd to usr.sbin/mergemaster
> > +   and run `make install'.
> > +
> 
> etcupdate is installing group and master.passwd to /lib/libc/gen.  It
> seems wrong.
> 
> root@x230:/usr/src # etcupdate -p
>   A /lib/libc/gen/group
>   A /lib/libc/gen/master.passwd

*facepalm*

Let me ask again: why these files were moved out of their perfectly
(and likely the only) reasonable location (etc/) under some cryptic
libc/gen/ which not only makes little sense per se, but also breaks
mergemaster(8) and similar tools?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r347953 - head/cddl/contrib/opensolaris/cmd/zfs

[Alexey Dokuchaev]

On Sat, May 18, 2019 at 04:53:35AM -0400, Allan Jude wrote:
> On 5/18/19 8:37 AM, Alexey Dokuchaev wrote:
> > On Sat, May 18, 2019 at 12:27:22PM +, Allan Jude wrote:
> >> New Revision: 347953
> >> URL: https://svnweb.freebsd.org/changeset/base/347953
> >>
> >> ...
> >> -  (void) nvlist_lookup_string(lnvl, propname,
> >> -  );
> >> -  (void) nvlist_lookup_string(rnvl, propname,
> >> -  );
> >> +  if ((nvlist_lookup_string(lnvl, propname,
> >> +  ) == ENOENT) ||
> >> +  (nvlist_lookup_string(rnvl, propname,
> >> +  ) == ENOENT)) {
> >> +  goto compare_nums;
> >> +  }
> > 
> > Another thing not to like about ZoL: their completely bogus code style
> > and formatting practices (look at those ") == ENOENT").  If they
> > are going to listen to us, can we at least try to convince them not to
> > break existing, much FreeBSD-like formatting?
> 
> They have strict CI that enforces Solaris cstyle, as that is what the
> code base has always used. No commit is merged until it passes that, and
> many other tests.

Hmm, OK then, sorry for the noise.  It looked rather bad in the email
client, but that's common issue with tabs, I should've checked better.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r347953 - head/cddl/contrib/opensolaris/cmd/zfs

[Alexey Dokuchaev]

On Sat, May 18, 2019 at 12:27:22PM +, Allan Jude wrote:
> New Revision: 347953
> URL: https://svnweb.freebsd.org/changeset/base/347953
> 
> Log:
>   MFV/ZoL: `zfs userspace` ignored all unresolved UIDs after the first
>   
>   zfsonlinux/zfs@88cfff182432e4d1c24c877f33b47ee6cf109eee
>   
>   zfs_main: fix `zfs userspace` squashing unresolved entries
>   
> ...
> @@ -2368,10 +2369,12 @@ us_compare(const void *larg, const void *rarg, void *u
>   if (rv64 != lv64)
>   rc = (rv64 < lv64) ? 1 : -1;
>   } else {
> - (void) nvlist_lookup_string(lnvl, propname,
> - );
> - (void) nvlist_lookup_string(rnvl, propname,
> - );
> + if ((nvlist_lookup_string(lnvl, propname,
> + ) == ENOENT) ||
> + (nvlist_lookup_string(rnvl, propname,
> + ) == ENOENT)) {
> + goto compare_nums;
> + }

Another thing not to like about ZoL: their completely bogus code style
and formatting practices (look at those ") == ENOENT").  If they
are going to listen to us, can we at least try to convince them not to
break existing, much FreeBSD-like formatting?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r347638 - in head: . etc lib/libc/gen

[Alexey Dokuchaev]

On Thu, May 16, 2019 at 08:05:29AM -0700, Conrad Meyer wrote:
> Hi Brad,
> 
> Can you revert this?  It seems to break every config-management tool
> we have (mergemaster, etcupdate, beinstall ...).

+1.

> The commit message describes what was changed, but not *why* you
> thought it was a good idea.
> 
> > Added:
> >   head/lib/libc/gen/group
> >  - copied unchanged from r347637, head/etc/group
> >   head/lib/libc/gen/master.passwd
> >  - copied unchanged from r347637, head/etc/master.passwd

What was wrong with etc/?  What can be more logical than this?
libc/gen/ makes so no sense.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r347410 - in head: . sys/amd64/conf sys/arm/conf sys/arm64/conf sys/i386/conf sys/powerpc/conf sys/riscv/conf sys/sparc64/conf

[Alexey Dokuchaev]

On Fri, May 10, 2019 at 09:44:58PM +0300, Andrey V. Elsukov wrote:
> On 10.05.2019 21:39, Alexey Dokuchaev wrote:
> >> The second cause -- reduce overhead that IPSEC produces even when it
> >> is not used.
> > 
> > So does it mean that if I don't plan to use IPSEC, I can safely remove
> > IPSEC_SUPPORT from my config and also get slight performance boost?
> 
> Yes, currently each call to IPsec has check like
> `if (ipsec_enabled) {...}`, when you build the kernel without
> IPSEC/IPSEC_SUPPORT, this check will be removed too, this can add some
> performance boost :-)

Got it. :-)  Thanks for the fast response (and proper quoting).

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r347410 - in head: . sys/amd64/conf sys/arm/conf sys/arm64/conf sys/i386/conf sys/powerpc/conf sys/riscv/conf sys/sparc64/conf

[Alexey Dokuchaev]

On Fri, May 10, 2019 at 12:11:47PM +0300, Andrey V. Elsukov wrote:
> On 10.05.2019 11:46, Alexey Dokuchaev wrote:
> > ...
> > What is the reason behind having IPSEC_SUPPORT option instead of no
> > special option at all?
> 
> IPSEC_SUPPORT builds into the kernel PF_KEY domain protocol, that is
> required by IPsec implementation to interact with userlevel. Currently
> the kernel does not support unregistering of protocol domains. This is
> mostly why option IPSEC_SUPPORT was introduced.

Okay, I see, thank you Andrey for explanation.

> The second cause -- reduce overhead that IPSEC produces even when it
> is not used.

So does it mean that if I don't plan to use IPSEC, I can safely remove
IPSEC_SUPPORT from my config and also get slight performance boost?

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r347410 - in head: . sys/amd64/conf sys/arm/conf sys/arm64/conf sys/i386/conf sys/powerpc/conf sys/riscv/conf sys/sparc64/conf

[Alexey Dokuchaev]

On Thu, May 09, 2019 at 10:38:15PM +, Andrew Gallatin wrote:
> Author: gallatin
> Date: Thu May  9 22:38:15 2019
> New Revision: 347410
> URL: https://svnweb.freebsd.org/changeset/base/347410
> 
> Log:
>   Remove IPSEC from GENERIC due to performance issues
>   
> @@ -30,7 +30,6 @@ options PREEMPTION  # Enable ...
>  options  VIMAGE  # Subsystem virtualization, e.g. VNET
>  options  INET# InterNETworking
>  options  INET6   # IPv6 communications protocols
> -options  IPSEC   # IP (v4/v6) security
>  options  IPSEC_SUPPORT   # Allow kldload of ipsec and tcpmd5

I've asked this question some two years ago, but no one could answer it
back then, so I'll try again.

What is the reason behind having IPSEC_SUPPORT option instead of no special
option at all?  If I grep for SUPPORT in conf/GENERIC, I see things like
INVARIANT_SUPPORT or IEEE80211_SUPPORT_MESH (with meaningful explanations)
but IPSEC_SUPPORT which, per the comment, "allows to kldload of ipsec and
tcpmd5", is totally beyond me.  Lots of kernel features are/can be loaded
as modules, but we don't have things like SOUND_SUPPORT or USB_SUPPORT.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r346932 - head/sys/ufs/ufs

[Alexey Dokuchaev]

On Mon, Apr 29, 2019 at 10:05:26PM +, Mark Johnston wrote:
> New Revision: 346932
> URL: https://svnweb.freebsd.org/changeset/base/346932
> 
> Log:
>   Optimize lseek(SEEK_DATA) on UFS.
>   
>   This version fixes the problems identified in r345244.
>   
>   Reviewed by:kib
> @@ -56,6 +56,9 @@ __FBSDID("$FreeBSD$");
>  #include 
>  #include 
>  
> +static ufs_lbn_t lbn_count(struct ufsmount *, int);
> +static int readindir(struct vnode *, ufs_lbn_t, ufs2_daddr_t, struct buf **);

Is the prototype for static readindir() really needed here?

> +static int
> +readindir(vp, lbn, daddr, bpp)
> + struct vnode *vp;
> + ufs_lbn_t lbn;
> + ufs2_daddr_t daddr;
> + struct buf **bpp;

Don't we prefer ASNI declarations over K these days?

> +static ufs_lbn_t
> +lbn_count(ump, level)
> + struct ufsmount *ump;
> + int level;

Ditto.

> +int
> +ufs_bmap_seekdata(vp, offp)
> + struct vnode *vp;
> + off_t *offp;

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r346588 - head/lib/libc/powerpc64/string

[Alexey Dokuchaev]

On Tue, Apr 23, 2019 at 02:53:53AM +, Justin Hibbits wrote:
> New Revision: 346588
> URL: https://svnweb.freebsd.org/changeset/base/346588
> 
> Log:
>   powerpc64: Rewrite strcmp in asm to take advantage of word size
> ...
>   Some performance gain rates between the current and the optimized
>   solution:
> 
>   String size (bytes) Gain rate
>   <=8 0.59%
>   <=161.92%
>   32  3.02%
>   64  5.60%
>   128 10.16%
>   256 18.05%
>   512 30.18%
>   102442.82%

Nice!  This should help to speed up buildworld quite a bit.  Would it
be feasible to patch ppc32 in a similar fashion?  Thanks,

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r346318 - head/lib/libcasper/services/cap_fileargs

[Alexey Dokuchaev]

On Wed, Apr 17, 2019 at 04:31:30PM +, Ed Maste wrote:
> New Revision: 346318
> URL: https://svnweb.freebsd.org/changeset/base/346318
> 
> Log:
>   cap_fileargs: fix GCC build, don't shadow 'stat'
>   
> @@ -566,7 +566,7 @@ static int
>  fileargs_command_lstat(const nvlist_t *limits, nvlist_t *nvlin,
>  nvlist_t *nvlout)
>  {
> - int stat;
> + int error;
>   const char *name;
>   struct stat sb;
>  
> @@ -578,7 +578,7 @@ fileargs_command_lstat(const nvlist_t *limits, nvlist_
>  
>   name = nvlist_get_string(nvlin, "name");
>  
> - stat = lstat(name, );
> + error = lstat(name, );
>   if (stat < 0)
>   return (errno);

Looks like "if (stat < 0)" is wrong now.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r344648 - in head: . sys/kern sys/sys

[Alexey Dokuchaev]

On Sun, Mar 10, 2019 at 03:11:40PM +, Alexey Dokuchaev wrote:
> On Thu, Feb 28, 2019 at 07:47:51AM +0000, Alexey Dokuchaev wrote:
> > On Wed, Feb 27, 2019 at 10:56:55PM +, Mateusz Guzik wrote:
> > > New Revision: 344648
> > > URL: https://svnweb.freebsd.org/changeset/base/344648
> > > 
> > > Log:
> > >   Rename seq to seqc to avoid namespace clashes with Linux
> > >   
> > > ...
> > > Added:
> > >   head/sys/sys/seqc.h   (contents, props changed)
> > > Deleted:
> > >   head/sys/sys/seq.h
> > 
> > Why it was deleted and added as new file instead of being repocopied?
> 
> Retransmit.

Retransmit.

./danfe
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r345680 - head/sys/conf

[Alexey Dokuchaev]

On Fri, Mar 29, 2019 at 05:40:05PM +1100, Bruce Evans wrote:
> >> ...
> >> -options   BOOT_TAG=\"---<>---\"
> >> +options   BOOT_TAG=\"\"
> 
> Useful variations are not always easy to invent.  I usually just add 1 for
> numeric values.  This example is useful.  It kills the boot tag.  It gives
> an empty tag, and the kernel does extra work to avoid printing a newline
> after an empty tag.

That's a nice idea, thanks Bruce.  I always had to revert commits that had
added this stupid boot tag; if empty tag does not imply a newline, then it
makes things much easier.

./danfe

P.S.  Boot tag of a healthy human is "Copyright (c) 1992-2018 The FreeBSD
Project".
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"