Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Fri, Jul 1, 2016 at 9:51 AM, Masao Uebayashi wrote: > On Tue, Jun 28, 2016 at 10:08 AM, Ryota Ozaki wrote: >> On Sat, Jun 25, 2016 at 11:56 AM, matthew green wrote: > Since we already use preempt_disable() to force an lwp to stick to a cpu, > doesn't that solve the problem? If need be, we can enforce > nonpreemptable > lwp's don't migrate. >>> >>> why would we want to disable preemption in code that merely wants >>> to run on a particular cpu. >>> >>> i dno't understand why using the side effect of preempt_disable() >>> is better than explicitly stating what is wanted. >> >> Yes. That's why the API is introduced. > > And by introducing such a primitive function, you're doubling the > number of combinations of primitives... > > I hope (some of) you surely understand all the implications of those > combinations ... but I'm not. I'm even not sure if kpreempt_disable() > really prevents LWPs from migrating between CPUs. It'd be really > helpful if restrictions are expressed by *strict* assertions. > > Could you at least put KASSERT((l->l_pflags & LP_BOUND) == 0) in > curlwp_bind()? The assertion is invalid because curlwp_bind can be used regardless of if LP_BOUND is set or not; it just makes sure LP_BOUND is set and restore the original state in curlwp_bindx, just like spl*/splx do. ozaki-r
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Tue, Jun 28, 2016 at 10:08 AM, Ryota Ozaki wrote: > On Sat, Jun 25, 2016 at 11:56 AM, matthew green wrote: >>> > Since we already use preempt_disable() to force an lwp to stick to a cpu, >>> > doesn't that solve the problem? If need be, we can enforce nonpreemptable >>> > lwp's don't migrate. >> >> why would we want to disable preemption in code that merely wants >> to run on a particular cpu. >> >> i dno't understand why using the side effect of preempt_disable() >> is better than explicitly stating what is wanted. > > Yes. That's why the API is introduced. And by introducing such a primitive function, you're doubling the number of combinations of primitives... I hope (some of) you surely understand all the implications of those combinations ... but I'm not. I'm even not sure if kpreempt_disable() really prevents LWPs from migrating between CPUs. It'd be really helpful if restrictions are expressed by *strict* assertions. Could you at least put KASSERT((l->l_pflags & LP_BOUND) == 0) in curlwp_bind()?
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Sat, Jun 25, 2016 at 11:56 AM, matthew green wrote: >> > Since we already use preempt_disable() to force an lwp to stick to a cpu, >> > doesn't that solve the problem? If need be, we can enforce nonpreemptable >> > lwp's don't migrate. > > why would we want to disable preemption in code that merely wants > to run on a particular cpu. > > i dno't understand why using the side effect of preempt_disable() > is better than explicitly stating what is wanted. Yes. That's why the API is introduced. ozaki-r
re: Introduce curlwp_bind and curlwp_unbind for psref(9)
> > Since we already use preempt_disable() to force an lwp to stick to a cpu, > > doesn't that solve the problem? If need be, we can enforce nonpreemptable > > lwp's don't migrate. why would we want to disable preemption in code that merely wants to run on a particular cpu. i dno't understand why using the side effect of preempt_disable() is better than explicitly stating what is wanted. .mrg.
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Fri, Jun 17, 2016 at 12:57 PM, Matt Thomas wrote: > >> On Jun 13, 2016, at 5:53 PM, Ryota Ozaki wrote: >> >> On Mon, Jun 13, 2016 at 11:21 PM, Taylor R Campbell >> wrote: >>> Date: Mon, 13 Jun 2016 14:00:16 +0200 >>> From: Joerg Sonnenberger >>> >>> On Mon, Jun 13, 2016 at 07:36:31PM +0900, Ryota Ozaki wrote: Currently we do it by open-coding in each place, but we should provide some API to simplify codes. riastradh@ suggested curlwp_bind and curlwp_unbind some time ago (*1) and this patch (*2) just follows the idea. >>> >>> The primary question for me is whether nesting should be allowed or not. >>> That would mean a reference count behind the flag. >>> >>> This `reference count' gets stored on the stack. The caller does: >>> >>>int bound = curlwp_bind(); >>> >>>... psref_wotsit ... >>> >>>curlwp_unbind(bound); >>> >>> If it was already bound, bound = 1 and curlwp_unbind does nothing; if >>> it was not already bound, bound = 0 and curlwp_unbind unbinds it. >>> >>> Perhaps the name should be `curlwp_bound_restore' or something else to >>> emphasize this, but I haven't come up with one that I like better on >>> aesthetic grounds. >> >> - curlwp_bind and curlwp_unbind >> - curlwp_bound_set and curlwp_bound_restore >> - curlwp_bound and curlwp_boundx >> >> Any other ideas? :) > > Since we already use preempt_disable() to force an lwp to stick to a cpu, > doesn't that solve the problem? If need be, we can enforce nonpreemptable > lwp's don't migrate. +1 LP_BOUND (and LP_INTR) looks like a property, rather than a state, which is set only once when a kthread is created. (Except the one in panic(9); but usually "let's not do this after a panic" is done by sprinkling if (panicstr != NULL) ...) I'm thinking of if there are cases where preempt-disabled threads have to be migratable, but can't think of any (yet).
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
> On Jun 13, 2016, at 5:53 PM, Ryota Ozaki wrote: > > On Mon, Jun 13, 2016 at 11:21 PM, Taylor R Campbell > wrote: >> Date: Mon, 13 Jun 2016 14:00:16 +0200 >> From: Joerg Sonnenberger >> >> On Mon, Jun 13, 2016 at 07:36:31PM +0900, Ryota Ozaki wrote: >>> Currently we do it by open-coding in each place, >>> but we should provide some API to simplify codes. >>> riastradh@ suggested curlwp_bind and curlwp_unbind >>> some time ago (*1) and this patch (*2) just follows >>> the idea. >> >> The primary question for me is whether nesting should be allowed or not. >> That would mean a reference count behind the flag. >> >> This `reference count' gets stored on the stack. The caller does: >> >>int bound = curlwp_bind(); >> >>... psref_wotsit ... >> >>curlwp_unbind(bound); >> >> If it was already bound, bound = 1 and curlwp_unbind does nothing; if >> it was not already bound, bound = 0 and curlwp_unbind unbinds it. >> >> Perhaps the name should be `curlwp_bound_restore' or something else to >> emphasize this, but I haven't come up with one that I like better on >> aesthetic grounds. > > - curlwp_bind and curlwp_unbind > - curlwp_bound_set and curlwp_bound_restore > - curlwp_bound and curlwp_boundx > > Any other ideas? :) Since we already use preempt_disable() to force an lwp to stick to a cpu, doesn't that solve the problem? If need be, we can enforce nonpreemptable lwp's don't migrate.
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Wed, Jun 15, 2016 at 11:44 AM, Ryota Ozaki wrote: > On Wed, Jun 15, 2016 at 11:29 AM, David Young wrote: >> On Wed, Jun 15, 2016 at 10:56:57AM +0900, Ryota Ozaki wrote: >>> On Tue, Jun 14, 2016 at 7:58 PM, Joerg Sonnenberger wrote: >>> > On Tue, Jun 14, 2016 at 09:53:33AM +0900, Ryota Ozaki wrote: >>> >> - curlwp_bind and curlwp_unbind >>> >> - curlwp_bound_set and curlwp_bound_restore >>> >> - curlwp_bound and curlwp_boundx >>> >> >>> >> Any other ideas? :) >>> > >>> > curlwp_bind_push / curlwp_bind_pop >>> >>> Hmm, I think the naming fits in if Linux, but not in NetBSD. >>> And >>> bound = curlwp_bind_push(); >>> ... >>> curlwp_bind_pop(bound); >>> looks odd to me. >> >> bound = curlwp_bind_get(); curlwp_bind_put(bound)? Oops. > I think it's stepping away from the API's intention. The API > actually sets and restores a flag. The return value (and the ^^^ a flag and restores an original state is correct. > argument) is a secondary product and the API name perhaps > shouldn't reflect it. > > ozaki-r
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Wed, Jun 15, 2016 at 11:29 AM, David Young wrote: > On Wed, Jun 15, 2016 at 10:56:57AM +0900, Ryota Ozaki wrote: >> On Tue, Jun 14, 2016 at 7:58 PM, Joerg Sonnenberger wrote: >> > On Tue, Jun 14, 2016 at 09:53:33AM +0900, Ryota Ozaki wrote: >> >> - curlwp_bind and curlwp_unbind >> >> - curlwp_bound_set and curlwp_bound_restore >> >> - curlwp_bound and curlwp_boundx >> >> >> >> Any other ideas? :) >> > >> > curlwp_bind_push / curlwp_bind_pop >> >> Hmm, I think the naming fits in if Linux, but not in NetBSD. >> And >> bound = curlwp_bind_push(); >> ... >> curlwp_bind_pop(bound); >> looks odd to me. > > bound = curlwp_bind_get(); curlwp_bind_put(bound)? I think it's stepping away from the API's intention. The API actually sets and restores a flag. The return value (and the argument) is a secondary product and the API name perhaps shouldn't reflect it. ozaki-r
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Wed, Jun 15, 2016 at 10:56:57AM +0900, Ryota Ozaki wrote: > On Tue, Jun 14, 2016 at 7:58 PM, Joerg Sonnenberger wrote: > > On Tue, Jun 14, 2016 at 09:53:33AM +0900, Ryota Ozaki wrote: > >> - curlwp_bind and curlwp_unbind > >> - curlwp_bound_set and curlwp_bound_restore > >> - curlwp_bound and curlwp_boundx > >> > >> Any other ideas? :) > > > > curlwp_bind_push / curlwp_bind_pop > > Hmm, I think the naming fits in if Linux, but not in NetBSD. > And > bound = curlwp_bind_push(); > ... > curlwp_bind_pop(bound); > looks odd to me. bound = curlwp_bind_get(); curlwp_bind_put(bound)? Dave -- David Young //\ Trestle Technology Consulting (217) 721-9981 Urbana, ILhttp://trestle.tech
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Tue, Jun 14, 2016 at 7:58 PM, Joerg Sonnenberger wrote: > On Tue, Jun 14, 2016 at 09:53:33AM +0900, Ryota Ozaki wrote: >> - curlwp_bind and curlwp_unbind >> - curlwp_bound_set and curlwp_bound_restore >> - curlwp_bound and curlwp_boundx >> >> Any other ideas? :) > > curlwp_bind_push / curlwp_bind_pop Hmm, I think the naming fits in if Linux, but not in NetBSD. And bound = curlwp_bind_push(); ... curlwp_bind_pop(bound); looks odd to me. ozaki-r
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Tue, Jun 14, 2016 at 09:53:33AM +0900, Ryota Ozaki wrote: > - curlwp_bind and curlwp_unbind > - curlwp_bound_set and curlwp_bound_restore > - curlwp_bound and curlwp_boundx > > Any other ideas? :) curlwp_bind_push / curlwp_bind_pop Joerg
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Tue, Jun 14, 2016 at 10:26 AM, Paul Goyette wrote: > On Tue, 14 Jun 2016, Ryota Ozaki wrote: > >>> Perhaps the name should be `curlwp_bound_restore' or something else to >>> emphasize this, but I haven't come up with one that I like better on >>> aesthetic grounds. >> >> >> - curlwp_bind and curlwp_unbind >> - curlwp_bound_set and curlwp_bound_restore >> - curlwp_bound and curlwp_boundx >> >> Any other ideas? :) > > > - curlwp_bind and curlwp_bindx ? > > (I like x for exit, and prefer a present-tense verb vs past tense.) Sure. Actually curlwp_unbind implies that it tries to clear the flag as joerg confused. And x is familiar for us (because of splx) and it implies that it tries to restore the original state. (but I'm not sure it's really better than the others.) Anyway I prepared another patch for curlwp_bindx: http://www.netbsd.org/~ozaki-r/curlwp_bindx.diff ozaki-r
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Tue, 14 Jun 2016, Ryota Ozaki wrote: Perhaps the name should be `curlwp_bound_restore' or something else to emphasize this, but I haven't come up with one that I like better on aesthetic grounds. - curlwp_bind and curlwp_unbind - curlwp_bound_set and curlwp_bound_restore - curlwp_bound and curlwp_boundx Any other ideas? :) - curlwp_bind and curlwp_bindx ? (I like x for exit, and prefer a present-tense verb vs past tense.) +--+--++ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com | | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org | +--+--++
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Mon, Jun 13, 2016 at 11:21 PM, Taylor R Campbell wrote: >Date: Mon, 13 Jun 2016 14:00:16 +0200 >From: Joerg Sonnenberger > >On Mon, Jun 13, 2016 at 07:36:31PM +0900, Ryota Ozaki wrote: >> Currently we do it by open-coding in each place, >> but we should provide some API to simplify codes. >> riastradh@ suggested curlwp_bind and curlwp_unbind >> some time ago (*1) and this patch (*2) just follows >> the idea. > >The primary question for me is whether nesting should be allowed or not. >That would mean a reference count behind the flag. > > This `reference count' gets stored on the stack. The caller does: > > int bound = curlwp_bind(); > > ... psref_wotsit ... > > curlwp_unbind(bound); > > If it was already bound, bound = 1 and curlwp_unbind does nothing; if > it was not already bound, bound = 0 and curlwp_unbind unbinds it. > > Perhaps the name should be `curlwp_bound_restore' or something else to > emphasize this, but I haven't come up with one that I like better on > aesthetic grounds. - curlwp_bind and curlwp_unbind - curlwp_bound_set and curlwp_bound_restore - curlwp_bound and curlwp_boundx Any other ideas? :) ozaki-r
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
Date: Mon, 13 Jun 2016 14:00:16 +0200 From: Joerg Sonnenberger On Mon, Jun 13, 2016 at 07:36:31PM +0900, Ryota Ozaki wrote: > Currently we do it by open-coding in each place, > but we should provide some API to simplify codes. > riastradh@ suggested curlwp_bind and curlwp_unbind > some time ago (*1) and this patch (*2) just follows > the idea. The primary question for me is whether nesting should be allowed or not. That would mean a reference count behind the flag. This `reference count' gets stored on the stack. The caller does: int bound = curlwp_bind(); ... psref_wotsit ... curlwp_unbind(bound); If it was already bound, bound = 1 and curlwp_unbind does nothing; if it was not already bound, bound = 0 and curlwp_unbind unbinds it. Perhaps the name should be `curlwp_bound_restore' or something else to emphasize this, but I haven't come up with one that I like better on aesthetic grounds.
Re: Introduce curlwp_bind and curlwp_unbind for psref(9)
On Mon, Jun 13, 2016 at 07:36:31PM +0900, Ryota Ozaki wrote: > Currently we do it by open-coding in each place, > but we should provide some API to simplify codes. > riastradh@ suggested curlwp_bind and curlwp_unbind > some time ago (*1) and this patch (*2) just follows > the idea. The primary question for me is whether nesting should be allowed or not. That would mean a reference count behind the flag. Joerg
