linux-kernel - Re: [PATCH V3] NVMe: basic conversion to blk-mq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 29 May 2014 13:32:45 -0600
From:	Jens Axboe <axboe@...nel.dk>
To:	Keith Busch <keith.busch@...el.com>,
	Matias Bjørling <m@...rling.me>
CC:	willy@...ux.intel.com, sbradshaw@...ron.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V3] NVMe: basic conversion to blk-mq

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.

-- 
Jens Axboe


View attachment "nvme-flush.patch" of type "text/x-patch" (2179 bytes)