io_request_lock question (2.2)

Hi

I'm trying to write a block device driver which does some network stuff to satisfy the requests. The problem is, that the network stuff wants to grab the io_request_lock which does not work because this lock is already locked when I come into the request_fn of my device.

I looked at the implementation of the nbd which just calls

	spin_unlock_irq(&io_request_lock);
	... do network io ...
	spin_lock_irq(&io_request_lock);

This seems to work but it looks very dangerous to me (and ugly, too). Isn't there a better way to do this?

Thanks very much

Reto

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

It is only dangerous if you unlock it in the wrong place, unlocking it as much as possible is good behaviour. You need it locked until you get the actual request off the queue, you need it locked when you complete the request. The rest of the time you can be polite

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I'm sorry but I still have some doubts:

The add_request function calls

	spin_lock_irqsave(&io_request_lock,flags); 
and then calls our request_fn which does 
	spin_unlock_irq(&io_request_lock);

	...do network I/O ...

	spin_lock_irq(&io_request_lock);
we finish the request and return to the add_request function which calls
	spin_unlock_irqrestore(&io_request_lock,flags);

and restores the flags.

Isn't it possible now that the flags which we restore are out of date now?

Is this idiom the right one to use for 2.2?

Thanks,

Reto
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

It is fine for 2.2 as well.

The flags you restore are ok. It restores the interrupt state to the state it was in when you were called. Think of save_flags/restore_flags as bracketing regions of code (and being nestable in pairs). The only real bizarre rule is that you cannot save_flags in one function and restore_flags in another without upsetting DaveM - as it breaks on the sparc if you do that

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Yes, and I believe that this is what's broken about the SCSI midlayer. The the io_request_lock cannot be completely released in a SCSI HBA because the flags for that save are way up the stack somewhere. This means that if you need to, for example, sleep on something like loop coming up, you can spin_unlock_irq, but you can't restore flags, so for the UP case, you just hang. To avoid this, I'm having to build a kernel thread to handle all of this kind of stuff in a separate context. What a PITA.

-matt

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

You can drop it with spin_unlock_irq and that is fine. I do that with no problems in the I2O scsi driver for example

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I am (like, I think I *finally* got locking sorta right in my QLogic driver), but doesn't this still leave ints blocked for this CPU at least?

-matt

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I am actually concerned about the following case:

The add_request ON CPU_1 function calls

        spin_lock_irqsave(&io_request_lock,flags); 

Our I/O Function unlocks the spinlock and goes to sleep.

Finally, the add_request function, NOW ON CPU_2 calls

        spin_unlock_irqrestore(&io_request_lock,flags);

and restores the flags of CPU_1 on CPU_2.

What am I missing? Are the flags which we restore valid for all the CPU's the same?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Absolutely not. They're state *for that CPU*.

Before I gave up doing my own locking in my HBA, I was using flags in my softc, so I could do things like this in transitioning from an int svc routine to another module (in this case, a SCSI target mode handler):

	<interrupt starts>
	spinlock_irq_save(&isp->isp_lk, isp->isp_lkflags);
	<get stuff from response queue>
	spinlock_irq_restore(&isp->isp_lk, isp->isp_lkflags);
	<call scsi_target_mode_handler>
	spinlock_irq_save(&isp->isp_lk, isp->isp_lkflags);
	<reinstruct card>
	spinlock_irq_restore(&isp->isp_lk, isp->isp_lkflags);
	(return from interrupt)

So, if you acquire the lock, the flags go with *that* CPU's acquisition of the lock. But that's dangerous in that you're passing the flags around to be possibly 'restored' (incorrectly) in some other context. That's why flags seems to be usually an auto variable construct.

But what I still don't get is why it's okay to do a spin_unlock_irq on io_request_lock (because you don't have access to the flags that were locked with it (if any)) and not end up with a system that can deadlock. Well, enlightenment will come some day I'm sure.

-matt

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

spin_unlock_irq unconditionally enables

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

spin_unlock_irq() does a __sti()
spin_unlock() doesn't.

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Umm. Okay, but you haven't changed your processor priority though, right? (I just don't *get* i386 stuff... I'll go off and ponder SParc code - &that& I understand).

-matt

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

There is no concept of spl levels in Linux ;) It only knows about interrupts on or off.

It is very simple[1] : interrupts can be on or off. __cli() disables them, __sti() enables them again.

When you want nesting use the _flags versions.

-Andi
[1] in linux, there are some architecture extensions for interrupt priorities, but linux doesn't use them.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/