linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 6/6] pgo: Fixup code style issues.
@ 2021-05-28 20:12 Jarmo Tiitto
  2021-05-28 20:37 ` Joe Perches
  2021-05-31 18:18 ` Nathan Chancellor
  0 siblings, 2 replies; 3+ messages in thread
From: Jarmo Tiitto @ 2021-05-28 20:12 UTC (permalink / raw)
  To: samitolvanen
  Cc: wcw, nathan, ndesaulniers, linux-kernel, clang-built-linux, Jarmo Tiitto

Signed-off-by: Jarmo Tiitto <jarmo.tiitto@gmail.com>
---
 kernel/pgo/instrument.c | 106 ++++++++++++++++++++--------------------
 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/kernel/pgo/instrument.c b/kernel/pgo/instrument.c
index a95c86d668b5..b30104411879 100644
--- a/kernel/pgo/instrument.c
+++ b/kernel/pgo/instrument.c
@@ -31,7 +31,7 @@
  * ensures that we don't try to serialize data that's only partially updated.
  */
 static DEFINE_SPINLOCK(pgo_lock);
-static int current_node = 0;
+static int current_node;
 
 unsigned long prf_lock(void)
 {
@@ -55,58 +55,58 @@ void prf_unlock(unsigned long flags)
 static struct llvm_prf_value_node *allocate_node(struct llvm_prf_data *p,
 						 u32 index, u64 value)
 {
-    struct prf_mod_private_data *pmod;
-    struct llvm_prf_data *start = __llvm_prf_data_start;
-    struct llvm_prf_data *end = __llvm_prf_data_end;
-    struct module * mod;
-    struct llvm_prf_value_node * vnds = __llvm_prf_vnds_start;
-    struct llvm_prf_value_node * vnds_end = __llvm_prf_vnds_end;
-
-    if(start <= p && p < end) {
-        /* vmlinux core node */
-        if (&vnds[current_node + 1] >= vnds_end)
-            return NULL; /* Out of nodes */
-
-        current_node++;
-
-        /* Make sure the node is entirely within the section
-         */
-        if (&vnds[current_node] >= vnds_end ||
-            &vnds[current_node + 1] > vnds_end)
-            return NULL;
-
-        return &vnds[current_node];
-
-    } else {
-        /* maybe an module node
-         * find in what module section p points into and
-         * then allocate from that module
-         */
-        rcu_read_lock();
-        list_for_each_entry_rcu(pmod,&prf_mod_list,link) {
-            mod = READ_ONCE(pmod->mod);
-            if(mod) {
-                /* get section bounds */
-                start = mod->prf_data;
-                end = mod->prf_data + mod->prf_data_size;
-                if(start <= p && p < end)
-                {
-                    vnds = mod->prf_vnds;
-                    vnds_end = mod->prf_vnds + mod->prf_vnds_size;
-                    if (&vnds[pmod->current_node + 1] < vnds_end) {
-                        pmod->current_node++;
-
-                        vnds = &vnds[pmod->current_node];
-                        rcu_read_unlock();
-                        return vnds;
-                        break;
-                    }
-                }
-            }
-        }
-        rcu_read_unlock();
-        return NULL; /* Out of nodes */
-    }
+	struct prf_mod_private_data *pmod;
+	struct llvm_prf_data *start = __llvm_prf_data_start;
+	struct llvm_prf_data *end = __llvm_prf_data_end;
+	struct module *mod;
+	struct llvm_prf_value_node *vnds = __llvm_prf_vnds_start;
+	struct llvm_prf_value_node *vnds_end = __llvm_prf_vnds_end;
+
+	if (start <= p && p < end) {
+		/* vmlinux core node */
+		if (&vnds[current_node + 1] >= vnds_end)
+			return NULL; /* Out of nodes */
+
+		current_node++;
+
+		/* Make sure the node is entirely within the section
+		 */
+		if (&vnds[current_node] >= vnds_end ||
+			&vnds[current_node + 1] > vnds_end)
+			return NULL;
+
+		return &vnds[current_node];
+
+	} else {
+		/* maybe an module node
+		 * find in what module section p points into and
+		 * then allocate from that module
+		 */
+		rcu_read_lock();
+		list_for_each_entry_rcu(pmod, &prf_mod_list, link) {
+			mod = READ_ONCE(pmod->mod);
+			if (mod) {
+				/* get section bounds */
+				start = mod->prf_data;
+				end = mod->prf_data + mod->prf_data_size;
+
+				if (start <= p && p < end) {
+					vnds = mod->prf_vnds;
+					vnds_end = mod->prf_vnds + mod->prf_vnds_size;
+
+					if (&vnds[pmod->current_node + 1] < vnds_end) {
+						pmod->current_node++;
+
+						vnds = &vnds[pmod->current_node];
+						rcu_read_unlock();
+						return vnds;
+					}
+				}
+			}
+		}
+		rcu_read_unlock();
+		return NULL; /* Out of nodes */
+	}
 }
 
 /*
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 6/6] pgo: Fixup code style issues.
  2021-05-28 20:12 [PATCH 6/6] pgo: Fixup code style issues Jarmo Tiitto
@ 2021-05-28 20:37 ` Joe Perches
  2021-05-31 18:18 ` Nathan Chancellor
  1 sibling, 0 replies; 3+ messages in thread
From: Joe Perches @ 2021-05-28 20:37 UTC (permalink / raw)
  To: Jarmo Tiitto, samitolvanen
  Cc: wcw, nathan, ndesaulniers, linux-kernel, clang-built-linux

On Fri, 2021-05-28 at 23:12 +0300, Jarmo Tiitto wrote:
> Signed-off-by: Jarmo Tiitto <jarmo.tiitto@gmail.com>

This should have some commit description like:

Do not initialize statics and use tab indentation.
Some would suggest this should be 2 separate patches.

> diff --git a/kernel/pgo/instrument.c b/kernel/pgo/instrument.c
[]
> +	if (start <= p && p < end) {

Generally clearer / more commonly written using

[]
	if (p >= start && p < end)

> +		/* vmlinux core node */
> +		if (&vnds[current_node + 1] >= vnds_end)
> +			return NULL; /* Out of nodes */
> +
> +		current_node++;
> +
> +		/* Make sure the node is entirely within the section
> +		 */
> +		if (&vnds[current_node] >= vnds_end ||
> +			&vnds[current_node + 1] > vnds_end)
> +			return NULL;
> +
> +		return &vnds[current_node];
> +
> +	} else {

I suggest getting ride of the else and unindenting the block below too
in yet another separate patch so the function fits in 80 columns.

> +		/* maybe an module node
> +		 * find in what module section p points into and
> +		 * then allocate from that module
> +		 */

...



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 6/6] pgo: Fixup code style issues.
  2021-05-28 20:12 [PATCH 6/6] pgo: Fixup code style issues Jarmo Tiitto
  2021-05-28 20:37 ` Joe Perches
@ 2021-05-31 18:18 ` Nathan Chancellor
  1 sibling, 0 replies; 3+ messages in thread
