Re: CVS commit: src/sys/arch/alpha/include

2020-09-03 Thread Jason Thorpe


> On Sep 3, 2020, at 1:14 PM, matthew green  wrote:
> 
> "Jason R Thorpe" writes:
>> Module Name: src
>> Committed By:thorpej
>> Date:Thu Sep  3 04:20:54 UTC 2020
>> 
>> Modified Files:
>>  src/sys/arch/alpha/include: cpu.h
>> 
>> Log Message:
>> Garabage-collect curpcb / cpu_info::ci_curpcb.
> 
> does alpha have modules?  this may be a ABI change needing
> a kernel version bump...

"Sort of."  They don't work (not all of the required relocations are handled 
correctly), so I'm not that concerned.

-- thorpej



Re: CVS commit: src/sys/dev/pci

2020-07-17 Thread Jason Thorpe



> On Jul 17, 2020, at 3:50 PM, matthew green  wrote:
> 
> any chance you can look at NET_MPSAFE here etc? :)

I have a bunch of local changes for this in one of my trees, and I hope to get 
back to it after netbsd-10 branches.

-- thorpej



Re: CVS commit: src/sys

2020-07-08 Thread Jason Thorpe



> On Jul 8, 2020, at 12:22 AM, Martin Husemann  wrote:
> 
> On Tue, Jul 07, 2020 at 03:38:49AM +, Jason R Thorpe wrote:
>>  Provide a new resource provider API:
> 
> This is *extremly* verbose and overflows message buffer, can you move the
> new messages to aprint_debug or ifdef with some proper DEBUG_* please?

Oops, sorry, pci_conf_debug was enabled by mistake in previous commit.  I've 
fixed this now.

-- thorpej



Re: CVS commit: src/sys/arch/powerpc/booke

2020-07-06 Thread Jason Thorpe



> On Jul 6, 2020, at 5:28 PM, Rin Okuyama  wrote:
> 
> Module Name:  src
> Committed By: rin
> Date: Tue Jul  7 00:28:31 UTC 2020
> 
> Modified Files:
>   src/sys/arch/powerpc/booke: e500_tlb.c
> 
> Log Message:
> Fix kernel panic due to tmpfs.
> 
> pmap for booke assumes that the ``va'' argument for pmap_kenter_pa(9) is
> page-aligned. However, by recent changes, tmpfs became to use ``va'' with
> page offset via ubc_uiomove(9). So, truncate it to page boundary.

This change seems wrong.  I think it needs to be fixed in tmpfs.

-- thorpej



Re: CVS commit: src/tests/lib/libarchive

2020-06-16 Thread Jason Thorpe



> On Jun 16, 2020, at 8:43 AM, Martin Husemann  wrote:
> 
> No, that is definitively not OK, which is what the PR is about.
> 
> It is not OK for a regular atf run to cause a reboot of the test machine
> though, so this is a temporary hack around the issue (and admitedly a very
> ugly hack).

At the very least, the user-land watchdog tickler should wire itself down.

-- thorpej



Re: CVS commit: src/common/lib/libprop

2020-06-11 Thread Jason Thorpe


> On Jun 11, 2020, at 4:28 PM, Kamil Rytarowski  wrote:
> 
>> Repeating that statement doesn't make it true. The amount of legit
>> problems found by them is dwarfed by far by the number of false
>> positives seen. That's complete ignoring basic QoI issues like "where is
>> this actually triggered" and no "I know, shut up".
>> 
>> Joerg
>> 
> 
> Please list legitimate false positives. There is practically nothing
> like that possible for using deprecated APIs (at least kept longer
> term). Besides that, the report shall be lowered to warning (like it
> used to be for Clang).

Like it or not, __warn_references() is the thing we do in the NetBSD source 
tree, and I have other reasons for not marking them deprecated in the headers 
for the moment.  Clang can't cope, so I will change it so it's suppressed only 
for clang.

-- thorpej



Re: CVS commit: src

2020-06-07 Thread Jason Thorpe


> On Jun 7, 2020, at 1:57 PM, Joerg Sonnenberger  wrote:
> 
>> Example?
> 
> https://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/Function-Attributes.html#index-g_t_0040code_007bdeprecated_007d-attribute_002e-2502

I meant an example in our tree.  We use __warn_reference() in several places 
already.  I also don't want to introduce compiler warnings just yet.

-- thorpej



Re: CVS commit: src

2020-06-07 Thread Jason Thorpe


> On Jun 7, 2020, at 1:22 PM, Joerg Sonnenberger  wrote:
> 
> On Sat, Jun 06, 2020 at 09:26:00PM +, Jason R Thorpe wrote:
>> ==> Deprecate mutable prop_data(3) and prop_string(3) objects.  The old
>>APIs that support them still exist, but will now produce link-time
>>warnings when used.
> 
> Please replace them with proper deprecration annotation in the headers.

Example?

-- thorpej



Re: CVS commit: src

2020-06-07 Thread Jason Thorpe



> On Jun 6, 2020, at 10:51 PM, matthew green  wrote:
> 
> "Jason R Thorpe" writes:
>> Module Name: src
>> Committed By:thorpej
>> Date:Sat Jun  6 21:26:00 UTC 2020
>> 
>> Modified Files:
>>  src/lib/libprop: Makefile shlib_version
> 
> i wonder, since this adds to the kernel ABI, you should have
> also bumped the kernel version, since modules using this new
> version won't run on older versions of this version now.

I guess I could bump it... but I have some related kernel changes coming that 
will require a bump regardless.  Worth bumping twice?  I guess version #s are 
cheap.

-- thorpej



Re: CVS commit: src/sys/kern

2020-05-31 Thread Jason Thorpe



> On May 31, 2020, at 1:33 AM, Rin Okuyama  wrote:
> 
> db_show_callout(): struct callout_cpu and cpu_info are too much for stack.
> 
> XXX
> DDB can be running in the interrupt context, e.g., when activated from
> console. Therefore, use kmem_intr_alloc(9) instead of kmem_alloc(9).
> 
> Frame size, e.g. for m68k, becomes:
>9212 (oops!) --> 0
> 

I'm not sure we want to be calling memory allocators from within ddb.  I think 
it would be better to simply allocate that space in the .bss segment -- ddb 
only runs on 1 CPU at a time.

-- thorpej



Re: CVS commit: src/sys/arch/aarch64

2020-05-23 Thread Jason Thorpe


> On May 23, 2020, at 11:08 AM, Ryo Shimizu  wrote:
> 
> Module Name:  src
> Committed By: ryo
> Date: Sat May 23 18:08:59 UTC 2020
> 
> Modified Files:
>   src/sys/arch/aarch64/aarch64: cpufunc.c cpuswitch.S exec_machdep.c
>   genassym.cf netbsd32_machdep.c vectors.S vm_machdep.c
>   src/sys/arch/aarch64/include: armreg.h machdep.h proc.h
> 
> Log Message:
> Not only the kernel thread, but also the userland PAC keys
> (APIA,APIB,APDA,APDB,APGA) are now randomly initialized at exec, and switched
> when context switch.
> userland programs are able to perform pointer authentication on ARMv8.3+PAC 
> cpu.

Has any consideration be given to perhaps creating a new MACHINE_ARCH for this, 
or somehow otherwise decorating the ELF files to indicate their exec-ability?

> 
> reviewd by maxv@, thanks.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.18 -r1.19 src/sys/arch/aarch64/aarch64/cpufunc.c
> cvs rdiff -u -r1.20 -r1.21 src/sys/arch/aarch64/aarch64/cpuswitch.S
> cvs rdiff -u -r1.6 -r1.7 src/sys/arch/aarch64/aarch64/exec_machdep.c
> cvs rdiff -u -r1.24 -r1.25 src/sys/arch/aarch64/aarch64/genassym.cf
> cvs rdiff -u -r1.12 -r1.13 src/sys/arch/aarch64/aarch64/netbsd32_machdep.c
> cvs rdiff -u -r1.16 -r1.17 src/sys/arch/aarch64/aarch64/vectors.S
> cvs rdiff -u -r1.7 -r1.8 src/sys/arch/aarch64/aarch64/vm_machdep.c
> cvs rdiff -u -r1.44 -r1.45 src/sys/arch/aarch64/include/armreg.h
> cvs rdiff -u -r1.10 -r1.11 src/sys/arch/aarch64/include/machdep.h
> cvs rdiff -u -r1.6 -r1.7 src/sys/arch/aarch64/include/proc.h
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 

-- thorpej



Re: Freeze or panic during boot was: Re: CVS commit: src/sys/dev/ata

2020-05-02 Thread Jason Thorpe


> On May 1, 2020, at 1:07 PM, Ryo ONODERA  wrote:
> 
> Hi,
> 
> Have you missed this thread?
> 
> If the problem requires more time to investigate,
> could you consider to revert ata change for a while?
> 
> Thank you.

I backed it out, but would appreciate some help tracking down the issue because 
no other problems were reported other than on these specific machines.

-- thorpej



Re: CVS commit: src

2020-04-26 Thread Jason Thorpe


> On Apr 26, 2020, at 5:23 AM, Maxime Villard  wrote:
> 
> Because I modeled my tests after the ufetchstore and threadpool tests which 
> are
> both in this directory and provide user access to kernel internals via 
> modules,
> which is exactly what I'm doing.

..and those are there because, IIRC, that's where someone asked me to move them 
to.

-- thorpej


Re: CVS commit: src/sys/kern

2020-04-26 Thread Jason Thorpe



> On Apr 26, 2020, at 2:04 PM, Michael van Elst  wrote:
> 
> Log Message:
> fix DIAGNOSTIC build

non-DIAGNOSTIC you mean?

-- thorpej



Re: CVS commit: src/sys/rump

2020-04-25 Thread Jason Thorpe
Why were any of these files touched by Xen changes?

> On Apr 25, 2020, at 8:42 AM, Manuel Bouyer  wrote:
> 
> Module Name:  src
> Committed By: bouyer
> Date: Sat Apr 25 15:42:15 UTC 2020
> 
> Modified Files:
>   src/sys/rump: listsrcdirs
>   src/sys/rump/dev/lib/libumass: Makefile
>   src/sys/rump/fs/lib/libffs: Makefile
>   src/sys/rump/include/rump: rump_syscalls.h
>   src/sys/rump/librump/rumpkern: lwproc.c rump.c rump_syscalls.c sleepq.c
>   src/sys/rump/librump/rumpvfs: rump_vfs.c rumpfs.c
> 
> Log Message:
> Merge the bouyer-xenpvh branch, bringing in Xen PV drivers support under HVM
> guests in GENERIC.
> Xen support can be disabled at runtime with
> boot -c
> disable hypervisor
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.49 -r1.50 src/sys/rump/listsrcdirs
> cvs rdiff -u -r1.11 -r1.12 src/sys/rump/dev/lib/libumass/Makefile
> cvs rdiff -u -r1.18 -r1.19 src/sys/rump/fs/lib/libffs/Makefile
> cvs rdiff -u -r1.115 -r1.116 src/sys/rump/include/rump/rump_syscalls.h
> cvs rdiff -u -r1.47 -r1.48 src/sys/rump/librump/rumpkern/lwproc.c
> cvs rdiff -u -r1.345 -r1.346 src/sys/rump/librump/rumpkern/rump.c
> cvs rdiff -u -r1.146 -r1.147 src/sys/rump/librump/rumpkern/rump_syscalls.c
> cvs rdiff -u -r1.19 -r1.20 src/sys/rump/librump/rumpkern/sleepq.c
> cvs rdiff -u -r1.92 -r1.93 src/sys/rump/librump/rumpvfs/rump_vfs.c
> cvs rdiff -u -r1.157 -r1.158 src/sys/rump/librump/rumpvfs/rumpfs.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 

-- thorpej



Re: CVS commit: src/sys

