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]

Date:   Wed, 16 May 2018 19:31:58 +0200
From:   "hch@....de" <hch@....de>
To:     Bart Van Assche <Bart.VanAssche@....com>
Cc:     "hch@....de" <hch@....de>,
        "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>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "ming.lei@...hat.com" <ming.lei@...hat.com>,
        "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, May 16, 2018 at 04:47:54PM +0000, Bart Van Assche wrote:
> I think your patch changes the order of changing the request state and
> calling mod_timer(). In my patch the request state and the deadline are
> updated first and mod_timer() is called afterwards. I think your patch
> changes the order of these operations into the following:
> (1) __blk_mq_start_request() sets the request deadline.
> (2) __blk_mq_start_request() calls __blk_add_timer() which in turn calls
>     mod_timer().
> (3) __blk_mq_start_request() changes the request state into MQ_RQ_IN_FLIGHT.
> 
> In the unlikely event of a significant delay between (2) and (3) it can
> happen that the timer fires and examines and ignores the request because
> its state differs from MQ_RQ_IN_FLIGHT. If the request for which this
> happened times out its timeout will only be handled the next time
> blk_mq_timeout_work() is called. Is this the behavior you intended?

We can move the timer manipulation after the change easily I think.
It would make sense to add comments explaining the ordering.