From: Nathan Chancellor @ 2021-05-31 18:18 UTC (permalink / raw)
  To: Jarmo Tiitto
  Cc: samitolvanen, wcw, ndesaulniers, linux-kernel, clang-built-linux

On Fri, May 28, 2021 at 11:12:13PM +0300, Jarmo Tiitto wrote:
> Signed-off-by: Jarmo Tiitto <jarmo.tiitto@gmail.com>

This should be squashed into 4/6. Do not introduce issues in the middle
of a patch series. A couple more comments below.

> ---
>  kernel/pgo/instrument.c | 106 ++++++++++++++++++++--------------------
>  1 file changed, 53 insertions(+), 53 deletions(-)
> 
> diff --git a/kernel/pgo/instrument.c b/kernel/pgo/instrument.c
> index a95c86d668b5..b30104411879 100644
> --- a/kernel/pgo/instrument.c
> +++ b/kernel/pgo/instrument.c
> @@ -31,7 +31,7 @@
>   * ensures that we don't try to serialize data that's only partially updated.
>   */
>  static DEFINE_SPINLOCK(pgo_lock);
> -static int current_node = 0;
> +static int current_node;
>  
>  unsigned long prf_lock(void)
>  {
> @@ -55,58 +55,58 @@ void prf_unlock(unsigned long flags)
>  static struct llvm_prf_value_node *allocate_node(struct llvm_prf_data *p,
>  						 u32 index, u64 value)
>  {
> -    struct prf_mod_private_data *pmod;
> -    struct llvm_prf_data *start = __llvm_prf_data_start;
> -    struct llvm_prf_data *end = __llvm_prf_data_end;
> -    struct module * mod;
> -    struct llvm_prf_value_node * vnds = __llvm_prf_vnds_start;
> -    struct llvm_prf_value_node * vnds_end = __llvm_prf_vnds_end;
> -
> -    if(start <= p && p < end) {
> -        /* vmlinux core node */
> -        if (&vnds[current_node + 1] >= vnds_end)
> -            return NULL; /* Out of nodes */
> -
> -        current_node++;
> -
> -        /* Make sure the node is entirely within the section
> -         */
> -        if (&vnds[current_node] >= vnds_end ||
> -            &vnds[current_node + 1] > vnds_end)
> -            return NULL;
> -
> -        return &vnds[current_node];
> -
> -    } else {
> -        /* maybe an module node
> -         * find in what module section p points into and
> -         * then allocate from that module
> -         */
> -        rcu_read_lock();
> -        list_for_each_entry_rcu(pmod,&prf_mod_list,link) {
> -            mod = READ_ONCE(pmod->mod);
> -            if(mod) {
> -                /* get section bounds */
> -                start = mod->prf_data;
> -                end = mod->prf_data + mod->prf_data_size;
> -                if(start <= p && p < end)
> -                {
> -                    vnds = mod->prf_vnds;
> -                    vnds_end = mod->prf_vnds + mod->prf_vnds_size;
> -                    if (&vnds[pmod->current_node + 1] < vnds_end) {
> -                        pmod->current_node++;
> -
> -                        vnds = &vnds[pmod->current_node];
> -                        rcu_read_unlock();
> -                        return vnds;
> -                        break;
> -                    }
> -                }
> -            }
> -        }
> -        rcu_read_unlock();
> -        return NULL; /* Out of nodes */
> -    }
> +	struct prf_mod_private_data *pmod;
> +	struct llvm_prf_data *start = __llvm_prf_data_start;
> +	struct llvm_prf_data *end = __llvm_prf_data_end;
> +	struct module *mod;
> +	struct llvm_prf_value_node *vnds = __llvm_prf_vnds_start;
> +	struct llvm_prf_value_node *vnds_end = __llvm_prf_vnds_end;
> +
> +	if (start <= p && p < end) {
> +		/* vmlinux core node */
> +		if (&vnds[current_node + 1] >= vnds_end)
> +			return NULL; /* Out of nodes */
> +
> +		current_node++;
> +
> +		/* Make sure the node is entirely within the section
> +		 */

Move the '*/' to the end of the previous line.

> +		if (&vnds[current_node] >= vnds_end ||
> +			&vnds[current_node + 1] > vnds_end)
> +			return NULL;
> +
> +		return &vnds[current_node];
> +

Remove this blank line.

> +	} else {
> +		/* maybe an module node
> +		 * find in what module section p points into and
> +		 * then allocate from that module
> +		 */
> +		rcu_read_lock();
> +		list_for_each_entry_rcu(pmod, &prf_mod_list, link) {
> +			mod = READ_ONCE(pmod->mod);
> +			if (mod) {
> +				/* get section bounds */
> +				start = mod->prf_data;
> +				end = mod->prf_data + mod->prf_data_size;
> +
> +				if (start <= p && p < end) {
> +					vnds = mod->prf_vnds;
> +					vnds_end = mod->prf_vnds + mod->prf_vnds_size;
> +
> +					if (&vnds[pmod->current_node + 1] < vnds_end) {
> +						pmod->current_node++;
> +
> +						vnds = &vnds[pmod->current_node];
> +						rcu_read_unlock();
> +						return vnds;
> +					}
> +				}
> +			}
> +		}
> +		rcu_read_unlock();
> +		return NULL; /* Out of nodes */
> +	}
>  }
>  
>  /*
> -- 
> 2.31.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-31 18:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-28 20:12 [PATCH 6/6] pgo: Fixup code style issues Jarmo Tiitto
2021-05-28 20:37 ` Joe Perches
2021-05-31 18:18 ` Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).