linux-kernel - Re: [PATCH] blk-wbt: get back the missed wakeup from __wbt

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: <00445460-d826-4d85-c5f4-fa565892da01@oracle.com>
Date:   Mon, 27 Aug 2018 11:52:26 +0800
From:   "jianchao.wang" <jianchao.w.wang@...cle.com>
To:     Jens Axboe <axboe@...nel.dk>, Anchal Agarwal <anchalag@...n.com>
Cc:     fllinden@...zon.com,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] blk-wbt: get back the missed wakeup from __wbt_done

Hi Jens

On 08/25/2018 11:41 PM, Jens Axboe wrote:
>  	do {
> -		set_current_state(TASK_UNINTERRUPTIBLE);
> +		if (test_bit(0, &data.flags))
> +			break;
>  
> -		if (!has_sleeper && rq_wait_inc_below(rqw, get_limit(rwb, rw)))
> +		WARN_ON_ONCE(list_empty(&data.wq.entry));
> +
> +		if (!has_sleeper &&
> +		    rq_wait_inc_below(rqw, get_limit(rwb, rw))) {
> +			finish_wait(&rqw->wait, &data.wq);
> +
> +			/*
> +			 * We raced with wbt_wake_function() getting a token,
> +			 * which means we now have two. Put ours and wake
> +			 * anyone else potentially waiting for one.
> +			 */
> +			if (test_bit(0, &data.flags))
> +				wbt_rqw_done(rwb, rqw, wb_acct);
>  			break;

Just use 'bool' variable should be OK 
After finish_wait, no one could race with us here.

> +		}
>  
>  		if (lock) {
>  			spin_unlock_irq(lock);
> @@ -511,11 +569,11 @@ static void __wbt_wait(struct rq_wb *rwb, enum wbt_flags wb_acct,
>  			spin_lock_irq(lock);
>  		} else
>  			io_schedule();
> +
>  		has_sleeper = false;
>  	} while (1);

I cannot get the point of "since we can't rely on just being woken from the ->func handler
we set".
Do you mean there could be someone else could wake up this task ?

Thanks
Jianchao