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  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, 21 Dec 2017 11:56:49 +0800
From:   "jianchao.wang" <jianchao.w.wang@...cle.com>
To:     Tejun Heo <tj@...nel.org>, jbacik@...com
Cc:     jack@...e.cz, axboe@...nel.dk, clm@...com, kernel-team@...com,
        linux-kernel@...r.kernel.org, linux-btrfs@...r.kernel.org,
        peterz@...radead.org, Bart.VanAssche@....com
Subject: Re: [PATCH 5/7] blk-mq: remove REQ_ATOM_COMPLETE usages from blk-mq

Hi tejun

On 12/16/2017 08:07 PM, Tejun Heo wrote:
> After the recent updates to use generation number and state based
> synchronization, blk-mq no longer depends on REQ_ATOM_COMPLETE except
> to avoid firing the same timeout multiple times.
> 
> Remove all REQ_ATOM_COMPLETE usages and use a new rq_flags flag
> RQF_MQ_TIMEOUT_EXPIRED to avoid firing the same timeout multiple
> times.  This removes atomic bitops from hot paths too.
> 
> v2: Removed blk_clear_rq_complete() from blk_mq_rq_timed_out().
> 
> v3: Added RQF_MQ_TIMEOUT_EXPIRED flag.
> 
> Signed-off-by: Tejun Heo <tj@...nel.org>
> Cc: "jianchao.wang" <jianchao.w.wang@...cle.com>
> ---
>  block/blk-mq.c         | 18 ++++++++----------
>  block/blk-timeout.c    |  1 +
>  include/linux/blkdev.h |  2 ++
>  3 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 88baa82..47e722b 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -607,14 +607,12 @@ void blk_mq_complete_request(struct request *rq)
>  	 */
>  	if (!(hctx->flags & BLK_MQ_F_BLOCKING)) {
>  		rcu_read_lock();
> -		if (blk_mq_rq_aborted_gstate(rq) != rq->gstate &&
> -		    !blk_mark_rq_complete(rq))
> +		if (blk_mq_rq_aborted_gstate(rq) != rq->gstate)
>  			__blk_mq_complete_request(rq);
>  		rcu_read_unlock();
>  	} else {
>  		srcu_idx = srcu_read_lock(hctx->queue_rq_srcu);
> -		if (blk_mq_rq_aborted_gstate(rq) != rq->gstate &&
> -		    !blk_mark_rq_complete(rq))
> +		if (blk_mq_rq_aborted_gstate(rq) != rq->gstate)
>  			__blk_mq_complete_request(rq);
>  		srcu_read_unlock(hctx->queue_rq_srcu, srcu_idx);
>  	}
It's worrying that even though the blk_mark_rq_complete() here is intended to synchronize with
timeout path, but it indeed give the blk_mq_complete_request() the capability to exclude with 
itself. Maybe this capability should be reserved.

Thanks
Jianchao

Powered by blists - more mailing lists