Re: CVS commit: src

2018-12-30 Thread Christos Zoulas
On Dec 31,  4:23am, u...@stderr.spb.ru (Valery Ushakov) wrote:
-- Subject: Re: CVS commit: src

| On Sun, Dec 30, 2018 at 18:19:41 -0500, Christos Zoulas wrote:
| 
| > Modified Files:
| > src: build.sh
| > 
| > Log Message:
| > add build libs (undocumented).
| 
| Just curious, what is it for?  Also build.sh -V NOBINARIES= build
| already does more or less the same, isn't it?

I did not know about that. I am using it to just build enough libraries
so I can cross build native tools. It already builds too much (too many
libraries that are not needed). I did not know about NOBINARIES=

christos


Re: CVS commit: src

2018-12-30 Thread Valery Ushakov
On Sun, Dec 30, 2018 at 18:19:41 -0500, Christos Zoulas wrote:

> Modified Files:
>   src: build.sh
> 
> Log Message:
> add build libs (undocumented).

Just curious, what is it for?  Also build.sh -V NOBINARIES= build
already does more or less the same, isn't it?

-uwe


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 Sevan Janiyan
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?


Sevan


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: CVS commit: src/sys/arch

2018-12-30 Thread Cherry G . Mathew
Christoph Badura  writes:

> On Tue, Dec 25, 2018 at 11:45:42AM +0530, Cherry G.Mathew wrote:
>> Joerg Sonnenberger  writes:
>> > On Sat, Dec 22, 2018 at 09:27:22PM +, Cherry G. Mathew wrote:
>> >> Modified Files:
>> >>   src/sys/arch/amd64/amd64: cpufunc.S
>> >>   src/sys/arch/i386/i386: cpufunc.S i386func.S
>> >>   src/sys/arch/xen/x86: xenfunc.c
>> >> Log Message:
>> >> Introduce a weak alias method of exporting different implementations
>> >> of the same API.
>> > Why? As in: what's the advantage of making them weak.
>> I'd posted separately before this commit explaining the rationale.
>
> It took me a while to check the most obvious places and figure out which
> message it might have been.  A more precise reference would have helped.
>
>> Here's the scenario above:
>> 
>> let's take lcr3(). On native this is a ring3 operation, implemented in
>> assembler. On xen, this is a PV call. Currently there's no need to have
>> both implementations in a single binary, since PV and native binaries
>> are distinct. With PVHVM, we have a situation where at boot time, the
>> native version is required, and after XEN is detected, we can use the
>> XEN version of the call. I've taken the approach of naming the
>> individual functions distinctly, eg: i386_lcr3(), or xen_lcr3(), while
>> and exporting them weakly as the consumed version, ie; lcr3().
>> 
>> What happens is that when the individual binary is compiled, the
>> appropriate weakly exported symbol takes over, and things work as
>> expected. When  the combined binary for PVHVM is required, we write a
>> strongly exported "switch" function, called lcr3() (I haven't committed
>> this yet) which overrides both the weak symbols. It can then do the
>> runtime calls by calling into the appropriate i386_lcr3() or xen_lcr3(),
>> depending on the mode in which it is running.
>
> I don't find this argument for weak aliases convincing.  You plan to
> write the function that makes a runtime decision between the
> implemenation anyway.  You might as well write that function now and avoid
> another bit of hidden magic.
>

The other options have been suggested earlier (function pointers and
hotpatching).

Function pointers require reprototyping every function required (I don't
have an exhaustive list yet). Hotpatching isn't useful in that it will
overwrite the default version, so we'll have to hotpatch twice. Once to
override the default, and then to make a copy so that the default is
available. It's also ugly, so for me this is the least preferable
method.

> You can have multiple versions of that "switch" function and select the
> appropriate one with config(8) logic.  

It's too late for that. Things like lcr3() need to work way before
config(8) is a twinkle in boot(8)s eyes.

> Or you can select with #ifdef.  Somewhere you have to place the logic
> for conditional compilation/linking.  Having that logic concentrated
> in a single place instead of N seems preferable to me.

Yes, it's pretty intense - see x86/mainbus.c:mainbus_attach() for a
sample of what is to come.

-- 
~cherry