Skip to contentSkip to conversation

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

KJ aLeJ 3 participants 6 messages Jun 30, 2005 Kernel janitors

lore
Share & tools
lore

aLeJ

author
#0 lore raw

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

smbfs/proc.c renamed from smbfs/proc.c.orig +1 -1
@@ -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
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#2 lore raw
In reply to aLeJ

Quoting aLeJ <[email protected]>
Sent on Thu, 30 Jun 2005 12:16:13 +0200

smbfs/proc.c smb_proc_readdir_long(struct file *filp, · line 2400

Quoted 8 lines
smbfs/proc.c renamed from smbfs/proc.c.orig +1 -1
@@ -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 :)

Signature · 6 lines
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.

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

author
#3 lore raw
In reply to aLeJ
Quoted 8 lines

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)

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#4 lore raw
In reply to aLeJ
Quoted 11 lines · aLeJ
Quoted 8 lines

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:

Quoted 8 lines

"...
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

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

author
#5 lore raw
In reply to aLeJ
Quoted 32 lines · Nishanth Aravamudan
Quoted 11 lines · aLeJ
Quoted 8 lines

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:

Quoted 8 lines

"...
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.

Quoted 7 lines

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
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors