Correctly locking a Block Device Request Handler

[Marcel Müller]

On 11/18/2015 07:11 AM, Pranay Srivastava wrote:
> 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
>
>
Hello,

Thank you! Your explanations and code showed me some new things one can
do, so
that definitely helped!

- Marcel