linux-kernel - Re: [PATCH v3 08/14] blk-mq: remove unncessary error count and commit in blk_mq_plug_issue

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: <20230111050612.GB16286@lst.de>
Date:   Wed, 11 Jan 2023 06:06:12 +0100
From:   Christoph Hellwig <hch@....de>
To:     Kemeng Shi <shikemeng@...weicloud.com>
Cc:     hch@....de, axboe@...nel.dk, dwagner@...e.de, hare@...e.de,
        ming.lei@...hat.com, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, john.garry@...wei.com, jack@...e.cz
Subject: Re: [PATCH v3 08/14] blk-mq: remove unncessary error count and
 commit in blk_mq_plug_issue_direct

On Wed, Jan 11, 2023 at 09:01:53PM +0800, Kemeng Shi wrote:
> We need only to explicitly commit in two error cases:
>  -did not queue everything initially scheduled to queue
>  -the last attempt to queue a request failed
> (see comment of blk_mq_commit_rqs for more details).
> Both cases can be checked with ret of last request which breaks list walk.
> Remove unnecessary error count and unnecessary commit triggered by error
> which is not covered by cases described above.
> 
> Signed-off-by: Kemeng Shi <shikemeng@...weicloud.com>
> ---
>  block/blk-mq.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 98f6003474f2..c6c84f44c7a6 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2706,11 +2706,10 @@ static void blk_mq_plug_issue_direct(struct blk_plug *plug)
>  	struct blk_mq_hw_ctx *hctx = NULL;
>  	struct request *rq;
>  	int queued = 0;
> -	int errors = 0;
> +	blk_status_t ret;

I think we need to initialize this to BLK_STS_OK here.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@....de>