Correctly locking a Block Device Request Handler

[Pranay Srivastava]

Hi

On Mon, Nov 16, 2015 at 10:02 PM, Marcel Müller <neikos at 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-a-block-device-driver-while-serving-requests)
> 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 at kernelnewbies.org
> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies



-- 
        ---P.K.S