[KJ] fs/smbfs/proc.c -> replace direct assignment with

[KJ] fs/smbfs/proc.c -> replace direct assignment with

[aLeJ]

Description:
Use set_current_state() instead of using a direct assignment

Signed-off-by: Alejandro Andrés <fuzzy.alej@gmail.com>

--- 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);
 			schedule_timeout(HZ/5);
 			continue;
                 }


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

[KJ] fs/smbfs/proc.c -> replace direct assignment with

[aLeJ]

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

Re: [KJ] fs/smbfs/proc.c -> replace direct assignment with

[Ricardo Nabinger Sanchez]

[-- 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

Re: [KJ] fs/smbfs/proc.c -> replace direct assignment with

[aLeJ]

[-- 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

Re: [KJ] fs/smbfs/proc.c -> replace direct assignment with

[Nishanth Aravamudan]

[-- 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

Re: [KJ] fs/smbfs/proc.c -> replace direct assignment with

[aLeJ]

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