* [KJ] fs/smbfs/proc.c -> replace direct assignment with
2005-06-30 8:28 [KJ] fs/smbfs/proc.c -> replace direct assignment with aLeJ
@ 2005-06-30 8:28 ` aLeJ
2005-06-30 16:12 ` Ricardo Nabinger Sanchez
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: aLeJ @ 2005-06-30 8:28 UTC (permalink / raw)
To: kernel-janitors
Description:
Use __set_current_state() instead of using a direct assignment
Signed-off-by: Alejandro Andrés <fuzzy.alej@gmail.com>
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [KJ] fs/smbfs/proc.c -> replace direct assignment with
2005-06-30 8:28 [KJ] fs/smbfs/proc.c -> replace direct assignment with aLeJ
2005-06-30 8:28 ` aLeJ
@ 2005-06-30 16:12 ` Ricardo Nabinger Sanchez
2005-06-30 17:53 ` aLeJ
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Ricardo Nabinger Sanchez @ 2005-06-30 16:12 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]
Quoting aLeJ <fuzzy.alej@gmail.com>
Sent on Thu, 30 Jun 2005 12:16:13 +0200
> --- fs/smbfs/proc.c.orig 2005-06-22 21:33:05.000000000 +0200
> +++ fs/smbfs/proc.c 2005-06-30 12:02:45.000000000 +0200
> @@ -2397,7 +2397,7 @@ smb_proc_readdir_long(struct file *filp,
> if (req->rq_rcls == ERRSRV && req->rq_err == ERRerror) {
> /* a damn Win95 bug - sometimes it clags if you
> ask it too fast */
> - current->state = TASK_INTERRUPTIBLE;
> + set_current_state(TASK_INTERRUPTIBLE);
the memory barrier is really necessary (as pointed out earlier in the list
by Nishanth Aravamudan)? if not, __set_current_state does the job.
I'd appreciate some notes/thoughts/tips/whatever on which conditions a
memory barrier is really needed, or, more clearly, when the absence of a
memory barrier breaks stuff.
TIA :)
--
Ricardo Nabinger Sanchez
GNU/Linux #140696 [http://counter.li.org]
Slackware Linux + FreeBSD
How long a minute is depends on which
side of the bathroom door you're on.
[-- 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [KJ] fs/smbfs/proc.c -> replace direct assignment with
2005-06-30 8:28 [KJ] fs/smbfs/proc.c -> replace direct assignment with aLeJ
2005-06-30 8:28 ` aLeJ
2005-06-30 16:12 ` Ricardo Nabinger Sanchez
@ 2005-06-30 17:53 ` aLeJ
2005-06-30 18:39 ` Nishanth Aravamudan
2005-06-30 20:03 ` aLeJ
4 siblings, 0 replies; 6+ messages in thread
From: aLeJ @ 2005-06-30 17:53 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 622 bytes --]
> the memory barrier is really necessary (as pointed out earlier in the list
> by Nishanth Aravamudan)? if not, __set_current_state does the job.
>
> I'd appreciate some notes/thoughts/tips/whatever on which conditions a
> memory barrier is really needed, or, more clearly, when the absence of a
> memory barrier breaks stuff.
>
> TIA :)
As said in the TODO:
"...
Don't forget to set_current_state(TASK_{,UN}INTERRUPTIBLE); before
schedule*().
..."
And in this case, it is just before schedule, so I considered to change it.
Maybe the use of __set_current is for places where there is not schedule?
(Just a try :p)
[-- 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [KJ] fs/smbfs/proc.c -> replace direct assignment with
2005-06-30 8:28 [KJ] fs/smbfs/proc.c -> replace direct assignment with aLeJ
` (2 preceding siblings ...)
2005-06-30 17:53 ` aLeJ
@ 2005-06-30 18:39 ` Nishanth Aravamudan
2005-06-30 20:03 ` aLeJ
4 siblings, 0 replies; 6+ messages in thread
From: Nishanth Aravamudan @ 2005-06-30 18:39 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1409 bytes --]
On 30.06.2005 [21:48:56 +0200], aLeJ wrote:
> > the memory barrier is really necessary (as pointed out earlier in the list
> > by Nishanth Aravamudan)? if not, __set_current_state does the job.
> >
> > I'd appreciate some notes/thoughts/tips/whatever on which conditions a
> > memory barrier is really needed, or, more clearly, when the absence of a
> > memory barrier breaks stuff.
> >
> > TIA :)
>
> As said in the TODO:
>
> "...
> Don't forget to set_current_state(TASK_{,UN}INTERRUPTIBLE); before
> schedule*().
> ..."
>
> And in this case, it is just before schedule, so I considered to change it.
> Maybe the use of __set_current is for places where there is not schedule?
> (Just a try :p)
So, when you are adding yourself to a wait-queue, you have to be set in
the appropriate state to make sure you receive the appropriate wake-up
events.
When removing yourself, though, it is less critical, so you are able to
use __set_current_state() [which is the same as direct assignment].
I had a mail from Oliver Neukum which discussed these issues, but am
having some trouble locating it. Once I do, I'll send it back out in
reply to this thread.
In any case, this patch is not necessary, as I already have a patch in
the -KJ tree which replaces the state change and timeout with msleep().
Please make sure your fixes don't collide with existing -kj fixes (or
-mm ones, if possible).
Thanks,
Nish
[-- 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [KJ] fs/smbfs/proc.c -> replace direct assignment with
2005-06-30 8:28 [KJ] fs/smbfs/proc.c -> replace direct assignment with aLeJ
` (3 preceding siblings ...)
2005-06-30 18:39 ` Nishanth Aravamudan
@ 2005-06-30 20:03 ` aLeJ
4 siblings, 0 replies; 6+ messages in thread
From: aLeJ @ 2005-06-30 20:03 UTC (permalink / raw)
To: kernel-janitors
El jue, 30-06-2005 a las 11:39 -0700, Nishanth Aravamudan escribió:
> On 30.06.2005 [21:48:56 +0200], aLeJ wrote:
> > > the memory barrier is really necessary (as pointed out earlier in the list
> > > by Nishanth Aravamudan)? if not, __set_current_state does the job.
> > >
> > > I'd appreciate some notes/thoughts/tips/whatever on which conditions a
> > > memory barrier is really needed, or, more clearly, when the absence of a
> > > memory barrier breaks stuff.
> > >
> > > TIA :)
> >
> > As said in the TODO:
> >
> > "...
> > Don't forget to set_current_state(TASK_{,UN}INTERRUPTIBLE); before
> > schedule*().
> > ..."
> >
> > And in this case, it is just before schedule, so I considered to change it.
> > Maybe the use of __set_current is for places where there is not schedule?
> > (Just a try :p)
>
> So, when you are adding yourself to a wait-queue, you have to be set in
> the appropriate state to make sure you receive the appropriate wake-up
> events.
>
> When removing yourself, though, it is less critical, so you are able to
> use __set_current_state() [which is the same as direct assignment].
>
> I had a mail from Oliver Neukum which discussed these issues, but am
> having some trouble locating it. Once I do, I'll send it back out in
> reply to this thread.
>
> In any case, this patch is not necessary, as I already have a patch in
> the -KJ tree which replaces the state change and timeout with msleep().
> Please make sure your fixes don't collide with existing -kj fixes (or
> -mm ones, if possible).
>
> Thanks,
> Nish
Ok, I'll be waiting the replay to see the mail! :D
And thanks, I'm very newbie and I love learning so all help given to me
is very appreciated.
Thank you,
aLeJ. ;)
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread