On 05/29/2014 01:32 PM, Jens Axboe wrote:
> On 05/29/2014 08:25 AM, Jens Axboe wrote:
>>>> +static int nvme_submit_flush_sync(struct nvme_queue *nvmeq, struct
>>>> nvme_ns *ns)
>>>> +{
>>>> +    struct request *req;
>>>> +    struct nvme_command cmnd;
>>>> +
>>>> +    req = blk_mq_alloc_request(ns->queue, WRITE, GFP_KERNEL, false);
>>>> +    if (!req)
>>>> +        return -ENOMEM;
>>>> +
>>>> +    nvme_setup_flush(&cmnd, ns, req->tag);
>>>> +    nvme_submit_sync_cmd(req, &cmnd, NULL, NVME_IO_TIMEOUT);
>>>>
>>>>     return 0;
>>>> }
>>>
>>> It looks like this function above is being called from an interrupt
>>> context where we are already holding a spinlock. The sync command will
>>> try to take that same lock.
>>
>> Yes, that code still looks very buggy. The initial alloc for
>> flush_cmd_info should also retry, not fail hard, if that alloc fails.
>> For the reinsert part, Matias, you want to look at the flush code in
>> blk-mq and how that handles it.
> 
> There's an easy fix for this. Once it's managed by blk-mq, blk-mq will
> decompose requests for you. This means a flush with data will be turned
> into two commands for you, so we can kill this code attempting to handle
> flush request with data.
> 
> Patch attached. Depending on how the series needs to look, the prep
> patch of support bio flush with data should just be dropped however. No
> point in adding that, and the removing it again.

Updated, we can kill the flush_cmd_info struct as well.

-- 
Jens Axboe