2020-04-24 Thread Jason Thorpe


> On Apr 24, 2020, at 3:39 AM, Kamil Rytarowski  wrote:
> 
> This is a good idea (and preexisting in other kernels), unfortunately we
> had locking issues in rust. If a multithreaded process is forked, we
> shall create a replica of a calling thread and keep mutexes functional.
> We tried to preserve the caller's LWP to guarantee this.
> 
> This problem can be back. I don't ask to revert or revisit this change,
> but if it will be back, we shall find a solution for it.

This has been fixed by joerg@ in ld.elf_so already.

-- thorpej



Re: CVS commit: src

2020-04-22 Thread Jason Thorpe


> On Apr 22, 2020, at 7:52 AM, Kamil Rytarowski  wrote:
> 
> Would it be possible to keep a global unique TID as 64-bit integer
> (int64_t) equal to: pid << 32 | lwpid ?
> 
> That way we could have predicatable numbers in the system and possibly
> simplify the involved code avoiding one extra unique 32-bit id.

Yes, I've been discussing this w/ ad@, chs@, and riastradh@.  I have a 
different approach that's almost ready to go, and will remove this separate 
"TID".  The system call was not yet exposed in libc, so it's not a problem to 
pull it out.

For various reasons, the ID must be limited to less than 32 bits.

-- thorpej



Re: CVS commit: src/sys

2020-04-17 Thread Jason Thorpe


> On Apr 17, 2020, at 7:46 AM, Robert Elz  wrote:
> 
>Date:Fri, 17 Apr 2020 15:37:33 +0200
>From:Manuel Bouyer 
>Message-ID:  <20200417133733.ga5...@antioche.eu.org>
> 
>  | And that would be a problem for me. I regulary update a single file to a
>  | specific revision in a source tree.
> 
> Me too - I pull the current sh into NetBSD 8 (and I guess 9 now too,
> though I haven't done that yet) and build it there for some people who
> like to test and report bugs.

The New Hotness (which isn't particularly new, at this point) is to create 
branches and merge what you want into that branch.

-- thorpej



Re: CVS commit: src/sys

2020-04-17 Thread Jason Thorpe


> On Apr 17, 2020, at 6:28 AM, Rin Okuyama  wrote:
> 
> On 2020/04/17 22:14, Jason Thorpe wrote:
>>> On Apr 17, 2020, at 12:24 AM, Robert Elz  wrote:
>>> 
>>> For this, RCS and RCS semantics are irrelevant aren't they?
>> No, not really.  With the modern systems, the "commit ID" identifies the 
>> state of the entire collection of files, not individual ones.  Thus, you 
>> only need exactly one instance of the ID, not one ID per file.
> 
> Exactly, but at the same time I think that RCSID is still useful till
> we switch to a sane VCS with unique commit ID's.
> 
> The attached patch adds SHF_MERGE|SHF_STRINGS flags as Joerg suggested.
> I've confirmed that it works fine both for GCC/binutils and LLVM (for
> kernel and userland).
> 
> OK to commit, or objections?

I'm fine with this change as a workaround for the current issue with the 
RCS-style IDs, but it will still be a huge mistake to embed a "commit ID" into 
every file when we (eventually, hopefully) switch to a modern revision control 
system because it would obviously require every file with such an embedded to 
change on each commit, which seems like a really really bad idea.

> 
> Thanks,
> rin
> 

-- thorpej



Re: CVS commit: src/sys

2020-04-17 Thread Jason Thorpe


> On Apr 17, 2020, at 12:24 AM, Robert Elz  wrote:
> 
> For this, RCS and RCS semantics are irrelevant aren't they?

No, not really.  With the modern systems, the "commit ID" identifies the state 
of the entire collection of files, not individual ones.  Thus, you only need 
exactly one instance of the ID, not one ID per file.

-- thorpej



Re: CVS commit: src/sys

2020-04-16 Thread Jason Thorpe


> On Apr 16, 2020, at 5:51 PM, Joerg Sonnenberger  wrote:
> 
> 
> That can be fixed generically? .ident needs the SHF_MERGE|SHF_STRINGS as
> section flags and then the linker should do the rest by itself.

Does it matter?  The sooner we get off of "things that use RCS semantics" the 
better.

-- thorpej



Re: CVS commit: src/share/man/man4

2020-04-10 Thread Jason Thorpe


> On Apr 10, 2020, at 4:44 AM, m...@netbsd.org  wrote:
> 
> I had to stop using m_defrag because implementation details broke
> bwfm@pci. It can only handle a chain of length 1, and m_defrag gives
> a minimum of 2.

Exactly.  If it can compact the packet into a single cluster mbuf, it should do 
so.  It makes it slightly more difficult to use correctly, but those would be 
better semantics.

-- thorpej



Re: CVS commit: src/share/man/man4

2020-04-09 Thread Jason Thorpe



> On Apr 9, 2020, at 7:19 PM, SAITOH Masanobu  wrote:
> 
> You're welcome.
> Some drivers still have no m_defrag() code, so we should add it
> to them().

Others do something different than m_defrag() do essentially the same effect.  
Personally, I am not a huge fan of the m_defrag() API.

-- thorpej



Re: CVS commit: src/sys

2020-04-03 Thread Jason Thorpe


> On Apr 3, 2020, at 5:45 AM, Tetsuya Isaki  wrote:
> 
> By the way, I am planning the following.  How about this?

I very much dislike creating an additional header file for this.

I would prefer if the default were tuned for modern systems and overridable by 
a value exported (in a namespace-appropriate way) by a header that already 
exists on all platforms.

-- thorpej



Re: CVS commit: src/sys/modules/examples/fopsmapper

2020-04-01 Thread Jason Thorpe


> On Apr 1, 2020, at 7:17 AM, Christos Zoulas  wrote:
> 
> Which we have been slowly fixing. I think in this case the sign-compare
> warnings are annoying, but putting casts on each warning is cluttering
> the code needlessly. Unfortunately the alternative (to make the PAGESIZE
> constant unsigned is more dangerous -- unless of course you are willing to 
> examine all the places it is used and make sure that the semantics don't
> change). Another way would be to make:
> 
>#define PAGESIZE_U ((unsigned)PAGESIZE)

If PAGE_SIZE is ostensibly a vsize_t / size_t, why not define it as (1U << 
PAGE_SHIFT)?

-- thorpej



Re: CVS commit: src/sys

2020-03-29 Thread Jason Thorpe


> On Mar 29, 2020, at 8:03 AM, Joerg Sonnenberger  wrote:
> 
> Yes and I see no fundamental reason for not allowing buffering with 1ms
> frames in that case. I expect Alpha to be fast enough to do that with
> little overhead.

That's fine, I just wanted to point it out.

-- thorpej



Re: CVS commit: src/sys

2020-03-29 Thread Jason Thorpe


> On Mar 29, 2020, at 6:11 AM, Joerg Sonnenberger  wrote:
> 
> I would allow at least 1/HZ as baseline.

Be careful, some platforms have a relatively high HZ (e.g. Alpha HZ=1024).

-- thorpej



Re: CVS commit: src/external/gpl3

2020-03-25 Thread Jason Thorpe


> On Mar 25, 2020, at 5:26 PM, Kamil Rytarowski  wrote:
> 
> Upstream (GCC) is strongly against this change (even under __NetBSD__
> ifdef) as /var/tmp is typically larger than /tmp:
> 
>> I'd strongly recommend against this as-is.
>> 
>> The whole reason we prefer /var/tmp is because it's often dramatically
> larger
>> than a ram-backed /tmp.
> 
> -- by Jeff Law.

That's bonkers.  It needlessly hurts performance and, on modern systems, adds 
needless SSD writes.

-- thorpej



Re: CVS import: src/external/broadcom/bwfm/dist

2020-03-22 Thread Jason Thorpe


> On Mar 22, 2020, at 12:00 PM, Jason R Thorpe  wrote:
> 
> 3 conflicts created by this import.

Note: These are false conflicts that are the result of the files being "cvs 
add"ed rather than imported originally.  I verified before the import that all 
files that we currently distribute are identical to versions in the current 
linux-firmware snapshot.

-- thorpej



Re: Modularize sun2 kernel (Re: CVS commit: src/sys/arch/sun2/conf)

2020-03-14 Thread Jason Thorpe


> On Mar 14, 2020, at 8:10 PM, Jason Thorpe  wrote:
> 
> redundant things (e.g. extent vs vmem)

In case this wasn't obvious, I favor ejecting extent in favor of vmem.

-- thorpej



Re: Modularize sun2 kernel (Re: CVS commit: src/sys/arch/sun2/conf)

2020-03-14 Thread Jason Thorpe



> On Mar 14, 2020, at 7:57 PM, Rin Okuyama  wrote:
> 
> I think that we can no longer support 4MB system because of (2); hangs
> due to (2) are much more serious for 4MB system than it is in 7MB system.
> Modern kernel allocates too much things on demand rather than statically
> allocating them in data segment...

We should be striving to make that work.  There are some redundant things (e.g. 
extent vs vmem) that we should explore getting rid of... and find ways to trim 
optionally functionality so we can keep these old systems running.

-- thorpej



Re: CVS commit: src/sys/arch/mips/mips

2020-03-13 Thread Jason Thorpe


> On Mar 13, 2020, at 9:11 AM, Christos Zoulas  wrote:
> 
> I think this is better done in the driver, as other ports
> do the same check and it catches bugs.

x86 *explcitly* checks for 0 to skip work.  If you want to find bugs, change 
the most-often-used implementation maybe?

-- thorpej



Re: CVS commit: src/external/bsd/libarchive/dist/libarchive

2020-02-25 Thread Jason Thorpe
I repled to you and gson off-list.

> On Feb 25, 2020, at 8:24 AM, Kamil Rytarowski  wrote:
> 
> On 25.02.2020 16:24, Andreas Gustafsson wrote:
>> Kamil Rytarowski wrote:
>>> To generate a diff of this commit:
>>> cvs rdiff -u -r1.1.1.4 -r1.2 \
>>>src/external/bsd/libarchive/dist/libarchive/archive_read.c
>> 
>> What kind of sorcery is this?  Why is the diff not relative to 1.1?
>> 
> 
> I don't know. I was on the HEAD branch.
> 

-- thorpej



Re: CVS commit: src/sys

2020-02-08 Thread Jason Thorpe



> On Feb 8, 2020, at 12:45 PM, Andrew Doran  wrote:
> 
> Bit concerning that we're growing a ton of kthreads in the network stack (my
> test system now has something like 700 kthreads total) but I'm less worried
> about that and moreso that a lot of these seem to be in the kernel RT range
> and will therefore cause kernel preemptions when triggered.
> 
> For something like a link state change that's no biggie but I'm not fond of
> the idea of bulk wire & protcol processing needing mi_switch() + kpreempt(). 
> Way back when we made a concious decision not copy the design pattern our
> sister project went with on this front.  Maybe it is the right way to go now
> but I think that should also be a concious design decision.

Roger that.  I agree with you -- I would prefer "regular" stuff to continue 
using softints.  For some other stuff, though, we've made the policy decision 
that softints can't block for anything other than an adaptive mutex, which is 
fine... this particular case sends a message to the routing socket, which 
requires gathering interface statistics, which blocks on something other than 
an adaptive mutex.

I'm not a fan of the workqueue interface, and I have something better sitting 
unfinished in my Bucket Of Stuff I'm Working On ... a generalized "task" 
interface originally written-but-not-finished by Taylor that uses either thread 
pools or softints to do the work... so at least the threads won't linger around 
in the "mostly idle" case.

-- thorpej



Re: CVS commit: src/sys/net

2020-01-28 Thread Jason Thorpe


> On Jan 28, 2020, at 10:25 PM, Kamil Rytarowski  wrote:
> 
> The distribution build breaks for me with:

