linux-kernel - Re: [PATCH v9 2/2] blk-mq: Rework blk-mq timeout handling again

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]

Message-ID: <8058d72fef475caffa78c962bc4220f5f8fa3dde.camel@wdc.com>
Date:   Wed, 16 May 2018 16:17:42 +0000
From:   Bart Van Assche <Bart.VanAssche@....com>
To:     "hch@....de" <hch@....de>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "israelr@...lanox.com" <israelr@...lanox.com>,
        "sagi@...mberg.me" <sagi@...mberg.me>,
        "sebott@...ux.ibm.com" <sebott@...ux.ibm.com>,
        "ming.lei@...hat.com" <ming.lei@...hat.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "jianchao.w.wang@...cle.com" <jianchao.w.wang@...cle.com>,
        "maxg@...lanox.com" <maxg@...lanox.com>,
        "tj@...nel.org" <tj@...nel.org>
Subject: Re: [PATCH v9 2/2] blk-mq: Rework blk-mq timeout handling again

On Wed, 2018-05-16 at 14:51 +0200, Christoph Hellwig wrote:
> I've been looking at this carefully, and I don't think we need cmpxchg64
> at all, and we don't need anywhere near as many cmpxchg operations either.
> 
> The only reason to include the deadline in the atomic operation is the
> blk_abort_request case, as the blk_mq_add_timer never modifies the
> deadline of a request that someone could be racing with.  So if we
> introduce a new aborted state for use by blk_abort_request we can modify
> the deadline separately (and in fact have a common field with the legacy
> path).

There is another reason the deadline is included in the atomic operation,
namely to handle races between the BLK_EH_RESET_TIMER case in blk_mq_rq_timed_out()
and blk_mq_complete_request(). I don't think that race is addressed properly by
your patch. I will see what I can do to address that race without using 64-bit
atomic operations.

Bart.