[Kernel-janitors] Re: set_current_state

[Kernel-janitors] Re: set_current_state

[Domen Puncer]

[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

On 27/06/04 14:24 -0500, Jay Bourque wrote:
> New guy here,
> 
> I've been following the discussion lately and with talk about the
> TODO list badly needing updating, I was wondering how up to
> date the kj.pl script is.  In particular, I ran it against the
> net/core/dev.c file and it showed that current->state needs to be
> changed to set_current_state. Is this still something that needs
> to be done?

Yes.
If you look at set_current_state, you see it's basically the same, with
a memory barrier.

> 
> Thanks,
> -Jay
> _______________________________________________
> Kernel-janitors mailing list
> Kernel-janitors@lists.osdl.org
> http://lists.osdl.org/mailman/listinfo/kernel-janitors

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] Re: set_current_state

[Anupam Kapoor]

[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]

hi,

i just checked with the latest sources and there are 417 entries with
'current->state'. is a blind search-replace to set_current_state ok ?

can you please let me know ?

kind regards
anupam

On Mon, 28 Jun 2004 00:33:56 +0200, Domen Puncer <domen@coderock.org> wrote:
> 
> On 27/06/04 14:24 -0500, Jay Bourque wrote:
> > New guy here,
> >
> > I've been following the discussion lately and with talk about the
> > TODO list badly needing updating, I was wondering how up to
> > date the kj.pl script is.  In particular, I ran it against the
> > net/core/dev.c file and it showed that current->state needs to be
> > changed to set_current_state. Is this still something that needs
> > to be done?
> 
> Yes.
> If you look at set_current_state, you see it's basically the same, with
> a memory barrier.
> 
> 
> 
> >
> > Thanks,
> > -Jay
> > _______________________________________________
> > Kernel-janitors mailing list
> > Kernel-janitors@lists.osdl.org
> > http://lists.osdl.org/mailman/listinfo/kernel-janitors
> 
> 
> 
> noname - 1K Download 
>

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

[Kernel-janitors] Re: set_current_state

[Domen Puncer]

[-- Attachment #1: Type: text/plain, Size: 340 bytes --]

On 28/06/04 18:51 +0530, Anupam Kapoor wrote:
> hi,
> 
> i just checked with the latest sources and there are 417 entries with
> 'current->state'. is a blind search-replace to set_current_state ok ?

Yes, usualy.
Or maybe you should use __set_current_state (but do we really care that
much about performance before schedule()?).

Opinions?

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

[Kernel-janitors] Re: set_current_state

[Anupam Kapoor]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

On Mon, 28 Jun 2004 16:33:12 +0200, Domen Puncer <domen@coderock.org> wrote:
> On 28/06/04 18:51 +0530, Anupam Kapoor wrote:
> > hi,
> >
> > i just checked with the latest sources and there are 417 entries with
> > 'current->state'. is a blind search-replace to set_current_state ok ?
> 
> Yes, usualy.
> Or maybe you should use __set_current_state (but do we really care that
> much about performance before schedule()?).
> 
> Opinions?
hmm, i am not sure about this but does it make sense to use
__set_current_state for functions that have fastcall ? and use
set_current_state for others ?

kind regards
anupam

>

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] Re: set_current_state

[Felipe W Damasio]

[-- Attachment #1: Type: text/plain, Size: 841 bytes --]



Anupam Kapoor wrote:

> hmm, i am not sure about this but does it make sense to use
> __set_current_state for functions that have fastcall ? and use
> set_current_state for others ?

	It's not _that_ simple. We need set_current_state to racing if 
instructions are reordered.

	__set_current_state should *always* be used when setting to TASK_RUNNING.

	But since function calls act also as memory barriers and most of the 
locking primitives also have memory barriers built into them, there are 
few places where we actually need set_current_state (rather than 
__set_current_state).

	Please check the LKML archives for further explanations.

	But in short: No, it's not just a blind search-and-replace kind of thing.

	Cheers,

Felipe
-- 
It's most certainly GNU/Linux, not Linux. Read more at
http://www.gnu.org/gnu/why-gnu-linux.html

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] Re: set_current_state

[Anupam Kapoor]

[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]

On Wed, 30 Jun 2004 09:30:12 -0300, Felipe W Damasio
<felipewd@terra.com.br> wrote:
> 
> 
> Anupam Kapoor wrote:
> 
> > hmm, i am not sure about this but does it make sense to use
> > __set_current_state for functions that have fastcall ? and use
> > set_current_state for others ?
> 
>         It's not _that_ simple. We need set_current_state to racing if
> instructions are reordered.
can you please elaborate what you mean here, i don't understand the
connotations sorry !

> 
>         __set_current_state should *always* be used when setting to TASK_RUNNING.
> 
>         But since function calls act also as memory barriers and most of the
> locking primitives also have memory barriers built into them, there are
> few places where we actually need set_current_state (rather than
> __set_current_state).
ah ok ! thanks for the explanation. i will check out lkml for more info.

kind regards
anupam
> 
>         Please check the LKML archives for further explanations.
> 
>         But in short: No, it's not just a blind search-and-replace kind of thing.
> 
>         Cheers,
> 
> Felipe
> --
> It's most certainly GNU/Linux, not Linux. Read more at
> http://www.gnu.org/gnu/why-gnu-linux.html
>

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] Re: set_current_state

[Felipe W Damasio]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

Anupam Kapoor wrote:
> can you please elaborate what you mean here, i don't understand the
> connotations sorry !

	There's the classic example that rml used when he wrote a few 
explanation notes of when set_current_state should be used.

	If we're setting the current task to TASK_* (other than RUNNING) and 
after that, manipulating some variable:

	current->state = TASK_INTERRUPTIBLE;
	if (doritos)
	{
		...
	}

	And on some of other code path we're checking to see if we're in 
TASK_INTERRUPTIBLE to also use the same variable:

	if (current->state == TASK_INTERRUPTIBLE)
	{
		do-something-to-doritos;
	}

	Then, in this case, we need a memory barrier the guarantee that the 
doritos variable will have the state set correctly _before_ manipulating it.

	But again, state setting that are still open coded are not that simple 
to see if we need set_current_state.

	Most places it's not the end of the world to have an useless memory 
barrier, but if we could things right and save an useless 
instruction..it'd be better.

	In short: If in doubt, always use set_current_state. :-)

	Cheers,

Felipe
-- 
It's most certainly GNU/Linux, not Linux. Read more at
http://www.gnu.org/gnu/why-gnu-linux.html

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors