linux-kernel - Re: [lustre-devel] [PATCH] staging : lustre : Remove braces from single-line body.

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] [day] [month] [year] [list]

Date:   Fri, 16 Dec 2016 17:23:03 +0000
From:   "Xiong, Jinshan" <jinshan.xiong@...el.com>
To:     Joe Perches <joe@...ches.com>
CC:     Tabrez khan <khan.tabrez21@...il.com>,
        "Dilger, Andreas" <andreas.dilger@...el.com>,
        "jsimmons@...radead.org" <jsimmons@...radead.org>,
        "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "lustre-devel@...ts.lustre.org" <lustre-devel@...ts.lustre.org>
Subject: Re: [lustre-devel] [PATCH] staging : lustre : Remove braces from
 single-line body.


> On Dec 16, 2016, at 8:05 AM, Joe Perches <joe@...ches.com> wrote:
> 
> On Fri, 2016-12-16 at 19:59 +0530, Tabrez khan wrote:
>> Remove unnecessary braces {} for single while statement.
> 
> Your patch is fine Tabrez, but to the lustre folk:
> 
>> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_io.c b/drivers/staging/lustre/lustre/obdclass/cl_io.c
> []
>> @@ -1371,9 +1371,9 @@ int cl_sync_io_wait(const struct lu_env *env, struct cl_sync_io *anchor,
> 	LASSERT(atomic_read(&anchor->csi_sync_nr) == 0);
> 
> 	/* wait until cl_sync_io_note() has done wakeup */
> -	while (unlikely(atomic_read(&anchor->csi_barrier) != 0)) {
> +	while (unlikely(atomic_read(&anchor->csi_barrier) != 0))
> 		cpu_relax();
> -	}
> +
> 
> What if the wakeup never occurs/succeeds?
> Should there be a timeout?

There is no wakeup at all. This piece of code is to solve the preempting race condition in cl_sync_io_end(), where it calls wake_up_all() to wake up the cl_sync_io_wait() process, and then is preempted _inside_ wake_up_all(), and then cl_sync_io_wait() process gains the CPU and frees memory cl_sync_io. Therefore when cl_sync_io_end() comes back to finish its work in wake_up_all(), a piece of freed memory will be accessed.

csi_barrier is proposed to solve this problem, which makes sure wake_up_all() is complete before cl_sync_io_wait() can continue. It should be a really short time so it’s reasonable for cl_sync_io_wait() to do a busy loop wait.

Jinshan

> _______________________________________________
> lustre-devel mailing list
> lustre-devel@...ts.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org