linux-kernel - Re: Re: [PATCH 1/3] blk-mq: remove redundant call to blk_freeze_queue_start in blk_mq_destroy

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: <42681e4e.15223d.18426b71124.Coremail.nickyc975@zju.edu.cn>
Date:   Sun, 30 Oct 2022 10:27:22 +0800 (GMT+08:00)
From:   "Jinlong Chen" <nickyc975@....edu.cn>
To:     "Bart Van Assche" <bvanassche@....org>
Cc:     axboe@...nel.dk, kbusch@...nel.org, hch@....de, sagi@...mberg.me,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-nvme@...ts.infradead.org
Subject: Re: Re: [PATCH 1/3] blk-mq: remove redundant call to
 blk_freeze_queue_start in blk_mq_destroy_queue

Hi, Bart.

> > Calling blk_freeze_queue results in a redundant call to
> > blk_freeze_queue_start as it has been called in blk_queue_start_drain.
> > 
> > Replace blk_freeze_queue with blk_mq_freeze_queue_wait to avoid the
> > redundant call.
> 
> blk_mq_destroy_queue() has more callers than blk_queue_start_drain() so 
> the above description is at least misleading.
> 
> Additionally, the word "cleanup" from the patch series title indicates 
> that no patch in this series changes the behavior of the code. This 
> patch involves a behavior change.

Sorry for my poor description. I'll send a new series with these
description problems resolved.

> I think this patch introduces a hang for every caller of 
> blk_mq_destroy_queue() other than blk_queue_start_drain().
> 
> Bart.

I don't see why the patch introduces a hang. The calling relationship in
blk_mq_destroy_queue is as follows:

blk_mq_destroy_queue()
    ...
    -> blk_queue_start_drain()
        -> blk_freeze_queue_start()  <- called
        ...
    -> blk_freeze_queue()
        -> blk_freeze_queue_start()  <- called again
        -> blk_mq_freeze_queue_wait()
    ...

So I think there is a redundant call to blk_freeze_queue_start(), we
just need to call blk_mq_freeze_queue_wait() after calling
blk_queue_start_drain().

Thanks!
Jinlong Chen