Should be fixed with:

 /cvsroot/src/sys/rump/net/lib/libnet/Makefile,v  <--  Makefile
 new revision: 1.33; previous revision: 1.32

> /public/netbsd-root/tooldir.NetBSD-9.99.38-amd64/lib/gcc/x86_64--netbsd/8.3.0/../../../../x86_64--netbsd/bin/ld:
> /public/netbsd-root/destdir.amd64/usr/lib/librumpnet_net.so: undefined
> reference to `rumpns_if_stats_init'
> /public/netbsd-root/tooldir.NetBSD-9.99.38-amd64/lib/gcc/x86_64--netbsd/8.3.0/../../../../x86_64--netbsd/bin/ld:
> /public/netbsd-root/destdir.amd64/usr/lib/librumpnet_net.so: undefined
> reference to `rumpns_if_stats_fini'
> /public/netbsd-root/tooldir.NetBSD-9.99.38-amd64/lib/gcc/x86_64--netbsd/8.3.0/../../../../x86_64--netbsd/bin/ld:
> /public/netbsd-root/destdir.amd64/usr/lib/librumpnet_net.so: undefined
> reference to `rumpns_if_stats_to_if_data'
> 

-- thorpej



Re: CVS commit: src/sys/arch/evbarm/conf

2020-01-25 Thread Jason Thorpe



> On Jan 25, 2020, at 6:31 AM, Christos Zoulas  wrote:
> 
>> This seems a little silly to have in the kernel configuration file.  I
>> think there's an argument to be made that there should be a header that
>> sets these defaults that can be tuned per-platform (or even some
>> functionality to tune this at run-time).
> 
> sysctl :-)

Shouldn't HAVE to know about the sysctl, either.  It should default to "good 
performance".

-- thorpej



Re: CVS commit: src/sys/arch/evbarm/conf

2020-01-25 Thread Jason Thorpe


> On Jan 25, 2020, at 4:26 AM, Jared D. McNeill  wrote:
> 
> Module Name:  src
> Committed By: jmcneill
> Date: Sat Jan 25 12:26:58 UTC 2020
> 
> Modified Files:
>   src/sys/arch/evbarm/conf: GENERIC GENERIC64
> 
> Log Message:
> Follow amd64 and set AUDIO_BLK_MS=4 by default

This seems a little silly to have in the kernel configuration file.  I think 
there's an argument to be made that there should be a header that sets these 
defaults that can be tuned per-platform (or even some functionality to tune 
this at run-time).

-- thorpej



Re: CVS commit: src/sys [freeze on boot]

2020-01-20 Thread Jason Thorpe


> On Jan 20, 2020, at 3:44 PM, Christos Zoulas  wrote:
> 
> In article <20200120185023.gd28...@homeworld.netbsd.org>,
> Andrew Doran   wrote:
>> Fix committed with sys/kern/kern_rwlock.c rev 1.62.  I didn't see the
>> problem as I am running with LOCKDEBUG.
>> 
>> Apologies for the disruption.
> 
> FYI: powerpc/arm do not build anymore...

This should fix the powerpc problem:

Index: lock_stubs.S
===
RCS file: /cvsroot/src/sys/arch/powerpc/powerpc/lock_stubs.S,v
retrieving revision 1.10
diff -u -p -r1.10 lock_stubs.S
--- lock_stubs.S28 Feb 2014 05:38:15 -  1.10
+++ lock_stubs.S21 Jan 2020 04:09:26 -
@@ -101,8 +101,8 @@ ENTRY(mutex_exit)
 /*
  * void rw_enter(krwlock_t *krw, krw_t op);
  */
-#if RW_READ_INCR != 16
-#error RW_READ_INCR != 16, clrrXi need fixing
+#if RW_READ_INCR != 32
+#error RW_READ_INCR != 32, clrrXi need fixing
 #endif
 #if RW_OWNER != 0
 #error RW_OWNER != 0, ldptr should be ldptru
@@ -115,7 +115,7 @@ ENTRY(rw_enter)
bne-1f
 
ldptr   %r9,RW_OWNER(%r3)
-   clrrptri %r9,%r9,4  /* clear low 4 bits */
+   clrrptri %r9,%r9,5  /* clear low 5 bits */
addi%r7,%r9,RW_READ_INCR
b   2f
 1:
@@ -140,7 +140,7 @@ ENTRY(rw_tryenter)
bne-1f
 
ldptr   %r9,RW_OWNER(%r3)
-   clrrptri %r9,%r9,4  /* clear low 4 bits */
+   clrrptri %r9,%r9,5  /* clear low 5 bits */
addi%r7,%r9,RW_READ_INCR
b   2f
 
@@ -169,7 +169,7 @@ ENTRY(rw_exit)
andi.   %r0,%r9,RW_WRITE_LOCKED
bne-1f
 
-   clrrptri. %r9,%r9,4 /* clear low 4 bits */
+   clrrptri. %r9,%r9,5 /* clear low 5 bits */
beq-3f  /* if 0, no reader, go slow */
 
addi%r7,%r9,-RW_READ_INCR

> 
> http://releng.netbsd.org/builds/HEAD/202001201020Z/
> 
> christos
> 

-- thorpej



Re: CVS commit: src/sys [freeze on boot]

2020-01-20 Thread Jason Thorpe



> On Jan 20, 2020, at 6:48 AM, Ryo ONODERA  wrote:
> 
> The black screen and ims(4) panic are not related to your change.
> Older src tree with LOCKDEBUG reproduces these problem.

I'll look at the ims(4) issuer.

-- thorpej



Re: MAX_PAGE_SIZE for m68k (Re: CVS commit:src/sys/arch/arm/include/arm32)

2020-01-14 Thread Jason Thorpe



> On Jan 14, 2020, at 2:37 PM, Christos Zoulas  wrote:
> 
> So what's the short term solution?
> 
> - Don't define {MIN,MAX}_PAGE_SIZE on m68k?
> - Define a different constant?
> - Add a define to tell uvm to ignore {MIN,MAX}_PAGE_SIZE?

#ifdef _KERNEL, define {MIN,MAX}_PAGE_SIZE to a constant that matches the 
system.  For !_KERNEL, define MIN_PAGE_SIZE as 4096 and MAX_PAGE_SIZE as 8192.

Seems like that would do the trick.

-- thorpej



Re: MAX_PAGE_SIZE for m68k (Re: CVS commit:src/sys/arch/arm/include/arm32)

2020-01-14 Thread Jason Thorpe



> On Jan 14, 2020, at 10:16 AM, Christos Zoulas  wrote:
> 
> We do this to save space, but as you say, not important for now. Perhaps
> the expedient solution is to define MALLOC_PAGE_SIZE...

I'd rather keep the use of these constants separate... who knows what they 
might be used for in the future?  Optimize #ifdef _KERNEL as-needed...

-- thorpej



Re: MAX_PAGE_SIZE for m68k (Re: CVS commit:src/sys/arch/arm/include/arm32)

2020-01-14 Thread Jason Thorpe



> On Jan 14, 2020, at 8:41 AM, Izumi Tsutsui  wrote:
> 
>> b) Modules should be built such that they can use a non-fixed PAGE_SIZE.
> 
> No, this is not necessary, because modules are built for each $MACHINE
> and (a) each $MACHINE has fixed PAGE_SIZE.

Yes, understood.  I think it would eventually be a nice idea to make modules 
$MACHINE_ARCH-sharable, but it's not critical for now.

-- thorpej



Re: MAX_PAGE_SIZE for m68k (Re: CVS commit: src/sys/arch/arm/include/arm32)

2020-01-13 Thread Jason Thorpe



> On Jan 13, 2020, at 8:15 AM, Izumi Tsutsui  wrote:
> 
> christos@ wrote:
> 
>>> Now I get the following erro during local tests of
>>> "build.sh -U -m hp300 release" on NetBSD/i386 9.0_RC1 host:
>>> 
>>> ---
>>> #create  compat_util/compat_exec.d
> :
>>> In file included from /s/cvs/src/sys/sys/param.h:149:0,
>>>from /s/cvs/src/sys/compat/common/compat_exec.c:41:
>>> ./m68k/pmap_motorola.h:165:5: error: operator '*' has no left operand
>>> #if PAGE_SIZE == 8192 /* NBPG / (SG4_LEV1SIZE * sizeof(st_entry_t)) */
>>>^
>>> nbmkdep: compile failed.
>>> *** [compat_exec.d] Error code 1
>> 
>> try cc -E?
> 
> It turns out the problem is more complicated.
> 
>  has the following definitions:
> 
> https://nxr.netbsd.org/xref/src/sys/uvm/uvm_param.h?r=1.38#135

I would suggest a possible workaround for this could be to define an additional:

#define __HAVE_FIXED_PAGE_SIZE

...at could also be tested here... the kernel truly does have a fixed page 
size, but userland not necessarily so.

> ---
>135  * If MIN_PAGE_SIZE and MAX_PAGE_SIZE are not equal, then we must use
>136  * non-constant PAGE_SIZE, et al for LKMs.
>137  */
>138 #if (MIN_PAGE_SIZE != MAX_PAGE_SIZE)
>139 #define__uvmexp_pagesize
>140 #if defined(_LKM) || defined(_MODULE)
>141 #undef PAGE_SIZE
>142 #undef PAGE_MASK
>143 #undef PAGE_SHIFT
>144 #endif
>145 #endif
>146 
>147 /*
>148  * Now provide PAGE_SIZE, PAGE_MASK, and PAGE_SHIFT if we do not
>149  * have ones that are compile-time constants.
>150  */
>151 #if !defined(PAGE_SIZE)
>152 extern const int *const uvmexp_pagesize;
>153 extern const int *const uvmexp_pagemask;
>154 extern const int *const uvmexp_pageshift;
>155 #definePAGE_SIZE   (*uvmexp_pagesize)  /* size of page 
> */
>156 #definePAGE_MASK   (*uvmexp_pagemask)  /* size of page 
> - 1 */
>157 #definePAGE_SHIFT  (*uvmexp_pageshift) /* bits to 
> shift for pages */
>158 #endif /* PAGE_SIZE */
> ---
> 
> I.e.  assumes PAGE_SIZE is not compile time constant
> for MODULEs if (MIN_PAGE_SIZE != MAX_PAGE_SIZE).
> 
> Probably this is the same reason of recent arm build failures:
> https://releng.netbsd.org/builds/HEAD/202001130720Z/
> https://releng.netbsd.org/builds/HEAD/202001130720Z/evbarm-earm.build.failed
> ---
> /tmp/genassym.Lq8h9d/assym.c:57:1: error: asm operand 0 probably doesn't 
> match constraints [-Werror]
> __asm("XYZZY VM_MIN_ADDRESS %0" : : "n" (VM_MIN_ADDRESS));
> ^
> /tmp/genassym.Lq8h9d/assym.c:58:1: error: asm operand 0 probably doesn't 
> match constraints [-Werror]
> __asm("XYZZY VM_MAXUSER_ADDRESS %0" : : "n" (VM_MAXUSER_ADDRESS));
> ^
> /tmp/genassym.Lq8h9d/assym.c:97:1: error: asm operand 0 probably doesn't 
> match constraints [-Werror]
> __asm("XYZZY PAGE_MASK %0" : : "n" (PAGE_MASK));
> ^
> /tmp/genassym.Lq8h9d/assym.c:98:1: error: asm operand 0 probably doesn't 
> match constraints [-Werror]
> __asm("XYZZY PAGE_SIZE %0" : : "n" (PAGE_SIZE));
> ^
> ---
> 
> Should we prepare independent constant for
> "possible pagesize value among different MACHINE with the same MACHINE_ARCH"
> for jemalloc(3)?
> 
> ---
> Izumi Tsutsui

-- thorpej



Re: CVS commit: src/sys

2020-01-13 Thread Jason Thorpe


> On Jan 12, 2020, at 10:20 PM, Kamil Rytarowski  wrote:
> 
> While there, could we garbage collect unused defines from sys/param.h?
> 
> I'm thinking in particular about:

As long as we still have tsleep(9) and friends, we can't rid ourselves of these 
historical defines.

Perhaps we should make a push to rid ourselves of those legacy interfaces?


> 
> /*
> * Historic priority levels.  These are meaningless and remain only
> * for source compatibility.  Do not use in new code.
> */
> #define PSWP0
> #define PVM 4
> #define PINOD   8
> #define PRIBIO  16
> #define PVFS20
> #define PZERO   22
> #define PSOCK   24
> #define PWAIT   32
> #define PLOCK   36
> #define PPAUSE  40
> #define PUSER   50
> #define MAXPRI  127
> 
> These symbols from time to time conflict with external loadable kernel
> modules. My immediate offender is VirtualBox and PVM symbol name conflict.
> 

-- thorpej



Re: CVS commit: src/sys/arch/x86

2020-01-12 Thread Jason Thorpe
We should absolutely verify this under DEBUG.

-- thorpej
Sent from my iPhone.

> On Jan 12, 2020, at 11:25 AM, Joerg Sonnenberger  wrote:
> 
> On Sun, Jan 12, 2020 at 01:01:12PM +, Andrew Doran wrote:
>> Module Name:src
>> Committed By:ad
>> Date:Sun Jan 12 13:01:12 UTC 2020
>> 
>> Modified Files:
>>src/sys/arch/x86/include: pmap.h pmap_pv.h
>>src/sys/arch/x86/x86: pmap.c vm_machdep.c x86_tlb.c
>> 
>> Log Message:
>> x86 pmap:
>> 
>> - It turns out that every page the pmap frees is necessarily zeroed.  Tell
>>  the VM system about this and use the pmap as a source of pre-zeroed pages.
> 
> Would it make sense to actually verify this under DEBUG? I fear the
> debugging we have to do if a chance ever violates this assumption.
> 
> Joerg



Re: CVS commit: src/sys/arch/arm/include/arm32

2020-01-12 Thread Jason Thorpe



> On Jan 11, 2020, at 10:47 PM, Izumi Tsutsui  wrote:
> 
> thorpej@ wrote:
> 
>>> PGSHIFT is defined in  and
>>> PAGE_SHIFT and PAGE_SIZE is in ,
>>> but there is no common .
>> 
>> Make a common  that all of the platforms can #include, and 
>> just put these common definitions in it (for now)?
> 
> How about the attached diff? (untested, just for review)

This looks good to me.

-- thorpej



Re: CVS commit: src/sys/arch/arm/include/arm32

2020-01-11 Thread Jason Thorpe



> On Jan 11, 2020, at 8:32 PM, Izumi Tsutsui  wrote:
> 
> PGSHIFT is defined in  and
> PAGE_SHIFT and PAGE_SIZE is in ,
> but there is no common .

Make a common  that all of the platforms can #include, and just 
put these common definitions in it (for now)?

-- thorpej



Re: CVS commit: src/sys/dev/pci

2020-01-09 Thread Jason Thorpe



> On Jan 9, 2020, at 9:02 PM, Masanobu SAITOH  wrote:
> 
> The reason why I moved stge_softc to if_stgereg.h was that ipgphy.c
> required to check stge's chip revision. So, if there is no any objection,
> I'll make new if_stgevar.h and share it with if_stge.c and ipgphy.c.

That seems fine.  Although it might be preferable to set a property on the 
parent dev_t and then query that from ipgphy, rather than accessing the softc.

-- thorpej



Re: CVS commit: src/sys/dev/pci

2020-01-09 Thread Jason Thorpe



> On Jan 9, 2020, at 6:11 PM, Masanobu SAITOH  wrote:
> 
> 
> Before my change, struct stge_softc is already in if_stgereg.h,
> so I had thought it would be OK to move to it.

Ah, I don't think I would have put it there when I wrote the driver originally, 
so it must have been moved there at some point.  In any case, moving it into 
if_stgereg.h was also an error.

> 
>> Please move them back to if_stge.c.  Doing incorrect things simply to reduce 
>> the diff against someone else's copy of the code is not something we should 
>> be undertaking.
> Two options:
> 
> a) Move some structs (including struct stge_softc) and defines
>to if_stge.c

There is no reason to have if_stgevar.h, because no other modules need these 
definitions, so it should all move to if_stge.c.

Thanks!

> 
> b) Move them to new if_stgevar.h
> 
> Which one is prefer?

-- thorpej



Re: CVS commit: src/sys/dev/pci

2020-01-09 Thread Jason Thorpe



> On Jan 9, 2020, at 2:54 AM, SAITOH Masanobu  wrote:
> 
> Module Name:  src
> Committed By: msaitoh
> Date: Thu Jan  9 10:54:16 UTC 2020
> 
> Modified Files:
>   src/sys/dev/pci: if_stge.c if_stgereg.h
> 
> Log Message:
> Reduce diff against OpenBSD. No functional change.
> 
> - USE CSR_{READ,WRITE}_*() macro.
> - Move some macros from if_stge.c to if_stgereg.h

This diff is incorrect.  The macros that were moved to if_stgereg.h are not 
related to hardware / register definitions, but are purely software constructs. 
 Please move them back to if_stge.c.  Doing incorrect things simply to reduce 
the diff against someone else's copy of the code is not something we should be 
undertaking.

-- thorpej



Re: CVS commit: src/sys

2020-01-03 Thread Jason Thorpe



> On Jan 3, 2020, at 5:08 PM, Simon Burge  wrote:
> 
> Hey Jason,
> 
> Jason Thorpe wrote:
> 
>>> On Jan 3, 2020, at 10:11 AM, Frank Kardel  wrote:
>>> 
>>> Hi Jason !
>>> 
>>> Could you please check that kmem using tools can cope with the missing 
>>> _boottime symbol.
>> 
>> Hey Frank... this should fix it:
>> 
>>  $NetBSD: vmstat.c,v 1.231 2020/01/03 19:13:54 thorpej Exp $
> 
> Is it worth keeping the boottime symbol about so that a netbsd-9 vmstat
> binary will still work with a -current kernel?  We could possibly wrap
> boottime with a COMPAT_90 check too?.

Define "work".  A dummy symbol would have no valid contents.  I guess if you 
call that "work" then go ahead?

The real problem here is that vmstat insists on finding the kernel symbols even 
if it's not going to use them, which is kind of silly.

-- thorpej



Re: CVS commit: src/sys

2020-01-03 Thread Jason Thorpe



> On Jan 3, 2020, at 10:11 AM, Frank Kardel  wrote:
> 
> Hi Jason !
> 
> Could you please check that kmem using tools can cope with the missing 
> _boottime symbol.

Hey Frank... this should fix it:

$NetBSD: vmstat.c,v 1.231 2020/01/03 19:13:54 thorpej Exp $

-- thorpej



Re: CVS commit: src/sys/arch

2019-12-23 Thread Jason Thorpe



> On Dec 23, 2019, at 7:53 PM, Taylor R Campbell 
>  wrote:
> 
>> Module Name:src
>> Committed By:   thorpej
>> Date:   Sun Dec 22 15:09:39 UTC 2019
>> 
>> Add intr_mask() and corresponding intr_unmask() calls that allow specific
>> interrupt lines / sources to be masked as needed (rather than making a
>> set of sources by IPL as with spl*()).
>> 
>> +   if (ci == curcpu() || !mp_online) {
>> +   intr_hwunmask_xcall(ih, NULL);
>> +   } else {
>> +   where = xc_unicast(0, intr_hwunmask_xcall, ih, NULL, ci);
>> +   xc_wait(where);
>> +   }
> 
> If this conditional is necessary, we should teach xc_unicast to make
> it unnecessary.

Agreed.  I followed the existing patter in intr.c.  Andrew pointed out in code 
review that the xcall code does at the the !mp_online bit now, but:

1- It does not do the curcpu() check.
2- It also fiddles with interrupt state.

For that reason, I was hesitant to make that change until further investigation 
was done.

-- thorpej



Re: CVS commit: src/sys/uvm

2019-12-23 Thread Jason Thorpe



> On Dec 23, 2019, at 7:22 PM, Taylor R Campbell 
>  wrote:
> 
> So I guess we won't be switching pg->phys_addr from paddr to pfn?

If that's the case, we should rename the field.

-- thorpej



Re: CVS commit: src/sys/kern

2019-12-09 Thread Jason Thorpe


> On Dec 9, 2019, at 1:08 PM, Paul Goyette  wrote:
> 
> On Mon, 9 Dec 2019, Andrew Doran wrote:
> 
>> Module Name: src
>> Committed By:ad
>> Date:Mon Dec  9 21:05:23 UTC 2019
>> 
>> Modified Files:
>>  src/sys/kern: kern_mutex.c
>> 
>> Log Message:
>> - Add a mutex_owner_running() for the benefit of the pagedaemon, which
>> needs help with locking things in reverse order.
> 
> Should this be added to the mutex(9) man page?

Honestly, I think it should be protectively wrapped in something like 
__MUTEX_PRIVATE or some such, because it's not really something that things 
should be using except under very specialized circumstances.

-- thorpej



Re: CVS commit: src/sys/arch/arm/broadcom

2019-11-28 Thread Jason Thorpe


> On Nov 28, 2019, at 2:21 AM, Jared McNeill  wrote:
> 
> I should have commented the code in gicv3 so that you would have realized it 
> was “unnecessarily complicated” for a reason :)

Ok, I'll fix and add a comment.

> 
> The interrupt_distribute(9) API makes an assumption that the return value of 
> anything_intr_establish can be used as input for the MD interrupt_distribute 
> implementation. For arm pic, struct intrsource * made most sense. It is a bit 
> of a hassle for fdtbus and really shows the need for an MI interrupt API.
> 
> So where this breaks things is if you have something in the kernel do:
> 
>   Ih = fdtbus_intr_establish(…)
>   interrupt_distribute(ih, target, NULL)
> 
> Currently the only place where this can happen is arch/arm/fdt/pmu_fdt.c but 
> IMHO the code is perfectly valid and could appear in other places in the 
> future.

-- thorpej



Re: case-sensitive APFS volumes

2019-11-13 Thread Jason Thorpe



> On Nov 13, 2019, at 1:57 AM, Christoph Badura  wrote:
> 
> Just HTF does one add the same cryptographic users that the / volume has?
> TFM does not tell.  It just says that "indeed, they should be added before
> encryption".

Hm, I'm not actually sure.  I'll do a bit of research.

> Maybe we can document this for the benfit of other NetBSD developers.
> 
> And yes, I do *not* want to dink around in the Disk Utility GUI.
> Automation or GTFO.

All of these things should be possible with the diskutil command.

-- thorpej



Re: CVS commit: src/sys/dev/ic

2019-09-21 Thread Jason Thorpe
Should we make a PRIxxx macro for it?

-- thorpej
Sent from my iPhone.

> On Sep 21, 2019, at 5:57 AM, Robert Elz  wrote:
> 
> Module Name:src
> Committed By:kre
> Date:Sat Sep 21 12:57:25 UTC 2019
> 
> Modified Files:
>src/sys/dev/ic: mpt.c
> 
> Log Message:
> bus_addt_t is different widths on different archs, so there is no
> one simple %?x format that will always work to print it.  Cast to
> intmax_t and use %jx which should work everywhere.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.19 -r1.20 src/sys/dev/ic/mpt.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 


Re: Leak Sanitizer - how to suppress leaks

2019-09-12 Thread Jason Thorpe


> On Sep 12, 2019, at 11:09 AM, Robert Elz  wrote:
> 
> To me it seems apparent that the sanatiser is detecting the local variable
> in main() go out of scope when main() returns, and so the value it contains
> (the pointer to the allocated memory) is lost, and so it is determined that
> there is a leak.   For any other function that would often be true, but for
> main() it never is.

It seems obvious that the sanitizer should special-case main().

-- thorpej



Re: CVS commit: src

2019-09-03 Thread Jason Thorpe


> On Sep 3, 2019, at 5:04 AM, Sevan Janiyan  wrote:
> 
> On 03/09/2019 12:59, Brad Spencer wrote:
>> One possible alternative to that is to install OpenZFS on MacOS and
>> create a ZFS filesystem inside of whatever...
> 
> Or a disk image which is case sensitive (hfs/apfs) problem is then that
> it's slow.

If your Mac has APFS, it's not quite that bad -- you can create a new file 
system that's case-insensitive inside the same APFS container as the file 
system with your home directory and space-share with that file system (the 
default behavior).  You can do this all within Disk Utility without having to 
remember any command line magic.  There is not a performance penalty for this.  
And the case-sensitive path is a well-tested, since that's what iOS uses.

But there is definitely an annoyance penalty.  It's an extra step (or two) that 
shouldn't be necessary.  At least not without announcing the deprecation of 
support for case-insensitive host platforms, and providing a transition period 
for people rather than letting them be surprised (and then stuck) when they 
went to update their source tree.

For Macs that aren't on APFS (I have an iMac at home in this situations that I 
build on all the time), they're just screwed by this change.  Essentially, any 
Mac model that was not supported by macOS 10.14 is possibly stuck with HFS+ 
(this is because 10.13 only converted all-flash Macs to APFS; systems with HDDs 
remained on HFS+).

While we're at it, are we going to say that pkgsrc is now case-sensitive-only, 
too?  And if not, then why are we being lazy about src?

-- thorpej



Re: CVS commit: src

2019-09-03 Thread Jason Thorpe



> On Sep 2, 2019, at 10:36 PM, Robert Elz  wrote:
> 
> I doubt that any of them really are truly
> case insensitive ... rather than are insenstive to the case of ascii
> chars, and that's usually it.

APFS on macOS is truly case-insensitive, not just for ASCII.  FWIW.

-- thorpej



Re: CVS commit: src/sys

2019-07-23 Thread Jason Thorpe
> On Jul 23, 2019, at 10:07 AM, Jason Thorpe  wrote:
> 
> 
> 
>> On Jul 23, 2019, at 9:14 AM, Rin Okuyama  wrote:
>> 
>> Ah, you are right. We leaks uninitialized memory via mmap.
>> 
>> However, I'm not sure that it is safe to write DMA buffer
>> above sc->sc_vramsize after bus_dmamap_load?
>> 
>> Thoughts, ARM experts?

Also, didn't Jared make a change to the Allwinner u-boot configs that ensure 
the frame buffer is page-aligned?  Should we make that change everywhere?

> 
> Since fundamental memory allocation is done on page boundaries, would it 
> suffice to simply ensure that the "leaked" memory is zero'd out first?  I 
> mean, nothing else can use it for anything meaninful...
> 
> -- thorpej
> 

-- thorpej



Re: CVS commit: src/sys

2019-07-23 Thread Jason Thorpe



> On Jul 23, 2019, at 9:14 AM, Rin Okuyama  wrote:
> 
> Ah, you are right. We leaks uninitialized memory via mmap.
> 
> However, I'm not sure that it is safe to write DMA buffer
> above sc->sc_vramsize after bus_dmamap_load?
> 
> Thoughts, ARM experts?

Since fundamental memory allocation is done on page boundaries, would it 
suffice to simply ensure that the "leaked" memory is zero'd out first?  I mean, 
nothing else can use it for anything meaninful...

-- thorpej



Re: CVS commit: src/sys/compat/sys

2019-06-28 Thread Jason Thorpe


> On Jun 26, 2019, at 7:10 PM, matthew green  wrote:
> 
>> Always include the 32 bit structure and definitions on _LP64 regardless
>> of compat32 being on or off, because we want the headers to work when
>> compiling modular kernels. Of course the 32 bit structs do not make sense
>> on platforms that don't have 32 bit modes (alpha), but we don't have
>> a define for that and it does not hurt.
> 
> i've been using _LP64 && !__alpha__ for this when it strikes.
> 
> sub-optimal, but also easy to grep and find :-)

Perhaps we should define "_LP64_ONLY" in  for this type of 
situation?

-- thorpej



Re: CVS commit: src/sys/arch/x86/x86

2019-05-15 Thread Jason Thorpe



> On May 15, 2019, at 9:19 PM, Maxime Villard  wrote:
> 
> Le 16/05/2019 à 04:36, SAITOH Masanobu a écrit :
>> Module Name: src
>> Committed By:msaitoh
>> Date:Thu May 16 02:36:30 UTC 2019
>> Modified Files:
>>  src/sys/arch/x86/x86: procfs_machdep.c
>> Log Message:
>>  Use ci_feat_val[7] instead of directly getting cpuid 7 edx.
>> To generate a diff of this commit:
>> cvs rdiff -u -r1.28 -r1.29 src/sys/arch/x86/x86/procfs_machdep.c
>> Please note that diffs are not public domain; they are subject to the
>> copyright notices on the relevant files.
> 
> The microcode updates CPUID7, if you read ci_feat_val you have the
> initial value, not the updated value. So reading CPUID7 was the right
> thing to do.

Should the microcode update procedure fix up the cached copy?

-- thorpej



Re: CVS commit: src/sys

2019-05-13 Thread Jason Thorpe



> On May 13, 2019, at 7:17 AM, Greg Troxel  wrote:
> 
> 2) Your option 2 seems to involve two things at once:
> 
>  - migration to lwp_specificadata
>  - using DEBUG instead of DIAGNOSTIC to control the leak check feature
> 
> I do not understand why changing the nature of the implementation is
> linked to how it is enabled.

I think Ozaki-san saying that the 3% performance hit only happens when 
lwp_specificdata is used, and hence that it would need to be wrapped in DEBUG 
rather than DIAGNOSTIC.

The original negligible-impact implementation did NOT use lwp_specificdata, and 
thus was fine for DIAGNOSTIC.  I believe Ozaki-san's preference is to use 
*this* implementation so that it can be exposed to a wider audience.  The 
lwp_specificdata approach was only explored after someone else suggested a 
preference for it.

At least, that's my understanding of the situation.

Now, choose wisely :-)

-- thorpej



Re: CVS commit: src/tools

2019-05-07 Thread Jason Thorpe



> On May 7, 2019, at 3:52 PM, Herbert J. Skuhra  wrote:
> 
> On Tue, 07 May 2019 12:23:12 +0200, "J. Hannken-Illjes" wrote:
>> 
>> 
>>> On 7. May 2019, at 12:17, matthew green  wrote:
>>> 
 Diff is NOT reversed.
>>> 
>>> ah yes, i see.  please commit.
> 
> Crossbuilding is still broken.

I just checked in a fix for this.

> 
> On FreeBSD 12.0-STABLE (amd64) and Arch GNU/Linux building earmv7hf and amd64 
> I get:
> 
> #   compile  libprop/prop_dictionary.lo
> cc -O   -I/usr/home/herbert/source/netbsd/src/tools/libprop/../compat 
> -I/usr/home/herbert/source/netbsd/src/tools/libprop/../../common/include 
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include/nbinclude
>  -DHAVE_NBTOOL_CONFIG_H=1 -D_FILE_OFFSET_BITS=64  
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include 
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include/nbinclude
>  -c -o prop_dictionary.lo.o
> /usr/home/herbert/source/netbsd/src/tools/libprop/../../common/lib/libprop/prop_dictionary.c
> 
> /usr/home/herbert/source/netbsd/src/tools/libprop/../../common/lib/libprop/prop_dictionary.c:37:10:
>  fatal error: 'sys/rbtree.h' file not found 
> #include
>  
> ^~
> 
> 1 error generated.
>  
> 
> *** Failed target:  prop_dictionary.lo
> *** Failed command: cc -O 
> -I/usr/home/herbert/source/netbsd/src/tools/libprop/../compat 
> -I/usr/home/herbert/source/netbsd/src/tools/libprop/../../common/include 
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include/nbinclude
>  -DHAVE_NBTOOL_CONFIG_H=1 -D_FILE_OFFSET_BITS=64 
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include 
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include/nbinclude
>  -c -o prop_dictionary.lo.o 
> /usr/home/herbert/source/netbsd/src/tools/libprop/../../common/lib/libprop/prop_dictionary.c
>  
> *** Error code 1
> 
> Stop.
> 
> -- 
> Herbert

-- thorpej



Re: CVS commit: src/tools

2019-05-07 Thread Jason Thorpe
I'll take a look at this this evening.

> On May 7, 2019, at 3:52 PM, Herbert J. Skuhra  wrote:
> 
> On Tue, 07 May 2019 12:23:12 +0200, "J. Hannken-Illjes" wrote:
>> 
>> 
>>> On 7. May 2019, at 12:17, matthew green  wrote:
>>> 
 Diff is NOT reversed.
>>> 
>>> ah yes, i see.  please commit.
> 
> Crossbuilding is still broken.
> 
> On FreeBSD 12.0-STABLE (amd64) and Arch GNU/Linux building earmv7hf and amd64 
> I get:
> 
> #   compile  libprop/prop_dictionary.lo
> cc -O   -I/usr/home/herbert/source/netbsd/src/tools/libprop/../compat 
> -I/usr/home/herbert/source/netbsd/src/tools/libprop/../../common/include 
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include/nbinclude
>  -DHAVE_NBTOOL_CONFIG_H=1 -D_FILE_OFFSET_BITS=64  
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include 
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include/nbinclude
>  -c -o prop_dictionary.lo.o
> /usr/home/herbert/source/netbsd/src/tools/libprop/../../common/lib/libprop/prop_dictionary.c
> 
> /usr/home/herbert/source/netbsd/src/tools/libprop/../../common/lib/libprop/prop_dictionary.c:37:10:
>  fatal error: 'sys/rbtree.h' file not found 
> #include
>  
> ^~
> 
> 1 error generated.
>  
> 
> *** Failed target:  prop_dictionary.lo
> *** Failed command: cc -O 
> -I/usr/home/herbert/source/netbsd/src/tools/libprop/../compat 
> -I/usr/home/herbert/source/netbsd/src/tools/libprop/../../common/include 
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include/nbinclude
>  -DHAVE_NBTOOL_CONFIG_H=1 -D_FILE_OFFSET_BITS=64 
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include 
> -I/usr/home/herbert/source/netbsd/src/../build/earmv7hf/tools/include/nbinclude
>  -c -o prop_dictionary.lo.o 
> /usr/home/herbert/source/netbsd/src/tools/libprop/../../common/lib/libprop/prop_dictionary.c
>  
> *** Error code 1
> 
> Stop.
> 
> -- 
> Herbert

-- thorpej



Re: CVS commit: src/tools

2019-05-07 Thread Jason Thorpe



> On May 7, 2019, at 12:32 PM, matthew green  wrote:
> 
> did you start from a clean *tooldir*?  that would be the bit
> that matters.  if one had an existing tooldir with nbpax in
> it, then this problem wouldn't occur.
> 
> that's my only guess to what happened...

Yah, that's probably it.

-- thorpej



Re: CVS commit: src/tools

2019-05-07 Thread Jason Thorpe


> On May 7, 2019, at 3:17 AM, matthew green  wrote:
> 
>> Diff is NOT reversed.
> 
> ah yes, i see.  please commit.

I bootstrapped the whole system 3 or 4 times without having this extra 
dependency and didn't see a build failure, so I'm a little confused, but 
whatever.

Thanks for fixing it.

-- thorpej



Re: CVS commit: src/sys/compat/freebsd

2019-04-06 Thread Jason Thorpe



> On Apr 6, 2019, at 10:45 AM, Robert Elz  wrote:
> 
> Actually, fetching & storing registers (register_t) is a common enough
> thing that it might be worth having ufetch_reg (and ustore_reg), probably
> as MD #defines that map into one of the other calls.

The only situation I know of where it's wacky is sparc64-built-as-ILP32 and 
mips64-built-as-ILP32, where register_t is 64-bit and long is 32-bit.

-- thorpej



Re: CVS commit: src/sys/arch/x86/acpi

2019-03-03 Thread Jason Thorpe



> On Mar 3, 2019, at 10:31 AM, m...@netbsd.org wrote:
> 
> Maybe we can use the longer name to avoid the confusion? PG_WIRED
> 
> (PG_W as writable exists in other archs, and there's precedence for
> PG_WIRED too)

Agreed, PG_WIRED is a better name for this.

-- thorpej



Re: CVS commit: src/sys/dev/pci

2019-01-27 Thread Jason Thorpe



> On Jan 28, 2019, at 8:11 AM, Masanobu SAITOH  wrote:
> 
> On 2019/01/28 15:07, Jason Thorpe wrote:
>> Doesn’t it seem a little dangerous to just blindly enable?
> 
> You mean it should be enable only when the secondary bridge is configured
> correctly?
> 
> Both FreeBSD and OpenBSD blindly enabled it. One of the difference between
> NetBSD and others is that they configure unconfigured bridge.

Right, if the address decoders aren't programmed properly, it seems like you 
could get into all sorts of trouble.

I really feel like the correct way to solve the problem is to fully configure 
the bus using information from ACPI.

-- thorpej



Re: CVS commit: src/sys/dev/pci

2019-01-27 Thread Jason Thorpe
Doesn’t it seem a little dangerous to just blindly enable?

-- thorpej
Sent from my iPhone.

> On Jan 28, 2019, at 6:09 AM, SAITOH Masanobu  wrote:
> 
> Module Name:src
> Committed By:msaitoh
> Date:Mon Jan 28 04:09:51 UTC 2019
> 
> Modified Files:
>src/sys/dev/pci: ppb.c
> 
> Log Message:
> Explicitly enable bus masterling in case BIOS, UEFI or firmware don't enable
> it. Might fix PR kern/53811.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.65 -r1.66 src/sys/dev/pci/ppb.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 



Re: CVS commit: src/etc

2019-01-13 Thread Jason Thorpe



> On Jan 13, 2019, at 5:08 PM, David Holland 
>  wrote:
> 
> Is there a way we could, for example, leverage the current hacks for
> chowning console devices to grant access to wpa_supplicant?

Some of this could be achieved with ttyaction(5), certainly.

-- thorpej



Re: CVS commit: src/etc

2019-01-13 Thread Jason Thorpe



> On Jan 13, 2019, at 5:21 AM, Greg Troxel  wrote:
> 
> Even if you have to be root, these changes are still hugely useful.
> "sudo wpa_cli" is not that hard, even if it seems like it should not be
> necessary.

...but made slightly more annoying seeing as how sudo is not part of the base 
OS.

-- thorpej



Re: Unaligned access in kernel on ARMv6+ (Re: CVS commit: src/sys/dev/usb)

2019-01-06 Thread Jason Thorpe



> On Jan 6, 2019, at 5:36 AM, Martin Husemann  wrote:
> 
> On Sun, Jan 06, 2019 at 08:31:53AM -0500, Greg Troxel wrote:
>> Why do we generate code with unaligned access in user space?  That seems
>> surprising, if the processor isn't happy about it.
> 
> The processor is happy with it, both in user- and kernel space.
> Only special memory regions mapped uncached make it trap.

There is a performance penalty for unaligned accesses, and not even all ARM 
versions can do it in a way that produces the expected results.  The device in 
question here is a USB device, and we support pre-ARMv6 systems that have USB 
capability.

> Nick suggest to change the mapping for bus_dma to cached, which would avoid
> the issue but may expose bugs in device drivers.

That's probably a good idea in any case, because there will almost certainly be 
a performance benefit, but I still think ensuring that drivers don't perform 
unaligned accesses is desirable.

-- thorpej



Re: CVS commit: src

2019-01-01 Thread Jason Thorpe


> On Jan 1, 2019, at 9:53 PM, m...@netbsd.org wrote:
> 
>> You're conflating two things that aren't related.  Mesa's use of LLVM is 
>> conceptually similar to the "Canadian cross", and at no point should the 
>> compiler run-times of the "build", "host", or "target" systems ever 
>> intermingle.
> 
> How come?
> 
> If an arrow means dynamic linking,
> 
> Octave -> Octave JIT -> LLVM 1
> |
> Mesa
> |
> LLVM 2
> 
> LLVM1 has someFunction(int x, int y)
> LLVM2 has someFunction(int x)

Yes, I see your point and it's clear that I misunderstood what you were saying. 
 But, the email I sent directly following addresses this situation.  My 
suggested solution would be:

Octave -> Octave JIT -> LLVM 1
|
Mesa
|
LLVM2-but-with-renamed-symbols-that-only-Mesa-knows, so you'd have:

LLVM1's someFunction(int x, int y)
...and...
LLVM2's __Mesa_Private_LLVM_someFunction(int x)

This could be done using the same mechanism by which we rename symbols in libc, 
or as a separate post-processing step similar to what librump does.

> I'm under the impression that, due to the flat namespace, whether a call
> to someFunction works correctly depends on which symbol is picked up
> first, LLVM1's or LLVM2's.
> 
> And, this applies to in-library calls in LLVM too.
> 
> This does seem to be the case. if I load a WebGL thing,
> 
>> pmap -p `pgrep firefox` |grep -i llvm
> 777D9FA0  56024K read/exec /usr/pkg/lib/libLLVM-7.so
> 777DA30B6000   2044K   /usr/pkg/lib/libLLVM-7.so
> ..
> 
> Fortunately Firefox does not use LLVM twice in different ways, so
> failures don't happen.
> (Also the Octave scenario is not possible any more)

...also, it seems like the GNU-style symbol versioning support might be another 
way to solve this, yah?

-- thorpej



Re: CVS commit: src

2019-01-01 Thread Jason Thorpe


> On Jan 1, 2019, at 9:23 PM, m...@netbsd.org wrote:
> 
> I'll note I don't know of a single package using LLVM and a library as
> well as libGL, even though it looks like it should be a likely scenario.
> 
> The closest example was Octave that uses 3D graphs (with acceleration)
> and at some opint had a LLVM-based JIT, but had given up on it because
> it was an unexpected amount of work to keep up with LLVM.

If this is something we're concerned about, then the right thing to would be to 
make a private version of the LLVM libraries with symbols renamed into a 
private namespace for Mesa to use, so that an application linking against Mesa 
*and* some other version of LLVM's internals would work.

-- thorpej



Re: CVS commit: src

2019-01-01 Thread Jason Thorpe



> On Jan 1, 2019, at 8:26 PM, m...@netbsd.org wrote:
> 
> Having two versions of LLVM in use by a binary is going to fail in
> surprising ways.
> 
> Base can be several years old at some point, increasing the odds of the
> LLVM non-compatibility biting us.
> And libGL is very widely used.

You're conflating two things that aren't related.  Mesa's use of LLVM is 
conceptually similar to the "Canadian cross", and at no point should the 
compiler run-times of the "build", "host", or "target" systems ever intermingle.

-- thorpej



Re: CVS commit: src

2019-01-01 Thread Jason Thorpe



> On Jan 1, 2019, at 11:23 AM, Martin Husemann  wrote:
> 
> Yeah (and to waste more than an additional hour in a full build of
> NetBSD) - but then we need a working plan how to provide libllvm as
> part of the installed system, not only headers for building libmesa.

I'm not arguing the merits of it one way or the other, just trying to explain 
why it's needed in this particular instance.  I agree that the issues involved 
here are thorny (especially if LLVM isn't providing API / ABI stability from 
release to release), and there are a lot of trade-offs to be weighed.

But I really just wanted to point out that this isn't really a toolchain issue. 
 IMO, the best way to handle this situation is to decouple "LLVM used to 
provide JIT / VM services to system libraries or other subsystems" from "LLVM 
used as compiler toolchain component".  The former, which is what Maya needs in 
this case, is something that can be updated on a different frequency from the 
latter (at the cost of source tree bloat, but whatever, not that big of a 
deal), and can be narrowed in scope (don't necessarily need the language 
front-ends or CPU-target back-ends that the normal compiler toolchain requires).

-- thorpej



Re: CVS commit: src

2019-01-01 Thread Jason Thorpe



> On Jan 1, 2019, at 8:02 AM, Robert Swindells  wrote:
> 
> How does the usage of libLLVM from MesaLib make use of the LLVM headers
> at *runtime* ?
> 
> If they are not needed at runtime then why do they need to be installed
> on a target machine ?

What's needed on the target machine are the LLVM libraries, and the headers are 
needed to use the APIs provided by those libraries.  This isn't any different 
than the target system's libc needing the headers installed that define the 
interfaces to libc so that some application that also runs on the target can 
use functions from libc.  The headers are needed so that MesaLib can get the 
interface descriptions for functions in the LLVM libraries, which it links 
against.

LLVM is more than just a build-time host tool... more and more it is being used 
to provide system run-time services on the target system, and I'm not talking 
about what is traditionally referred to as the "compiler runtime" (e.g. the 
libgcc analogue) ... I'm referring to the compiler (and the virtual machine it 
implements) itself.

-- thorpej



Re: CVS commit: src

2019-01-01 Thread Jason Thorpe



> On Jan 1, 2019, at 6:34 AM, Martin Husemann  wrote:
> 
> Can you explain how the MesaLib build uses llvm?
> 
> This kinda sounds like you need a special build time binary that would
> live in $TOOLDIR and it is not exactly clear how installing some headers
> helps building this tool (as you can not assume a NetBSD build host).

LLVM is used in the graphics pipeline at run-time to generate native code that 
runs on the GPU.

-- thorpej



Re: CVS commit: src/sys/arch/evbmips/conf

2018-12-30 Thread Jason Thorpe



> On Dec 30, 2018, at 4:34 PM, Sevan Janiyan  wrote:
> 
> On 30/12/2018 16:16, Jason Thorpe wrote:
>> Maybe create a std.evmips that the various std. files can
>> include to get options that you want for everything?
> 
> Noted, I was thinking about the pull up to the 8 branch and making that
> as easy as possible. I don't have anything else further at the moment,
> should I go ahead any way?

I would say pull this up to -8, and then make another cleanup pass.

> 
> 
> Sevan

-- thorpej



Re: CVS commit: src/sys/arch/evbmips/conf

2018-12-30 Thread Jason Thorpe



> On Dec 30, 2018, at 6:51 AM, Sevan Janiyan  wrote:
> 
> Modified Files:
>   src/sys/arch/evbmips/conf: ADM5120 ADM5120-NB ADM5120-USB ALCHEMY AP30
>   CI20 CPMBR1400 DB120 ERLITE GDIUM LINKITSMART7688 LOONGSON MALTA
>   MERAKI RB153 RB433UAH SBMIPS WGT624V3 XLSATX ZYXELKX

Maybe create a std.evmips that the various std. files can include to get 
options that you want for everything?

-- thorpej



Re: Weak x86 aliases

2018-12-30 Thread Jason Thorpe


> On Dec 28, 2018, at 10:14 AM, Mathew, Cherry G.  wrote:
> 
> ic, this probably explains why it's used sparingly so far. Thanks for the 
> explanation. I'll try to educate myself on this wrt enabling this for modload.

We use weak symbols all over the place in user space.  It's just that the 
kernel-resident linker doesn't support them (yet?), so we can't use them for 
symbols that are part of the ABI exposed to modules.

> The other option for me is a bunch of ugly #ifdefs , which I'm trying to 
> minimise.

Reducing #ifdefs is a laudable goal.

-- thorpej



Re: CVS commit: src

2018-12-30 Thread Jason Thorpe


> On Dec 26, 2018, at 4:11 PM, Taylor R Campbell 
>  wrote:
> 
> Job reference counting is currently slightly busted -- my draft for
> threadpool_schedule_job didn't increment it correctly, and your
> threadpool_schedule_job doesn't handle failure in threadpool_job_hold.


Ok, I looked at this some last night, and was up early this morning and took a 
peek at it again, and I think getting rid of the refcnt is going to be tough.

The rub is the special handling of "assigned to overseer" in cancel_async.  
Because of that, you can't really use job_thread as a proxy for "held", and 
there's a small window in the overseer thread where neither the pool nor the 
job are locked because of locking order (which is job -> pool), yet the job is 
referenced; this happens when there is a thread to give the job to, we pull the 
job off the overseer queue, but we can't lock the job until we drop the tp_lock 
-- it's during that window where we have phantom reference that needs to be 
accounted for.

So, here's how I think it can be fixed:

-- Must continue to use atomics for job_hold / job_rele.

-- job_hold remains lockless, but job_rele can only be called **without** the 
job_lock held, because it needs to acquire the lock in the unlikely case it 
performs a cv_broadcast (see below).  Also need a job_rele_locked.

-- In schedule_job, always job_hold for either case (overseer or direct 
hand-off to idle thread).  This is the job's lifecycle reference, and will be 
dropped either in cancel_async or in job_done.

-- In cancel_async, only job_rele_locked after successfully removing it from 
the overseer queue (protected by tp_lock).  Note the job_lock is held by the 
caller of cancel_async.

-- In overseer thread, after grabbing a local reference to the job, do a 
job_hold before dropping the tp_lock, to prevent racing with cancel_async.  
This is a temporary hold while we do our work, and will always be released, 
regardless of job being cancelled beneath us or given to a thread.  Note the 
most likely scenario is that the job was NOT cancelled beneath us, and thus we 
will not be dropping the last reference, and therefore we don't want to take 
the job_lock again in this common case just to drop our temporary reference.

-- job_done should assert there is always at least 1 reference, and should 
directly decrement it rather than doing job_rele, because it already has the 
job_lock held and always does the cv_broadcast anyway.

-- Don't need to handle any overflow in job_hold / job_rele ... there reference 
count should ever one be 0 (totally idle), 1 (scheduled/running OR phantom 
reference), or 2 (scheduled AND phantom reference).  We can assert no overflow 
in job_hold.

-- thorpej



Re: Weak x86 aliases

2018-12-28 Thread Jason Thorpe



> On Dec 28, 2018, at 6:44 PM, John Nemeth  wrote:
> 
> However, having said that, I suspect that your work with PVHVM
> (and presumably PVH after that) will make the idea of having seperate
> modules for Xen obsolete.  I.e. it appears to me that we're headed
> to a world where a single kernel will work both on raw iron and
> under Xen.

Indeed, Xen x86(-64) modules should be completely compatible with normal 
x86(-64) modules.  If they're not, it means we're leaking too much internal goo 
into the ABI.

-- thorpej



Re: Weak x86 aliases

2018-12-28 Thread Jason Thorpe



> On Dec 28, 2018, at 1:32 PM, Paul Goyette  wrote:
> 
> The in-kernel linker doesn't deal with weak symbols at all.  It would
> need a lot of thought to get it right.  For example, if module A
> (containing a weak reference) gets loaded, its weak references don't
> resolve.  Then module B gets loaded and defines the symbol(s).  Do we
> "go back" and re-run the linker for module A?  Or do we allow the
> results to be different depending on module load order?  (If module B
> were loaded first, and then module A, the weak reference would get
> resolved.)

This is why we should't try to support them in the kernel :-)

-- thorpej



Re: Weak x86 aliases

2018-12-28 Thread Jason Thorpe


> On Dec 28, 2018, at 6:06 AM, Cherry G.Mathew  wrote:
> 
> I don't like the imperative in your tone. NVMM is the user of modloader,
> not PVHVM. So if you feel like your usecase needs fixing, I'd say it's
> your problem - or don't use modules, but see below.

...but as the person who committed the change that broke it, you're 
responsible.  His use case didn't need fixing until you introduced weak symbols 
that our kernel loader can't currently resolve.

-- thorpej



Re: CVS commit: src

2018-12-27 Thread Jason Thorpe



> On Dec 27, 2018, at 6:04 AM, Jason Thorpe  wrote:
> 
> -- job_hold remains lockless, but job_rele can only be called **without** the 
> job_lock held, because it needs to acquire the lock in the unlikely case it 
> performs a cv_broadcast (see below).  Also need a job_rele_locked.

Correction -- all cases of job_rele can be called with the job_lock held.  (I 
mis-read the block of the code in the overseer where the job_lock is dropped 
immediately before calling job_rele.)

Index: kern_threadpool.c
===
RCS file: /cvsroot/src/sys/kern/kern_threadpool.c,v
retrieving revision 1.12
diff -u -p -r1.12 kern_threadpool.c
--- kern_threadpool.c   27 Dec 2018 04:45:29 -  1.12
+++ kern_threadpool.c   27 Dec 2018 14:37:46 -
@@ -134,7 +134,7 @@ static void threadpool_percpu_destroy(st
 
 static threadpool_job_fn_t threadpool_job_dead;
 
-static int threadpool_job_hold(struct threadpool_job *);
+static voidthreadpool_job_hold(struct threadpool_job *);
 static voidthreadpool_job_rele(struct threadpool_job *);
 
 static voidthreadpool_overseer_thread(void *) __dead;
@@ -650,19 +650,16 @@ threadpool_job_destroy(struct threadpool
(void)strlcpy(job->job_name, "deadjob", sizeof(job->job_name));
 }
 
-static int
+static void
 threadpool_job_hold(struct threadpool_job *job)
 {
unsigned int refcnt;
 
do {
refcnt = job->job_refcnt;
-   if (refcnt == UINT_MAX)
-   return EBUSY;
+   KASSERT(refcnt != UINT_MAX);
} while (atomic_cas_uint(>job_refcnt, refcnt, (refcnt + 1))
!= refcnt);
-
-   return 0;
 }
 
 static void
@@ -670,16 +667,16 @@ threadpool_job_rele(struct threadpool_jo
 {
unsigned int refcnt;
 
+   KASSERT(mutex_owned(job->job_lock));
+
do {
refcnt = job->job_refcnt;
KASSERT(0 < refcnt);
if (refcnt == 1) {
-   mutex_enter(job->job_lock);
refcnt = atomic_dec_uint_nv(>job_refcnt);
KASSERT(refcnt != UINT_MAX);
if (refcnt == 0)
cv_broadcast(>job_cv);
-   mutex_exit(job->job_lock);
return;
}
} while (atomic_cas_uint(>job_refcnt, refcnt, (refcnt - 1))
@@ -703,6 +700,16 @@ threadpool_job_done(struct threadpool_jo
curlwp->l_name = job->job_thread->tpt_lwp_savedname;
lwp_unlock(curlwp);
 
+   /*
+* Inline the work of threadpool_job_rele(); the job is already
+* locked, the most likely scenario (XXXJRT only scenario?) is
+* that we're dropping the last reference (the one taken in
+* threadpool_schedule_job()), and we always do the cv_broadcast()
+* anyway.
+*/
+   KASSERT(0 < job->job_refcnt);
+   unsigned int refcnt __diagused = atomic_dec_uint_nv(>job_refcnt);
+   KASSERT(refcnt != UINT_MAX);
cv_broadcast(>job_cv);
job->job_thread = NULL;
 }
@@ -725,6 +732,8 @@ threadpool_schedule_job(struct threadpoo
return;
}
 
+   threadpool_job_hold(job);
+
/* Otherwise, try to assign a thread to the job.  */
mutex_spin_enter(>tp_lock);
if (__predict_false(TAILQ_EMPTY(>tp_idle_threads))) {
@@ -740,7 +749,6 @@ threadpool_schedule_job(struct threadpoo
__func__, job->job_name, job->job_thread));
TAILQ_REMOVE(>tp_idle_threads, job->job_thread,
tpt_entry);
-   threadpool_job_hold(job);
job->job_thread->tpt_job = job;
}
 
@@ -786,6 +794,7 @@ threadpool_cancel_job_async(struct threa
mutex_spin_enter(>tp_lock);
TAILQ_REMOVE(>tp_jobs, job, job_entry);
mutex_spin_exit(>tp_lock);
+   threadpool_job_rele(job);
return true;
} else {
/* Too late -- already running.  */
@@ -889,15 +898,13 @@ threadpool_overseer_thread(void *arg)
}
 
/* There are idle threads, so try giving one a job.  */
-   bool rele_job = true;
struct threadpool_job *const job = TAILQ_FIRST(>tp_jobs);
TAILQ_REMOVE(>tp_jobs, job, job_entry);
-   error = threadpool_job_hold(job);
-   if (error) {
-   TAILQ_INSERT_HEAD(>tp_jobs, job, job_entry);
-   (void)kpause("pooljob", false, hz, >tp_lock);
-   continue;
-   }
+   /*
+* Take an extra reference on the job temporarily so that
+* it won't disappear on us while we have both locks dropped.
+   

Re: CVS commit: src

2018-12-26 Thread Jason Thorpe


> On Dec 26, 2018, at 8:32 PM, Taylor R Campbell 
>  wrote:
> 
> LGTM if it works!

Doesn't trip any asserts in the unit tests... I'm going to push this in and 
then do the ref counting change as a separate commit.

-- thorpej



Re: CVS commit: src

2018-12-26 Thread Jason Thorpe



> On Dec 26, 2018, at 7:52 PM, Taylor R Campbell 
>  wrote:
> 
> Probably wanna dereference job->job_thread _before_ setting it to
> NULL!

Picky, picky :-)

Index: kern_threadpool.c
===
RCS file: /cvsroot/src/sys/kern/kern_threadpool.c,v
retrieving revision 1.11
diff -u -p -r1.11 kern_threadpool.c
--- kern_threadpool.c   26 Dec 2018 22:16:26 -  1.11
+++ kern_threadpool.c   27 Dec 2018 04:07:45 -
@@ -107,6 +107,7 @@ TAILQ_HEAD(thread_head, threadpool_threa
 
 struct threadpool_thread {
struct lwp  *tpt_lwp;
+   char*tpt_lwp_savedname;
struct threadpool   *tpt_pool;
struct threadpool_job   *tpt_job;
kcondvar_t  tpt_cv;
@@ -693,6 +694,15 @@ threadpool_job_done(struct threadpool_jo
KASSERT(job->job_thread != NULL);
KASSERT(job->job_thread->tpt_lwp == curlwp);
 
+   /*
+* We can safely read this field; it's only modified right before
+* we call the job work function, and we are only preserving it
+* use here; no one cares if it conains junk afterward.
+*/
+   lwp_lock(curlwp);
+   curlwp->l_name = job->job_thread->tpt_lwp_savedname;
+   lwp_unlock(curlwp);
+
cv_broadcast(>job_cv);
job->job_thread = NULL;
 }
@@ -977,24 +987,25 @@ threadpool_thread(void *arg)
 
struct threadpool_job *const job = thread->tpt_job;
KASSERT(job != NULL);
-   mutex_spin_exit(>tp_lock);
-
-   TP_LOG(("%s: running job '%s' on thread %p.\n",
-   __func__, job->job_name, thread));
 
/* Set our lwp name to reflect what job we're doing.  */
lwp_lock(curlwp);
-   char *const lwp_name = curlwp->l_name;
+   char *const lwp_name __diagused = curlwp->l_name;
+   thread->tpt_lwp_savedname = curlwp->l_name;
curlwp->l_name = job->job_name;
lwp_unlock(curlwp);
 
+   mutex_spin_exit(>tp_lock);
+
+   TP_LOG(("%s: running job '%s' on thread %p.\n",
+   __func__, job->job_name, thread));
+
/* Run the job.  */
(*job->job_fn)(job);
 
-   /* Restore our lwp name.  */
-   lwp_lock(curlwp);
-   curlwp->l_name = lwp_name;
-   lwp_unlock(curlwp);
+   /* lwp name restored in threadpool_job_done(). */
+   KASSERTMSG((curlwp->l_name == lwp_name),
+   "someone forgot to call threadpool_job_done()!");
 
/* Job is done and its name is unreferenced.  Release it.  */
threadpool_job_rele(job);


-- thorpej



Re: CVS commit: src

2018-12-26 Thread Jason Thorpe



> On Dec 26, 2018, at 4:11 PM, Taylor R Campbell 
>  wrote:
> 
> Caveat: Need to
>   take care of the lwp name in threadpool_job_done so that we never
>   point at the job_name of a job in oblivion.

Something like this.

Index: kern_threadpool.c
===
RCS file: /cvsroot/src/sys/kern/kern_threadpool.c,v
retrieving revision 1.11
diff -u -p -r1.11 kern_threadpool.c
--- kern_threadpool.c   26 Dec 2018 22:16:26 -  1.11
+++ kern_threadpool.c   27 Dec 2018 03:46:07 -
@@ -107,6 +107,7 @@ TAILQ_HEAD(thread_head, threadpool_threa
 
 struct threadpool_thread {
struct lwp  *tpt_lwp;
+   char*tpt_lwp_savedname;
struct threadpool   *tpt_pool;
struct threadpool_job   *tpt_job;
kcondvar_t  tpt_cv;
@@ -695,6 +696,15 @@ threadpool_job_done(struct threadpool_jo
 
cv_broadcast(>job_cv);
job->job_thread = NULL;
+
+   /*
+* We can safely read this field; it's only modified right before
+* we call the job work function, and we are only preserving it
+* use here; no one cares if it conains junk afterward.
+*/
+   lwp_lock(curlwp);
+   curlwp->l_name = job->job_thread->tpt_lwp_savedname;
+   lwp_unlock(curlwp);
 }
 
 void
@@ -977,24 +987,22 @@ threadpool_thread(void *arg)
 
struct threadpool_job *const job = thread->tpt_job;
KASSERT(job != NULL);
-   mutex_spin_exit(>tp_lock);
-
-   TP_LOG(("%s: running job '%s' on thread %p.\n",
-   __func__, job->job_name, thread));
 
/* Set our lwp name to reflect what job we're doing.  */
lwp_lock(curlwp);
-   char *const lwp_name = curlwp->l_name;
+   thread->tpt_lwp_savedname = curlwp->l_name;
curlwp->l_name = job->job_name;
lwp_unlock(curlwp);
 
+   mutex_spin_exit(>tp_lock);
+
+   TP_LOG(("%s: running job '%s' on thread %p.\n",
+   __func__, job->job_name, thread));
+
/* Run the job.  */
(*job->job_fn)(job);
 
-   /* Restore our lwp name.  */
-   lwp_lock(curlwp);
-   curlwp->l_name = lwp_name;
-   lwp_unlock(curlwp);
+   /* lwp name restored in threadpool_job_done(). */
 
/* Job is done and its name is unreferenced.  Release it.  */
threadpool_job_rele(job);


-- thorpej



Re: CVS commit: src

2018-12-26 Thread Jason Thorpe



> On Dec 26, 2018, at 5:25 PM, Jason Thorpe  wrote:
> 
> I think this the way to go.  I’ll look at it a little later tonight.
> 
> -- thorpej
> Sent from my iPhone.
> 
>> On Dec 26, 2018, at 4:11 PM, Taylor R Campbell 
>>  wrote:
>> 
>> 2. Chuck the job reference count altogether.  Just use job_thread as a
>>  proxy for whether it's in use.  User must not call
>>  threadpool_schedule_job concurrently with threadpool_job_destroy;
>>  if a job may have been scheduled, user must cancel it before
>>  destroying it.

To elaborate -- this is the same requirement in the task(9) API -- In fact, my 
unit test tripped over this requirement by virtue of using implicit-task-done, 
and so the safe thing to do is always cancel before destroying unless you're 
doing extra bookkeeping in the caller where you're using other means to ensure 
jobs / tasks are complete.

-- thorpej



Re: CVS commit: src

2018-12-26 Thread Jason Thorpe
I think this the way to go.  I’ll look at it a little later tonight.

-- thorpej
Sent from my iPhone.

> On Dec 26, 2018, at 4:11 PM, Taylor R Campbell 
>  wrote:
> 
> 2. Chuck the job reference count altogether.  Just use job_thread as a
>   proxy for whether it's in use.  User must not call
>   threadpool_schedule_job concurrently with threadpool_job_destroy;
>   if a job may have been scheduled, user must cancel it before
>   destroying it.



Re: CVS commit: src

2018-12-26 Thread Jason Thorpe



> On Dec 26, 2018, at 10:04 AM, Taylor R Campbell 
>  wrote:
> 
> Can you use a module initialization routine for this, or call it in
> main if you don't want to deal with modules, instead of sprinkling
> RUN_ONCE in various places?

I thought about using a module initializer, the timing of when it's run might 
be too late or too early (or maybe at a perfect time, if you're lucky).  I 
generally dislike static initialization functions if lazy initialization can be 
used reasonably, and I thought this was such an application for it, but it's 
not a hill I'm willing to die on, so static initialization it is.

> Can we just have struct threadpool_job in the header file like I did
> originally, without the strict aliasing violations, ABI conditionals,
> ?  What does this buy us?  We don't have a culture of abusing
> abstractions simply because they happen to be written in header files,
> so we don't need to put technical measures in the way of such abuse.

I reverted it.  I prefer truly opaque objects being presented to the clients of 
an API, but again, not this hill.

>> +/* Idle out threads after 30 seconds */
>> +#define THREADPOOL_IDLE_TICKS   mstohz(30 * 1000)
> 
> Maybe this should be a sysctl knob?

Yah, I'll add that a little later.

>> +struct threadpool_unbound {
>> +/* must be first; see threadpool_create() */
>> +struct threadpool   tpu_pool;
> 
> Why must this be first?  Unless this is really crucial for some
> performance-critical reason, let's try to avoid putting constraints
> like this into new code.

I've rearranged it so callers of threadpool_create() and threadpool_destroy() 
manage the storage themselves.

>> +#ifdef THREADPOOL_VERBOSE
>> +#define TP_LOG(x)   printf x
>> +#else
>> +#define TP_LOG(x)   /* nothing */
>> +#endif /* THREADPOOL_VERBOSE */
> 
> Maybe make these dtrace probes?

That is my plan.

> Atomics here don't hurt but are not necessary because this is always
> done when the caller has exclusive access to the pool.  (This was a
> mistake in the original, sorry!)  This should just do
> 
>   KASSERT(mutex_owned(>tp_lock));
>   pool->tp_refcnt++;
> 
> which will always succeed with a 64-bit reference count.

Done.

> The atomics here don't hurt, but the mutex operations do, because this
> is called with the threadpool's lock held.  (Again, mistake in the
> original, sorry!)  This should just do
> 
>   KASSERT(mutex_owned(>tp_lock));
>   KASSERT(0 < pool->tp_refcnt);
>   if (--pool->tp_refcnt == 0)
>   cv_broadcast(>tp_overseer.tpt_cv);

Done.  The recursive acquisition would have been really hard to trigger, which 
is why my unit test missed it.

> For this to work, threadpool_overseer_thread and threadpool_thread must
> call threadpool_rele while holding the lock.
> 
> There's no issue with doing that: they don't use the pool afterward,
> and destruction -- including mutex_destroy(>tp_lock) -- can't
> proceed until the thread that called threadpool_rele releases the lock
> too.
> 
> Alternative, requiring more rototilling: The overseer could just join
> the worker threads when dying, and threadpool_destroy could just join
> the overseer -- then we could skip the reference counting altogether.
> Not sure whether this is a net win in bookkeeping, though.

The additional bookkeeping cost doesn't seem work it ... having to actively 
track active threads in addition to the idle ones, and then having to join them 
one at a time.  The reference count game is a lot easier.

> Not a KNF rule, but I'd prefer it if either every branch is braced or
> no branch is braced, rather than a mixture of braced and unbraced
> branches in a single `if'.

