Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1543494 > unrolled thread

[PATCH] staging : lustre : Remove braces from single-line body.

Back to article view | Back to linux.kernel

Contents

  [PATCH] staging : lustre : Remove braces from single-line body. Tabrez khan <khan.tabrez21@gmail.com> - 2016-12-16 15:40 +0100
    Re: [PATCH] staging : lustre : Remove braces from single-line body. Joe Perches <joe@perches.com> - 2016-12-16 17:10 +0100
      Re: [lustre-devel] [PATCH] staging : lustre : Remove braces from  single-line body. "Xiong, Jinshan" <jinshan.xiong@intel.com> - 2016-12-16 18:30 +0100

#1543494 — [PATCH] staging : lustre : Remove braces from single-line body.

Remove unnecessary braces {} for single while statement.
This warning is found using checkpatch.pl.

Signed-off-by: Tabrez khan <khan.tabrez21@gmail.com>
---
 drivers/staging/lustre/lustre/obdclass/cl_io.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/cl_io.c b/drivers/staging/lustre/lustre/obdclass/cl_io.c
index bc4b7b6..2daede8 100644
--- 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();
-	}
+
 
 	return rc;
 }
-- 
2.7.4

[toc] | [next] | [standalone]

#1543556

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?

[toc] | [prev] | [next] | [standalone]

#1543611 — Re: [lustre-devel] [PATCH] staging : lustre : Remove braces from single-line body.

> On Dec 16, 2016, at 8:05 AM, Joe Perches <joe@perches.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@lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web