Correctly locking a Block Device Request Handler
Hello everyone, I'm currently writing a block device driver and got stuck at trying to understand how to correctly handle the locking in the reqfn one passes to `blk_init_queue`. My code looks like this: static DEFINE_SPINLOCK(rblk_lock); /* Code */ static void rblk_request_handler(struct request_queue *q) __releases(q->queue_lock) __acquires(q->queue_lock) { struct request* req; unsigned long flags = 0; printk(KERN_INFO "rblk: Got request(s) \n"); while ((req = blk_fetch_request(q)) != NULL) { printk(KERN_INFO "rblk: Handling request \n"); // <- Gets printed spin_lock_irqsave(q->queue_lock, flags); blk_end_request_all(req, -ENOTTY); spin_unlock_irqrestore(q->queue_lock, flags); printk(KERN_INFO "rblk: Handled request \n"); // <- Does not get printed } } static in rblk_init() { /* Get major number, allocate devices */ for (i = 0; i < rblk_cnt; i++) { // For each device /* alloc_disk, check for allocation fail */ disk->queue = blk_init_queue(rblk_request_handler, &rblk_lock); } } This didn't work, and it was obvious to me that it was hanging in the spinlock. So I tried removing the locking, (this SO answer says that the queue is already locked: https://stackoverflow.com/questions/19418979/proper-way-to-lock-the-queue-in...) however, I still get the problem that the queue locks up and the second message never hits the message queue. What /does/ work is if I invert the order of locking. As in, unlock first, end_request and then lock again. However that doesn't seem to be the correct way. What am I doing completely wrong, what did I misunderstand? Full code: https://gist.github.com/TheNeikos/8798788defa1a9f316e6 Thanks
Hi On Mon, Nov 16, 2015 at 10:02 PM, Marcel Müller <neikos@neikos.email> wrote:
Hello everyone,
I'm currently writing a block device driver and got stuck at trying to understand how to correctly handle the locking in the reqfn one passes to `blk_init_queue`.
My code looks like this:
static DEFINE_SPINLOCK(rblk_lock);
/* Code */
static void rblk_request_handler(struct request_queue *q) __releases(q->queue_lock) __acquires(q->queue_lock)
as per code, the queue_lock would be held before it enters your request_func. So no you don't need to lock it here. If you must however need to do some stuff that requires the spin_lock_* to be released, you must make sure that before you leave this function you have reacquired that lock.
{ struct request* req; unsigned long flags = 0;
printk(KERN_INFO "rblk: Got request(s) \n"); while ((req = blk_fetch_request(q)) != NULL) { printk(KERN_INFO "rblk: Handling request \n"); // <- Gets printed
This isn't required if this is your request_function. Let's say you just want to consume the requests but want to actually handle them elsewhere then you may require to take the spin_lock_* over there.
spin_lock_irqsave(q->queue_lock, flags);
You are generating error for all requests?
blk_end_request_all(req, -ENOTTY); spin_unlock_irqrestore(q->queue_lock, flags); printk(KERN_INFO "rblk: Handled request \n"); // <- Does not get printed } }
static in rblk_init() { /* Get major number, allocate devices */ for (i = 0; i < rblk_cnt; i++) { // For each device /* alloc_disk, check for allocation fail */
disk->queue = blk_init_queue(rblk_request_handler, &rblk_lock); } }
This didn't work, and it was obvious to me that it was hanging in the spinlock. So I tried removing the locking, (this SO answer says that the queue is already locked: https://stackoverflow.com/questions/19418979/proper-way-to-lock-the-queue-in...) however, I still get the problem that the queue locks up and the second message never hits the message queue.
What /does/ work is if I invert the order of locking. As in, unlock first, end_request and then lock again. However that doesn't seem to be the correct way. What am I doing completely wrong, what did I misunderstand?
Full code: https://gist.github.com/TheNeikos/8798788defa1a9f316e6
You can check one here https://github.com/pranjas/block_driver. No real device though.
Thanks
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
-- ---P.K.S
On 11/18/2015 07:11 AM, Pranay Srivastava wrote:
Hi
On Mon, Nov 16, 2015 at 10:02 PM, Marcel Müller <neikos@neikos.email> wrote:
Hello everyone,
I'm currently writing a block device driver and got stuck at trying to understand how to correctly handle the locking in the reqfn one passes to `blk_init_queue`.
My code looks like this:
static DEFINE_SPINLOCK(rblk_lock);
/* Code */
static void rblk_request_handler(struct request_queue *q) __releases(q->queue_lock) __acquires(q->queue_lock) as per code, the queue_lock would be held before it enters your request_func. So no you don't need to lock it here.
If you must however need to do some stuff that requires the spin_lock_* to be released, you must make sure that before you leave this function you have reacquired that lock.
{ struct request* req; unsigned long flags = 0;
printk(KERN_INFO "rblk: Got request(s) \n"); while ((req = blk_fetch_request(q)) != NULL) { printk(KERN_INFO "rblk: Handling request \n"); // <- Gets printed
This isn't required if this is your request_function. Let's say you just want to consume the requests but want to actually handle them elsewhere then you may require to take the spin_lock_* over there.
spin_lock_irqsave(q->queue_lock, flags);
You are generating error for all requests?
blk_end_request_all(req, -ENOTTY); spin_unlock_irqrestore(q->queue_lock, flags); printk(KERN_INFO "rblk: Handled request \n"); // <- Does not get printed } }
static in rblk_init() { /* Get major number, allocate devices */ for (i = 0; i < rblk_cnt; i++) { // For each device /* alloc_disk, check for allocation fail */
disk->queue = blk_init_queue(rblk_request_handler, &rblk_lock); } }
This didn't work, and it was obvious to me that it was hanging in the spinlock. So I tried removing the locking, (this SO answer says that the queue is already locked: https://stackoverflow.com/questions/19418979/proper-way-to-lock-the-queue-in...) however, I still get the problem that the queue locks up and the second message never hits the message queue.
What /does/ work is if I invert the order of locking. As in, unlock first, end_request and then lock again. However that doesn't seem to be the correct way. What am I doing completely wrong, what did I misunderstand?
Full code: https://gist.github.com/TheNeikos/8798788defa1a9f316e6
You can check one here https://github.com/pranjas/block_driver. No real device though.
Thanks
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Hello, Thank you! Your explanations and code showed me some new things one can do, so that definitely helped! - Marcel
participants (2)
-
Marcel Müller -
Pranay Srivastava