Done.  (Well, at at least the lines I managed to find at a cursory glance.)

> 
>> +static int
>> +threadpool_job_hold(threadpool_job_impl_t *job)
>> +{
>> +unsigned int refcnt;
>> +do {
> 
> KNF, blank line between declarations and body.

Fixed.  (Deleting that line was a honest-to-goodness mistake.)

> 
>> +refcnt = job->job_refcnt;
>> +if (refcnt == UINT_MAX)
>> +return EBUSY;
>> +} while (atomic_cas_uint(>job_refcnt, refcnt, (refcnt + 1))
>> +!= refcnt);
>> +
> 
> Trailing whitespace.

F***ing vim.  I should install nvi on all of my Macs.

> (Consider (setq show-trailing-whitespace t) if you use Emacs!)

No thanks :-)

>> +bool
>> +threadpool_cancel_job_async(threadpool_t *pool, threadpool_job_t *ext_job)
>> +{
>> +threadpool_job_impl_t *job = THREADPOOL_JOB_TO_IMPL(ext_job);
>> +
>> +KASSERT(mutex_owned(job->job_lock));
>> +
>> +/*
>> + * XXXJRT This fails (albeit safely) when all of the following
>> + * are true:
>> + *
>> + *  => "pool" is something other than what the job was
>> + * scheduled on.  This can legitimately occur if,
>> + * for example, a job is percpu-scheduled on CPU0
>> + * and then CPU1 attempts 

Re: CVS commit: src/sys/kern

2018-12-24 Thread Jason Thorpe


> On Dec 24, 2018, at 9:51 PM, m...@netbsd.org wrote:
> 
> Heavily contended whitespace

Cage match material.

-- thorpej



Re: CVS commit: src/sys/dev/usb

2018-12-15 Thread Jason Thorpe



> On Dec 15, 2018, at 2:30 AM, SAITOH Masanobu  wrote:
> 
> Module Name:  src
> Committed By: msaitoh
> Date: Sat Dec 15 10:30:58 UTC 2018
> 
> Modified Files:
>   src/sys/dev/usb: if_urtwn.c
> 
> Log Message:
> Make IODATA WN-G150UMW work:
> - Increase delay to prevent "could not send firmware command". The value
>  is taken from FreeBSD.
> -Increase delay to prevent "timeout waiting for firmware readiness". The
>  vaule is taken from Linux.

This is worth a pull-up to netbsd-8, I think.  I get that firmware readiness 
error a lot on one of the urtwn devices I have on a RPI (I don't have I handy, 
so I can't tell you the exact model right now).

-- thorpej



Re: CVS commit: src/sys/dev/i2c

2018-12-14 Thread Jason Thorpe



> On Dec 14, 2018, at 2:05 PM, Michael Lorenz  wrote:
> 
> Module Name:  src
> Committed By: macallan
> Date: Fri Dec 14 22:05:36 UTC 2018
> 
> Modified Files:
>   src/sys/dev/i2c: ds1307.c files.i2c
> 
> Log Message:
> add options DSRTC_YEAR_START_2K for machines which use 2000 and not 1970
> as base to count years from, like Iyonix.
> While there apply the offset when writing to the clock as well.

This should not be done with a compile-time option.  A better way to handle 
this is to define a device property ("rtc-base-year" maybe?) as an and to use 
that if the property exists.  You can then set it in the iyonix 
device_register() function when you get a "dsrtc".

-- thorpej



  1   2   >