[PATCH trivial] include/linux/gfp.h: Improve the coding styles

[PATCH trivial] include/linux/gfp.h: Improve the coding styles

[chengang]

From: Chen Gang <chengang@emindsoft.com.cn>

Always notice about 80 columns, and the white space near '|'.

Let the wrapped function parameters align as the same styles.

Remove redundant statement "enum zone_type z;" in function gfp_zone.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 include/linux/gfp.h | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 36e0c5e..cf904ef 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -53,8 +53,10 @@ struct vm_area_struct;
 #define __GFP_DMA	((__force gfp_t)___GFP_DMA)
 #define __GFP_HIGHMEM	((__force gfp_t)___GFP_HIGHMEM)
 #define __GFP_DMA32	((__force gfp_t)___GFP_DMA32)
-#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
-#define GFP_ZONEMASK	(__GFP_DMA|__GFP_HIGHMEM|__GFP_DMA32|__GFP_MOVABLE)
+#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE) \
+						/* ZONE_MOVABLE allowed */
+#define GFP_ZONEMASK	(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32 | \
+			 __GFP_MOVABLE)
 
 /*
  * Page mobility and placement hints
@@ -151,9 +153,12 @@ struct vm_area_struct;
  */
 #define __GFP_IO	((__force gfp_t)___GFP_IO)
 #define __GFP_FS	((__force gfp_t)___GFP_FS)
-#define __GFP_DIRECT_RECLAIM	((__force gfp_t)___GFP_DIRECT_RECLAIM) /* Caller can reclaim */
-#define __GFP_KSWAPD_RECLAIM	((__force gfp_t)___GFP_KSWAPD_RECLAIM) /* kswapd can wake */
-#define __GFP_RECLAIM ((__force gfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM))
+#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) \
+							/* Caller can reclaim */
+#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) \
+							/* kswapd can wake */
+#define __GFP_RECLAIM	((__force gfp_t)(___GFP_DIRECT_RECLAIM | \
+			 ___GFP_KSWAPD_RECLAIM))
 #define __GFP_REPEAT	((__force gfp_t)___GFP_REPEAT)
 #define __GFP_NOFAIL	((__force gfp_t)___GFP_NOFAIL)
 #define __GFP_NORETRY	((__force gfp_t)___GFP_NORETRY)
@@ -262,7 +267,7 @@ struct vm_area_struct;
 			 ~__GFP_KSWAPD_RECLAIM)
 
 /* Convert GFP flags to their corresponding migrate type */
-#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE)
+#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE | __GFP_MOVABLE)
 #define GFP_MOVABLE_SHIFT 3
 
 static inline int gfpflags_to_migratetype(const gfp_t gfp_flags)
@@ -377,11 +382,10 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
 
 static inline enum zone_type gfp_zone(gfp_t flags)
 {
-	enum zone_type z;
 	int bit = (__force int) (flags & GFP_ZONEMASK);
+	enum zone_type z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
+			    ((1 << GFP_ZONES_SHIFT) - 1);
 
-	z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
-					 ((1 << GFP_ZONES_SHIFT) - 1);
 	VM_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
 	return z;
 }
@@ -428,8 +432,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 		       struct zonelist *zonelist, nodemask_t *nodemask);
 
 static inline struct page *
-__alloc_pages(gfp_t gfp_mask, unsigned int order,
-		struct zonelist *zonelist)
+__alloc_pages(gfp_t gfp_mask, unsigned int order, struct zonelist *zonelist)
 {
 	return __alloc_pages_nodemask(gfp_mask, order, zonelist, NULL);
 }
@@ -453,7 +456,7 @@ __alloc_pages_node(int nid, gfp_t gfp_mask, unsigned int order)
  * online.
  */
 static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
-						unsigned int order)
+					    unsigned int order)
 {
 	if (nid == NUMA_NO_NODE)
 		nid = numa_mem_id();
@@ -470,8 +473,9 @@ alloc_pages(gfp_t gfp_mask, unsigned int order)
 	return alloc_pages_current(gfp_mask, order);
 }
 extern struct page *alloc_pages_vma(gfp_t gfp_mask, int order,
-			struct vm_area_struct *vma, unsigned long addr,
-			int node, bool hugepage);
+				    struct vm_area_struct *vma,
+				    unsigned long addr, int node,
+				    bool hugepage);
 #define alloc_hugepage_vma(gfp_mask, vma, addr, order)	\
 	alloc_pages_vma(gfp_mask, order, vma, addr, numa_node_id(), true)
 #else
@@ -552,7 +556,8 @@ static inline bool pm_suspended_storage(void)
 }
 #endif /* CONFIG_PM_SLEEP */
 
-#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA)
+#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
+     defined(CONFIG_CMA)
 /* The below functions must be run on a range from a single zone. */
 extern int alloc_contig_range(unsigned long start, unsigned long end,
 			      unsigned migratetype);
-- 
1.9.3

[PATCH trivial] include/linux/gfp.h: Improve the coding styles

[chengang]

From: Chen Gang <chengang@emindsoft.com.cn>

Always notice about 80 columns, and the white space near '|'.

Let the wrapped function parameters align as the same styles.

Remove redundant statement "enum zone_type z;" in function gfp_zone.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 include/linux/gfp.h | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 36e0c5e..cf904ef 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -53,8 +53,10 @@ struct vm_area_struct;
 #define __GFP_DMA	((__force gfp_t)___GFP_DMA)
 #define __GFP_HIGHMEM	((__force gfp_t)___GFP_HIGHMEM)
 #define __GFP_DMA32	((__force gfp_t)___GFP_DMA32)
-#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
-#define GFP_ZONEMASK	(__GFP_DMA|__GFP_HIGHMEM|__GFP_DMA32|__GFP_MOVABLE)
+#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE) \
+						/* ZONE_MOVABLE allowed */
+#define GFP_ZONEMASK	(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32 | \
+			 __GFP_MOVABLE)
 
 /*
  * Page mobility and placement hints
@@ -151,9 +153,12 @@ struct vm_area_struct;
  */
 #define __GFP_IO	((__force gfp_t)___GFP_IO)
 #define __GFP_FS	((__force gfp_t)___GFP_FS)
-#define __GFP_DIRECT_RECLAIM	((__force gfp_t)___GFP_DIRECT_RECLAIM) /* Caller can reclaim */
-#define __GFP_KSWAPD_RECLAIM	((__force gfp_t)___GFP_KSWAPD_RECLAIM) /* kswapd can wake */
-#define __GFP_RECLAIM ((__force gfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM))
+#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) \
+							/* Caller can reclaim */
+#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) \
+							/* kswapd can wake */
+#define __GFP_RECLAIM	((__force gfp_t)(___GFP_DIRECT_RECLAIM | \
+			 ___GFP_KSWAPD_RECLAIM))
 #define __GFP_REPEAT	((__force gfp_t)___GFP_REPEAT)
 #define __GFP_NOFAIL	((__force gfp_t)___GFP_NOFAIL)
 #define __GFP_NORETRY	((__force gfp_t)___GFP_NORETRY)
@@ -262,7 +267,7 @@ struct vm_area_struct;
 			 ~__GFP_KSWAPD_RECLAIM)
 
 /* Convert GFP flags to their corresponding migrate type */
-#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE)
+#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE | __GFP_MOVABLE)
 #define GFP_MOVABLE_SHIFT 3
 
 static inline int gfpflags_to_migratetype(const gfp_t gfp_flags)
@@ -377,11 +382,10 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
 
 static inline enum zone_type gfp_zone(gfp_t flags)
 {
-	enum zone_type z;
 	int bit = (__force int) (flags & GFP_ZONEMASK);
+	enum zone_type z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
+			    ((1 << GFP_ZONES_SHIFT) - 1);
 
-	z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
-					 ((1 << GFP_ZONES_SHIFT) - 1);
 	VM_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
 	return z;
 }
@@ -428,8 +432,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
 		       struct zonelist *zonelist, nodemask_t *nodemask);
 
 static inline struct page *
-__alloc_pages(gfp_t gfp_mask, unsigned int order,
-		struct zonelist *zonelist)
+__alloc_pages(gfp_t gfp_mask, unsigned int order, struct zonelist *zonelist)
 {
 	return __alloc_pages_nodemask(gfp_mask, order, zonelist, NULL);
 }
@@ -453,7 +456,7 @@ __alloc_pages_node(int nid, gfp_t gfp_mask, unsigned int order)
  * online.
  */
 static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
-						unsigned int order)
+					    unsigned int order)
 {
 	if (nid == NUMA_NO_NODE)
 		nid = numa_mem_id();
@@ -470,8 +473,9 @@ alloc_pages(gfp_t gfp_mask, unsigned int order)
 	return alloc_pages_current(gfp_mask, order);
 }
 extern struct page *alloc_pages_vma(gfp_t gfp_mask, int order,
-			struct vm_area_struct *vma, unsigned long addr,
-			int node, bool hugepage);
+				    struct vm_area_struct *vma,
+				    unsigned long addr, int node,
+				    bool hugepage);
 #define alloc_hugepage_vma(gfp_mask, vma, addr, order)	\
 	alloc_pages_vma(gfp_mask, order, vma, addr, numa_node_id(), true)
 #else
@@ -552,7 +556,8 @@ static inline bool pm_suspended_storage(void)
 }
 #endif /* CONFIG_PM_SLEEP */
 
-#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA)
+#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
+     defined(CONFIG_CMA)
 /* The below functions must be run on a range from a single zone. */
 extern int alloc_contig_range(unsigned long start, unsigned long end,
 			      unsigned migratetype);
-- 
1.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[SeongJae Park]

Hello Chen,


On Thu, 25 Feb 2016, chengang@emindsoft.com.cn wrote:

> From: Chen Gang <chengang@emindsoft.com.cn>
>
> Always notice about 80 columns, and the white space near '|'.
>
> Let the wrapped function parameters align as the same styles.
>
> Remove redundant statement "enum zone_type z;" in function gfp_zone.
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> include/linux/gfp.h | 35 ++++++++++++++++++++---------------
> 1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 36e0c5e..cf904ef 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -53,8 +53,10 @@ struct vm_area_struct;
> #define __GFP_DMA	((__force gfp_t)___GFP_DMA)
> #define __GFP_HIGHMEM	((__force gfp_t)___GFP_HIGHMEM)
> #define __GFP_DMA32	((__force gfp_t)___GFP_DMA32)
> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
> -#define GFP_ZONEMASK	(__GFP_DMA|__GFP_HIGHMEM|__GFP_DMA32|__GFP_MOVABLE)
> +#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE) \
> +						/* ZONE_MOVABLE allowed */

Well, the indentation for the comment and the '\' looks odd to me.  If
the 80 column limit is necessary, how about moving the comment to above
line of the macro as below?  Because comments are usually placed before
the target they are explaining, I believe this may better to read.

  -#define __GFP_MOVABLE        ((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
  +/* ZONE_MOVABLE allowed */
  +#define __GFP_MOVABLE        ((__force gfp_t)___GFP_MOVABLE)

Maybe the opinion can be applied to below similar changes, too.


Thanks,
SeongJae Park.

> +#define GFP_ZONEMASK	(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32 | \
> +			 __GFP_MOVABLE)
>
> /*
>  * Page mobility and placement hints
> @@ -151,9 +153,12 @@ struct vm_area_struct;
>  */
> #define __GFP_IO	((__force gfp_t)___GFP_IO)
> #define __GFP_FS	((__force gfp_t)___GFP_FS)
> -#define __GFP_DIRECT_RECLAIM	((__force gfp_t)___GFP_DIRECT_RECLAIM) /* Caller can reclaim */
> -#define __GFP_KSWAPD_RECLAIM	((__force gfp_t)___GFP_KSWAPD_RECLAIM) /* kswapd can wake */
> -#define __GFP_RECLAIM ((__force gfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM))
> +#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) \
> +							/* Caller can reclaim */
> +#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) \
> +							/* kswapd can wake */
> +#define __GFP_RECLAIM	((__force gfp_t)(___GFP_DIRECT_RECLAIM | \
> +			 ___GFP_KSWAPD_RECLAIM))
> #define __GFP_REPEAT	((__force gfp_t)___GFP_REPEAT)
> #define __GFP_NOFAIL	((__force gfp_t)___GFP_NOFAIL)
> #define __GFP_NORETRY	((__force gfp_t)___GFP_NORETRY)
> @@ -262,7 +267,7 @@ struct vm_area_struct;
> 			 ~__GFP_KSWAPD_RECLAIM)
>
> /* Convert GFP flags to their corresponding migrate type */
> -#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE)
> +#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE | __GFP_MOVABLE)
> #define GFP_MOVABLE_SHIFT 3
>
> static inline int gfpflags_to_migratetype(const gfp_t gfp_flags)
> @@ -377,11 +382,10 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
>
> static inline enum zone_type gfp_zone(gfp_t flags)
> {
> -	enum zone_type z;
> 	int bit = (__force int) (flags & GFP_ZONEMASK);
> +	enum zone_type z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
> +			    ((1 << GFP_ZONES_SHIFT) - 1);
>
> -	z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
> -					 ((1 << GFP_ZONES_SHIFT) - 1);
> 	VM_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> 	return z;
> }
> @@ -428,8 +432,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
> 		       struct zonelist *zonelist, nodemask_t *nodemask);
>
> static inline struct page *
> -__alloc_pages(gfp_t gfp_mask, unsigned int order,
> -		struct zonelist *zonelist)
> +__alloc_pages(gfp_t gfp_mask, unsigned int order, struct zonelist *zonelist)
> {
> 	return __alloc_pages_nodemask(gfp_mask, order, zonelist, NULL);
> }
> @@ -453,7 +456,7 @@ __alloc_pages_node(int nid, gfp_t gfp_mask, unsigned int order)
>  * online.
>  */
> static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
> -						unsigned int order)
> +					    unsigned int order)
> {
> 	if (nid == NUMA_NO_NODE)
> 		nid = numa_mem_id();
> @@ -470,8 +473,9 @@ alloc_pages(gfp_t gfp_mask, unsigned int order)
> 	return alloc_pages_current(gfp_mask, order);
> }
> extern struct page *alloc_pages_vma(gfp_t gfp_mask, int order,
> -			struct vm_area_struct *vma, unsigned long addr,
> -			int node, bool hugepage);
> +				    struct vm_area_struct *vma,
> +				    unsigned long addr, int node,
> +				    bool hugepage);
> #define alloc_hugepage_vma(gfp_mask, vma, addr, order)	\
> 	alloc_pages_vma(gfp_mask, order, vma, addr, numa_node_id(), true)
> #else
> @@ -552,7 +556,8 @@ static inline bool pm_suspended_storage(void)
> }
> #endif /* CONFIG_PM_SLEEP */
>
> -#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA)
> +#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
> +     defined(CONFIG_CMA)
> /* The below functions must be run on a range from a single zone. */
> extern int alloc_contig_range(unsigned long start, unsigned long end,
> 			      unsigned migratetype);
> -- 
> 1.9.3
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[SeongJae Park]

Hello Chen,


On Thu, 25 Feb 2016, chengang@emindsoft.com.cn wrote:

> From: Chen Gang <chengang@emindsoft.com.cn>
>
> Always notice about 80 columns, and the white space near '|'.
>
> Let the wrapped function parameters align as the same styles.
>
> Remove redundant statement "enum zone_type z;" in function gfp_zone.
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> include/linux/gfp.h | 35 ++++++++++++++++++++---------------
> 1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 36e0c5e..cf904ef 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -53,8 +53,10 @@ struct vm_area_struct;
> #define __GFP_DMA	((__force gfp_t)___GFP_DMA)
> #define __GFP_HIGHMEM	((__force gfp_t)___GFP_HIGHMEM)
> #define __GFP_DMA32	((__force gfp_t)___GFP_DMA32)
> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
> -#define GFP_ZONEMASK	(__GFP_DMA|__GFP_HIGHMEM|__GFP_DMA32|__GFP_MOVABLE)
> +#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE) \
> +						/* ZONE_MOVABLE allowed */

Well, the indentation for the comment and the '\' looks odd to me.  If
the 80 column limit is necessary, how about moving the comment to above
line of the macro as below?  Because comments are usually placed before
the target they are explaining, I believe this may better to read.

  -#define __GFP_MOVABLE        ((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
  +/* ZONE_MOVABLE allowed */
  +#define __GFP_MOVABLE        ((__force gfp_t)___GFP_MOVABLE)

Maybe the opinion can be applied to below similar changes, too.


Thanks,
SeongJae Park.

> +#define GFP_ZONEMASK	(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32 | \
> +			 __GFP_MOVABLE)
>
> /*
>  * Page mobility and placement hints
> @@ -151,9 +153,12 @@ struct vm_area_struct;
>  */
> #define __GFP_IO	((__force gfp_t)___GFP_IO)
> #define __GFP_FS	((__force gfp_t)___GFP_FS)
> -#define __GFP_DIRECT_RECLAIM	((__force gfp_t)___GFP_DIRECT_RECLAIM) /* Caller can reclaim */
> -#define __GFP_KSWAPD_RECLAIM	((__force gfp_t)___GFP_KSWAPD_RECLAIM) /* kswapd can wake */
> -#define __GFP_RECLAIM ((__force gfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM))
> +#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) \
> +							/* Caller can reclaim */
> +#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) \
> +							/* kswapd can wake */
> +#define __GFP_RECLAIM	((__force gfp_t)(___GFP_DIRECT_RECLAIM | \
> +			 ___GFP_KSWAPD_RECLAIM))
> #define __GFP_REPEAT	((__force gfp_t)___GFP_REPEAT)
> #define __GFP_NOFAIL	((__force gfp_t)___GFP_NOFAIL)
> #define __GFP_NORETRY	((__force gfp_t)___GFP_NORETRY)
> @@ -262,7 +267,7 @@ struct vm_area_struct;
> 			 ~__GFP_KSWAPD_RECLAIM)
>
> /* Convert GFP flags to their corresponding migrate type */
> -#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE)
> +#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE | __GFP_MOVABLE)
> #define GFP_MOVABLE_SHIFT 3
>
> static inline int gfpflags_to_migratetype(const gfp_t gfp_flags)
> @@ -377,11 +382,10 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
>
> static inline enum zone_type gfp_zone(gfp_t flags)
> {
> -	enum zone_type z;
> 	int bit = (__force int) (flags & GFP_ZONEMASK);
> +	enum zone_type z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
> +			    ((1 << GFP_ZONES_SHIFT) - 1);
>
> -	z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
> -					 ((1 << GFP_ZONES_SHIFT) - 1);
> 	VM_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> 	return z;
> }
> @@ -428,8 +432,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
> 		       struct zonelist *zonelist, nodemask_t *nodemask);
>
> static inline struct page *
> -__alloc_pages(gfp_t gfp_mask, unsigned int order,
> -		struct zonelist *zonelist)
> +__alloc_pages(gfp_t gfp_mask, unsigned int order, struct zonelist *zonelist)
> {
> 	return __alloc_pages_nodemask(gfp_mask, order, zonelist, NULL);
> }
> @@ -453,7 +456,7 @@ __alloc_pages_node(int nid, gfp_t gfp_mask, unsigned int order)
>  * online.
>  */
> static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
> -						unsigned int order)
> +					    unsigned int order)
> {
> 	if (nid == NUMA_NO_NODE)
> 		nid = numa_mem_id();
> @@ -470,8 +473,9 @@ alloc_pages(gfp_t gfp_mask, unsigned int order)
> 	return alloc_pages_current(gfp_mask, order);
> }
> extern struct page *alloc_pages_vma(gfp_t gfp_mask, int order,
> -			struct vm_area_struct *vma, unsigned long addr,
> -			int node, bool hugepage);
> +				    struct vm_area_struct *vma,
> +				    unsigned long addr, int node,
> +				    bool hugepage);
> #define alloc_hugepage_vma(gfp_mask, vma, addr, order)	\
> 	alloc_pages_vma(gfp_mask, order, vma, addr, numa_node_id(), true)
> #else
> @@ -552,7 +556,8 @@ static inline bool pm_suspended_storage(void)
> }
> #endif /* CONFIG_PM_SLEEP */
>
> -#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA)
> +#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
> +     defined(CONFIG_CMA)
> /* The below functions must be run on a range from a single zone. */
> extern int alloc_contig_range(unsigned long start, unsigned long end,
> 			      unsigned migratetype);
> -- 
> 1.9.3
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Michal Hocko]

On Thu 25-02-16 06:26:31, chengang@emindsoft.com.cn wrote:
> From: Chen Gang <chengang@emindsoft.com.cn>
> 
> Always notice about 80 columns, and the white space near '|'.
> 
> Let the wrapped function parameters align as the same styles.
> 
> Remove redundant statement "enum zone_type z;" in function gfp_zone.

I do not think this is an improvement. The comment placement is just odd
and artificially splitting the mask into more lines makes git grep
harder to use.

> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  include/linux/gfp.h | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 36e0c5e..cf904ef 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -53,8 +53,10 @@ struct vm_area_struct;
>  #define __GFP_DMA	((__force gfp_t)___GFP_DMA)
>  #define __GFP_HIGHMEM	((__force gfp_t)___GFP_HIGHMEM)
>  #define __GFP_DMA32	((__force gfp_t)___GFP_DMA32)
> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
> -#define GFP_ZONEMASK	(__GFP_DMA|__GFP_HIGHMEM|__GFP_DMA32|__GFP_MOVABLE)
> +#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE) \
> +						/* ZONE_MOVABLE allowed */
> +#define GFP_ZONEMASK	(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32 | \
> +			 __GFP_MOVABLE)
>  
>  /*
>   * Page mobility and placement hints
> @@ -151,9 +153,12 @@ struct vm_area_struct;
>   */
>  #define __GFP_IO	((__force gfp_t)___GFP_IO)
>  #define __GFP_FS	((__force gfp_t)___GFP_FS)
> -#define __GFP_DIRECT_RECLAIM	((__force gfp_t)___GFP_DIRECT_RECLAIM) /* Caller can reclaim */
> -#define __GFP_KSWAPD_RECLAIM	((__force gfp_t)___GFP_KSWAPD_RECLAIM) /* kswapd can wake */
> -#define __GFP_RECLAIM ((__force gfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM))
> +#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) \
> +							/* Caller can reclaim */
> +#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) \
> +							/* kswapd can wake */
> +#define __GFP_RECLAIM	((__force gfp_t)(___GFP_DIRECT_RECLAIM | \
> +			 ___GFP_KSWAPD_RECLAIM))
>  #define __GFP_REPEAT	((__force gfp_t)___GFP_REPEAT)
>  #define __GFP_NOFAIL	((__force gfp_t)___GFP_NOFAIL)
>  #define __GFP_NORETRY	((__force gfp_t)___GFP_NORETRY)
> @@ -262,7 +267,7 @@ struct vm_area_struct;
>  			 ~__GFP_KSWAPD_RECLAIM)
>  
>  /* Convert GFP flags to their corresponding migrate type */
> -#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE)
> +#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE | __GFP_MOVABLE)
>  #define GFP_MOVABLE_SHIFT 3
>  
>  static inline int gfpflags_to_migratetype(const gfp_t gfp_flags)
> @@ -377,11 +382,10 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
>  
>  static inline enum zone_type gfp_zone(gfp_t flags)
>  {
> -	enum zone_type z;
>  	int bit = (__force int) (flags & GFP_ZONEMASK);
> +	enum zone_type z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
> +			    ((1 << GFP_ZONES_SHIFT) - 1);
>  
> -	z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
> -					 ((1 << GFP_ZONES_SHIFT) - 1);
>  	VM_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
>  	return z;
>  }
> @@ -428,8 +432,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
>  		       struct zonelist *zonelist, nodemask_t *nodemask);
>  
>  static inline struct page *
> -__alloc_pages(gfp_t gfp_mask, unsigned int order,
> -		struct zonelist *zonelist)
> +__alloc_pages(gfp_t gfp_mask, unsigned int order, struct zonelist *zonelist)
>  {
>  	return __alloc_pages_nodemask(gfp_mask, order, zonelist, NULL);
>  }
> @@ -453,7 +456,7 @@ __alloc_pages_node(int nid, gfp_t gfp_mask, unsigned int order)
>   * online.
>   */
>  static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
> -						unsigned int order)
> +					    unsigned int order)
>  {
>  	if (nid == NUMA_NO_NODE)
>  		nid = numa_mem_id();
> @@ -470,8 +473,9 @@ alloc_pages(gfp_t gfp_mask, unsigned int order)
>  	return alloc_pages_current(gfp_mask, order);
>  }
>  extern struct page *alloc_pages_vma(gfp_t gfp_mask, int order,
> -			struct vm_area_struct *vma, unsigned long addr,
> -			int node, bool hugepage);
> +				    struct vm_area_struct *vma,
> +				    unsigned long addr, int node,
> +				    bool hugepage);
>  #define alloc_hugepage_vma(gfp_mask, vma, addr, order)	\
>  	alloc_pages_vma(gfp_mask, order, vma, addr, numa_node_id(), true)
>  #else
> @@ -552,7 +556,8 @@ static inline bool pm_suspended_storage(void)
>  }
>  #endif /* CONFIG_PM_SLEEP */
>  
> -#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA)
> +#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
> +     defined(CONFIG_CMA)
>  /* The below functions must be run on a range from a single zone. */
>  extern int alloc_contig_range(unsigned long start, unsigned long end,
>  			      unsigned migratetype);
> -- 
> 1.9.3

-- 
Michal Hocko
SUSE Labs

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Michal Hocko]

On Thu 25-02-16 06:26:31, chengang@emindsoft.com.cn wrote:
> From: Chen Gang <chengang@emindsoft.com.cn>
> 
> Always notice about 80 columns, and the white space near '|'.
> 
> Let the wrapped function parameters align as the same styles.
> 
> Remove redundant statement "enum zone_type z;" in function gfp_zone.

I do not think this is an improvement. The comment placement is just odd
and artificially splitting the mask into more lines makes git grep
harder to use.

> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  include/linux/gfp.h | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 36e0c5e..cf904ef 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -53,8 +53,10 @@ struct vm_area_struct;
>  #define __GFP_DMA	((__force gfp_t)___GFP_DMA)
>  #define __GFP_HIGHMEM	((__force gfp_t)___GFP_HIGHMEM)
>  #define __GFP_DMA32	((__force gfp_t)___GFP_DMA32)
> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
> -#define GFP_ZONEMASK	(__GFP_DMA|__GFP_HIGHMEM|__GFP_DMA32|__GFP_MOVABLE)
> +#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE) \
> +						/* ZONE_MOVABLE allowed */
> +#define GFP_ZONEMASK	(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32 | \
> +			 __GFP_MOVABLE)
>  
>  /*
>   * Page mobility and placement hints
> @@ -151,9 +153,12 @@ struct vm_area_struct;
>   */
>  #define __GFP_IO	((__force gfp_t)___GFP_IO)
>  #define __GFP_FS	((__force gfp_t)___GFP_FS)
> -#define __GFP_DIRECT_RECLAIM	((__force gfp_t)___GFP_DIRECT_RECLAIM) /* Caller can reclaim */
> -#define __GFP_KSWAPD_RECLAIM	((__force gfp_t)___GFP_KSWAPD_RECLAIM) /* kswapd can wake */
> -#define __GFP_RECLAIM ((__force gfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM))
> +#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) \
> +							/* Caller can reclaim */
> +#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) \
> +							/* kswapd can wake */
> +#define __GFP_RECLAIM	((__force gfp_t)(___GFP_DIRECT_RECLAIM | \
> +			 ___GFP_KSWAPD_RECLAIM))
>  #define __GFP_REPEAT	((__force gfp_t)___GFP_REPEAT)
>  #define __GFP_NOFAIL	((__force gfp_t)___GFP_NOFAIL)
>  #define __GFP_NORETRY	((__force gfp_t)___GFP_NORETRY)
> @@ -262,7 +267,7 @@ struct vm_area_struct;
>  			 ~__GFP_KSWAPD_RECLAIM)
>  
>  /* Convert GFP flags to their corresponding migrate type */
> -#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE)
> +#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE | __GFP_MOVABLE)
>  #define GFP_MOVABLE_SHIFT 3
>  
>  static inline int gfpflags_to_migratetype(const gfp_t gfp_flags)
> @@ -377,11 +382,10 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
>  
>  static inline enum zone_type gfp_zone(gfp_t flags)
>  {
> -	enum zone_type z;
>  	int bit = (__force int) (flags & GFP_ZONEMASK);
> +	enum zone_type z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
> +			    ((1 << GFP_ZONES_SHIFT) - 1);
>  
> -	z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) &
> -					 ((1 << GFP_ZONES_SHIFT) - 1);
>  	VM_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
>  	return z;
>  }
> @@ -428,8 +432,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
>  		       struct zonelist *zonelist, nodemask_t *nodemask);
>  
>  static inline struct page *
> -__alloc_pages(gfp_t gfp_mask, unsigned int order,
> -		struct zonelist *zonelist)
> +__alloc_pages(gfp_t gfp_mask, unsigned int order, struct zonelist *zonelist)
>  {
>  	return __alloc_pages_nodemask(gfp_mask, order, zonelist, NULL);
>  }
> @@ -453,7 +456,7 @@ __alloc_pages_node(int nid, gfp_t gfp_mask, unsigned int order)
>   * online.
>   */
>  static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
> -						unsigned int order)
> +					    unsigned int order)
>  {
>  	if (nid == NUMA_NO_NODE)
>  		nid = numa_mem_id();
> @@ -470,8 +473,9 @@ alloc_pages(gfp_t gfp_mask, unsigned int order)
>  	return alloc_pages_current(gfp_mask, order);
>  }
>  extern struct page *alloc_pages_vma(gfp_t gfp_mask, int order,
> -			struct vm_area_struct *vma, unsigned long addr,
> -			int node, bool hugepage);
> +				    struct vm_area_struct *vma,
> +				    unsigned long addr, int node,
> +				    bool hugepage);
>  #define alloc_hugepage_vma(gfp_mask, vma, addr, order)	\
>  	alloc_pages_vma(gfp_mask, order, vma, addr, numa_node_id(), true)
>  #else
> @@ -552,7 +556,8 @@ static inline bool pm_suspended_storage(void)
>  }
>  #endif /* CONFIG_PM_SLEEP */
>  
> -#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA)
> +#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
> +     defined(CONFIG_CMA)
>  /* The below functions must be run on a range from a single zone. */
>  extern int alloc_contig_range(unsigned long start, unsigned long end,
>  			      unsigned migratetype);
> -- 
> 1.9.3

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Mel Gorman]

On Thu, Feb 25, 2016 at 06:26:31AM +0800, chengang@emindsoft.com.cn wrote:
> From: Chen Gang <chengang@emindsoft.com.cn>
> 
> Always notice about 80 columns, and the white space near '|'.
> 
> Let the wrapped function parameters align as the same styles.
> 
> Remove redundant statement "enum zone_type z;" in function gfp_zone.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>

NAK from me at least. From my perspective, it's preferrable to preserve
blame than go through a layer of cleanup when looking for the commit
that defined particular flags. It's ok to cleanup code at the same time
definitions change for functional or performance reasons.

-- 
Mel Gorman
SUSE Labs

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Mel Gorman]

On Thu, Feb 25, 2016 at 06:26:31AM +0800, chengang@emindsoft.com.cn wrote:
> From: Chen Gang <chengang@emindsoft.com.cn>
> 
> Always notice about 80 columns, and the white space near '|'.
> 
> Let the wrapped function parameters align as the same styles.
> 
> Remove redundant statement "enum zone_type z;" in function gfp_zone.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>

NAK from me at least. From my perspective, it's preferrable to preserve
blame than go through a layer of cleanup when looking for the commit
that defined particular flags. It's ok to cleanup code at the same time
definitions change for functional or performance reasons.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/25/16 09:01, SeongJae Park wrote:
> 
> Well, the indentation for the comment and the '\' looks odd to me.  If
> the 80 column limit is necessary, how about moving the comment to above
> line of the macro as below?  Because comments are usually placed before
> the target they are explaining, I believe this may better to read.
> 
>  -#define __GFP_MOVABLE        ((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
>  +/* ZONE_MOVABLE allowed */
>  +#define __GFP_MOVABLE        ((__force gfp_t)___GFP_MOVABLE)
> 
> Maybe the opinion can be applied to below similar changes, too.
> 

At least for me, what you said above is OK (it is a common way).

And welcome other members' suggestions.

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/25/16 09:01, SeongJae Park wrote:
> 
> Well, the indentation for the comment and the '\' looks odd to me.  If
> the 80 column limit is necessary, how about moving the comment to above
> line of the macro as below?  Because comments are usually placed before
> the target they are explaining, I believe this may better to read.
> 
>  -#define __GFP_MOVABLE        ((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
>  +/* ZONE_MOVABLE allowed */
>  +#define __GFP_MOVABLE        ((__force gfp_t)___GFP_MOVABLE)
> 
> Maybe the opinion can be applied to below similar changes, too.
> 

At least for me, what you said above is OK (it is a common way).

And welcome other members' suggestions.

Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/25/16 16:57, Michal Hocko wrote:
> On Thu 25-02-16 06:26:31, chengang@emindsoft.com.cn wrote:
>>
>> Always notice about 80 columns, and the white space near '|'.
>>
>> Let the wrapped function parameters align as the same styles.
>>
>> Remove redundant statement "enum zone_type z;" in function gfp_zone.
> 
> I do not think this is an improvement. The comment placement is just odd
> and artificially splitting the mask into more lines makes git grep
> harder to use.
> 

Excuse me, I am not quite sure your meaning is the whole contents of the
patch is worthless, or only for the "comment placement"?

For the "comment placement" the common way is below, but still make git
grep harder:

-#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
+/* ZONE_MOVABLE allowed */
+#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)

Then how about:

-#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
+#define __GFP_MOVABLE	\
		((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */

or:

-#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
+#define __GFP_MOVABLE	/* ZONE_MOVABLE allowed */ \
			((__force gfp_t)___GFP_MOVABLE)


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/25/16 16:57, Michal Hocko wrote:
> On Thu 25-02-16 06:26:31, chengang@emindsoft.com.cn wrote:
>>
>> Always notice about 80 columns, and the white space near '|'.
>>
>> Let the wrapped function parameters align as the same styles.
>>
>> Remove redundant statement "enum zone_type z;" in function gfp_zone.
> 
> I do not think this is an improvement. The comment placement is just odd
> and artificially splitting the mask into more lines makes git grep
> harder to use.
> 

Excuse me, I am not quite sure your meaning is the whole contents of the
patch is worthless, or only for the "comment placement"?

For the "comment placement" the common way is below, but still make git
grep harder:

-#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
+/* ZONE_MOVABLE allowed */
+#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)

Then how about:

-#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
+#define __GFP_MOVABLE	\
		((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */

or:

-#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
+#define __GFP_MOVABLE	/* ZONE_MOVABLE allowed */ \
			((__force gfp_t)___GFP_MOVABLE)


Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/25/16 17:27, Mel Gorman wrote:
> On Thu, Feb 25, 2016 at 06:26:31AM +0800, chengang@emindsoft.com.cn wrote:
>> From: Chen Gang <chengang@emindsoft.com.cn>
>>
>> Always notice about 80 columns, and the white space near '|'.
>>
>> Let the wrapped function parameters align as the same styles.
>>
>> Remove redundant statement "enum zone_type z;" in function gfp_zone.
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> 
> NAK from me at least. From my perspective, it's preferrable to preserve
> blame than go through a layer of cleanup when looking for the commit
> that defined particular flags. It's ok to cleanup code at the same time
> definitions change for functional or performance reasons.
> 

I can understand for your NAK, it is a trivial patch. For me, I guess
trivial@kernel.org will care about this kind of patch.

If we have another better way than sending trivial patch, that will be
OK to me. At present, I am learning mm in my free time, when I feel
something is valuable more or less, I will send related patch for it.

And excuse me, I guess my english is not quite well, I am not quite
understand the meaning below, could you provide more details?

  "it's preferable to preserve blame than go through a layer of cleanup
  when looking for the commit that defined particular flags".

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/25/16 17:27, Mel Gorman wrote:
> On Thu, Feb 25, 2016 at 06:26:31AM +0800, chengang@emindsoft.com.cn wrote:
>> From: Chen Gang <chengang@emindsoft.com.cn>
>>
>> Always notice about 80 columns, and the white space near '|'.
>>
>> Let the wrapped function parameters align as the same styles.
>>
>> Remove redundant statement "enum zone_type z;" in function gfp_zone.
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> 
> NAK from me at least. From my perspective, it's preferrable to preserve
> blame than go through a layer of cleanup when looking for the commit
> that defined particular flags. It's ok to cleanup code at the same time
> definitions change for functional or performance reasons.
> 

I can understand for your NAK, it is a trivial patch. For me, I guess
trivial@kernel.org will care about this kind of patch.

If we have another better way than sending trivial patch, that will be
OK to me. At present, I am learning mm in my free time, when I feel
something is valuable more or less, I will send related patch for it.

And excuse me, I guess my english is not quite well, I am not quite
understand the meaning below, could you provide more details?

  "it's preferable to preserve blame than go through a layer of cleanup
  when looking for the commit that defined particular flags".

Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Michal Hocko]

On Thu 25-02-16 22:23:38, Chen Gang wrote:
> On 2/25/16 16:57, Michal Hocko wrote:
> > On Thu 25-02-16 06:26:31, chengang@emindsoft.com.cn wrote:
> >>
> >> Always notice about 80 columns, and the white space near '|'.
> >>
> >> Let the wrapped function parameters align as the same styles.
> >>
> >> Remove redundant statement "enum zone_type z;" in function gfp_zone.
> > 
> > I do not think this is an improvement. The comment placement is just odd
> > and artificially splitting the mask into more lines makes git grep
> > harder to use.
> > 
> 
> Excuse me, I am not quite sure your meaning is the whole contents of the
> patch is worthless, or only for the "comment placement"?
> 
> For the "comment placement" the common way is below, but still make git
> grep harder:

if you did git grep ZONE_MOVABLE you would get less information

> 
> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
> +/* ZONE_MOVABLE allowed */
> +#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)
> 
> Then how about:
> 
> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
> +#define __GFP_MOVABLE	\
> 		((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */
> 
> or:
> 
> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
> +#define __GFP_MOVABLE	/* ZONE_MOVABLE allowed */ \
> 			((__force gfp_t)___GFP_MOVABLE)

Now looks worse then other, really. Please try to think what would be
a benefit of such change. As Mel already pointed out git blame would
take an additional step to get back to the patch which has introduced
them. And what is the advantage? Make 80 characters-per-line rule happy?
I just do not think this is worth changes at all.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Michal Hocko]

On Thu 25-02-16 22:23:38, Chen Gang wrote:
> On 2/25/16 16:57, Michal Hocko wrote:
> > On Thu 25-02-16 06:26:31, chengang@emindsoft.com.cn wrote:
> >>
> >> Always notice about 80 columns, and the white space near '|'.
> >>
> >> Let the wrapped function parameters align as the same styles.
> >>
> >> Remove redundant statement "enum zone_type z;" in function gfp_zone.
> > 
> > I do not think this is an improvement. The comment placement is just odd
> > and artificially splitting the mask into more lines makes git grep
> > harder to use.
> > 
> 
> Excuse me, I am not quite sure your meaning is the whole contents of the
> patch is worthless, or only for the "comment placement"?
> 
> For the "comment placement" the common way is below, but still make git
> grep harder:

if you did git grep ZONE_MOVABLE you would get less information

> 
> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
> +/* ZONE_MOVABLE allowed */
> +#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)
> 
> Then how about:
> 
> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
> +#define __GFP_MOVABLE	\
> 		((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */
> 
> or:
> 
> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
> +#define __GFP_MOVABLE	/* ZONE_MOVABLE allowed */ \
> 			((__force gfp_t)___GFP_MOVABLE)

Now looks worse then other, really. Please try to think what would be
a benefit of such change. As Mel already pointed out git blame would
take an additional step to get back to the patch which has introduced
them. And what is the advantage? Make 80 characters-per-line rule happy?
I just do not think this is worth changes at all.

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Jiri Kosina]

On Thu, 25 Feb 2016, Chen Gang wrote:

> I can understand for your NAK, it is a trivial patch. 

Not all trivial patches are NAKed :) But they have to be generally useful.

Shuffling code around, without actually changing / improving it a bit, 
just for the sole purpose of formatting, is kind of pointless (especially 
given the fact that the current code as-is is easily readable; it's not 
like it'd be a horrible mess difficult to understand).

Sure, it might had been formatted better at the time it was actually 
merged. But changing it "just because" after being in tree for long time 
doesn't fix any problem really.

> And excuse me, I guess my english is not quite well, I am not quite
> understand the meaning below, could you provide more details?
> 
>   "it's preferable to preserve blame than go through a layer of cleanup
>   when looking for the commit that defined particular flags".

git-blame. When looking at commits touching particular lines, you add an 
extra hop to the person who is trying to find a (functional) commit that 
touched a particular line.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Jiri Kosina]

On Thu, 25 Feb 2016, Chen Gang wrote:

> I can understand for your NAK, it is a trivial patch. 

Not all trivial patches are NAKed :) But they have to be generally useful.

Shuffling code around, without actually changing / improving it a bit, 
just for the sole purpose of formatting, is kind of pointless (especially 
given the fact that the current code as-is is easily readable; it's not 
like it'd be a horrible mess difficult to understand).

Sure, it might had been formatted better at the time it was actually 
merged. But changing it "just because" after being in tree for long time 
doesn't fix any problem really.

> And excuse me, I guess my english is not quite well, I am not quite
> understand the meaning below, could you provide more details?
> 
>   "it's preferable to preserve blame than go through a layer of cleanup
>   when looking for the commit that defined particular flags".

git-blame. When looking at commits touching particular lines, you add an 
extra hop to the person who is trying to find a (functional) commit that 
touched a particular line.

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Mel Gorman]

On Thu, Feb 25, 2016 at 10:38:58PM +0800, Chen Gang wrote:
> On 2/25/16 17:27, Mel Gorman wrote:
> > On Thu, Feb 25, 2016 at 06:26:31AM +0800, chengang@emindsoft.com.cn wrote:
> >> From: Chen Gang <chengang@emindsoft.com.cn>
> >>
> >> Always notice about 80 columns, and the white space near '|'.
> >>
> >> Let the wrapped function parameters align as the same styles.
> >>
> >> Remove redundant statement "enum zone_type z;" in function gfp_zone.
> >>
> >> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> > 
> > NAK from me at least. From my perspective, it's preferrable to preserve
> > blame than go through a layer of cleanup when looking for the commit
> > that defined particular flags. It's ok to cleanup code at the same time
> > definitions change for functional or performance reasons.
> > 
> 
> I can understand for your NAK, it is a trivial patch. For me, I guess
> trivial@kernel.org will care about this kind of patch.
> 

I do not want this patch to go through the trivial tree. It still adds
another step to identifying relevant commits through git blame and has
limited, if any, benefit to maintainability.

>   "it's preferable to preserve blame than go through a layer of cleanup
>   when looking for the commit that defined particular flags".
> 

git blame identifies what commit last altered a line. If a cleanup patch
is encountered then the tree before that commit needs to be examined
which adds time. It's rare that cleanup patches on their own are useful
and this is one of those cases.

-- 
Mel Gorman
SUSE Labs

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Mel Gorman]

On Thu, Feb 25, 2016 at 10:38:58PM +0800, Chen Gang wrote:
> On 2/25/16 17:27, Mel Gorman wrote:
> > On Thu, Feb 25, 2016 at 06:26:31AM +0800, chengang@emindsoft.com.cn wrote:
> >> From: Chen Gang <chengang@emindsoft.com.cn>
> >>
> >> Always notice about 80 columns, and the white space near '|'.
> >>
> >> Let the wrapped function parameters align as the same styles.
> >>
> >> Remove redundant statement "enum zone_type z;" in function gfp_zone.
> >>
> >> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> > 
> > NAK from me at least. From my perspective, it's preferrable to preserve
> > blame than go through a layer of cleanup when looking for the commit
> > that defined particular flags. It's ok to cleanup code at the same time
> > definitions change for functional or performance reasons.
> > 
> 
> I can understand for your NAK, it is a trivial patch. For me, I guess
> trivial@kernel.org will care about this kind of patch.
> 

I do not want this patch to go through the trivial tree. It still adds
another step to identifying relevant commits through git blame and has
limited, if any, benefit to maintainability.

>   "it's preferable to preserve blame than go through a layer of cleanup
>   when looking for the commit that defined particular flags".
> 

git blame identifies what commit last altered a line. If a cleanup patch
is encountered then the tree before that commit needs to be examined
which adds time. It's rare that cleanup patches on their own are useful
and this is one of those cases.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/25/16 22:47, Michal Hocko wrote:
>>
>> For the "comment placement" the common way is below, but still make git
>> grep harder:
> 
> if you did git grep ZONE_MOVABLE you would get less information
> 

OK.

>>
>> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
>> +/* ZONE_MOVABLE allowed */
>> +#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)
>>
>> Then how about:
>>
>> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
>> +#define __GFP_MOVABLE	\
>> 		((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */
>>
>> or:
>>
>> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
>> +#define __GFP_MOVABLE	/* ZONE_MOVABLE allowed */ \
>> 			((__force gfp_t)___GFP_MOVABLE)
> 
> Now looks worse then other, really. Please try to think what would be
> a benefit of such change. As Mel already pointed out git blame would
> take an additional step to get back to the patch which has introduced
> them. And what is the advantage? Make 80 characters-per-line rule happy?
> I just do not think this is worth changes at all.
> 

For 80 column limitation:

 - I often use vsp (vertical split window) in vim to reading code in the
   2 files, 80 columns limitation can avoid the line wrap, which will
   let code reading better.

 - Sometimes we need copy/past the code to a pdf files (e.g. print the
   interface header file contents to a new document as appendix), or
   print the code to a physical paper (e.g. write a book).

For worth or worthless:

  The shared header files (e.g. in our case), have more chances to be
  read or printed than the normal source code files. So for me, we need
  take more care about the coding styles of them.

For git-blame:

 - It really a good feature! Originally, I did not know about it :-).

 - Can it instead of sending trivial patch? (I guess not).


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/25/16 22:47, Michal Hocko wrote:
>>
>> For the "comment placement" the common way is below, but still make git
>> grep harder:
> 
> if you did git grep ZONE_MOVABLE you would get less information
> 

OK.

>>
>> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
>> +/* ZONE_MOVABLE allowed */
>> +#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)
>>
>> Then how about:
>>
>> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
>> +#define __GFP_MOVABLE	\
>> 		((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */
>>
>> or:
>>
>> -#define __GFP_MOVABLE	((__force gfp_t)___GFP_MOVABLE)  /* ZONE_MOVABLE allowed */
>> +#define __GFP_MOVABLE	/* ZONE_MOVABLE allowed */ \
>> 			((__force gfp_t)___GFP_MOVABLE)
> 
> Now looks worse then other, really. Please try to think what would be
> a benefit of such change. As Mel already pointed out git blame would
> take an additional step to get back to the patch which has introduced
> them. And what is the advantage? Make 80 characters-per-line rule happy?
> I just do not think this is worth changes at all.
> 

For 80 column limitation:

 - I often use vsp (vertical split window) in vim to reading code in the
   2 files, 80 columns limitation can avoid the line wrap, which will
   let code reading better.

 - Sometimes we need copy/past the code to a pdf files (e.g. print the
   interface header file contents to a new document as appendix), or
   print the code to a physical paper (e.g. write a book).

For worth or worthless:

  The shared header files (e.g. in our case), have more chances to be
  read or printed than the normal source code files. So for me, we need
  take more care about the coding styles of them.

For git-blame:

 - It really a good feature! Originally, I did not know about it :-).

 - Can it instead of sending trivial patch? (I guess not).


Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/25/16 23:12, Jiri Kosina wrote:
> On Thu, 25 Feb 2016, Chen Gang wrote:
> 
>> I can understand for your NAK, it is a trivial patch. 
> 
> Not all trivial patches are NAKed :) But they have to be generally useful.
> 
> Shuffling code around, without actually changing / improving it a bit, 
> just for the sole purpose of formatting, is kind of pointless (especially 
> given the fact that the current code as-is is easily readable; it's not 
> like it'd be a horrible mess difficult to understand).
> 
> Sure, it might had been formatted better at the time it was actually 
> merged. But changing it "just because" after being in tree for long time 
> doesn't fix any problem really.
> 

OK, thanks. I have replied the related contents in the other thread.

Welcome any ideas, suggestions, and completions in the other related
thread.

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/25/16 23:12, Jiri Kosina wrote:
> On Thu, 25 Feb 2016, Chen Gang wrote:
> 
>> I can understand for your NAK, it is a trivial patch. 
> 
> Not all trivial patches are NAKed :) But they have to be generally useful.
> 
> Shuffling code around, without actually changing / improving it a bit, 
> just for the sole purpose of formatting, is kind of pointless (especially 
> given the fact that the current code as-is is easily readable; it's not 
> like it'd be a horrible mess difficult to understand).
> 
> Sure, it might had been formatted better at the time it was actually 
> merged. But changing it "just because" after being in tree for long time 
> doesn't fix any problem really.
> 

OK, thanks. I have replied the related contents in the other thread.

Welcome any ideas, suggestions, and completions in the other related
thread.

Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/26/16 00:07, Mel Gorman wrote:
>>> On Thu, Feb 25, 2016 at 06:26:31AM +0800, chengang@emindsoft.com.cn wrote:
> 
> I do not want this patch to go through the trivial tree. It still adds
> another step to identifying relevant commits through git blame and has
> limited, if any, benefit to maintainability.
> 
>>   "it's preferable to preserve blame than go through a layer of cleanup
>>   when looking for the commit that defined particular flags".
>>
> 
> git blame identifies what commit last altered a line. If a cleanup patch
> is encountered then the tree before that commit needs to be examined
> which adds time. It's rare that cleanup patches on their own are useful
> and this is one of those cases.
> 

git is a tool mainly for analyzing code, but not mainly for normal
reading main code.

So for me, the coding styles need not consider about git.


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/26/16 00:07, Mel Gorman wrote:
>>> On Thu, Feb 25, 2016 at 06:26:31AM +0800, chengang@emindsoft.com.cn wrote:
> 
> I do not want this patch to go through the trivial tree. It still adds
> another step to identifying relevant commits through git blame and has
> limited, if any, benefit to maintainability.
> 
>>   "it's preferable to preserve blame than go through a layer of cleanup
>>   when looking for the commit that defined particular flags".
>>
> 
> git blame identifies what commit last altered a line. If a cleanup patch
> is encountered then the tree before that commit needs to be examined
> which adds time. It's rare that cleanup patches on their own are useful
> and this is one of those cases.
> 

git is a tool mainly for analyzing code, but not mainly for normal
reading main code.

So for me, the coding styles need not consider about git.


Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Jiri Kosina]

On Fri, 26 Feb 2016, Chen Gang wrote:

> > git blame identifies what commit last altered a line. If a cleanup patch
> > is encountered then the tree before that commit needs to be examined
> > which adds time. It's rare that cleanup patches on their own are useful
> > and this is one of those cases.
> 
> git is a tool mainly for analyzing code, but not mainly for normal
> reading main code.
> 
> So for me, the coding styles need not consider about git.

You are mistaken here. It's very helpful when debugging; usually you want 
to find the commit that introduced particular change, and read its 
changelog (at least). Having to cross rather pointless changes just adds 
time (need to restart git-blame with commit~1 as a base) for no really 
good reason.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Jiri Kosina]

On Fri, 26 Feb 2016, Chen Gang wrote:

> > git blame identifies what commit last altered a line. If a cleanup patch
> > is encountered then the tree before that commit needs to be examined
> > which adds time. It's rare that cleanup patches on their own are useful
> > and this is one of those cases.
> 
> git is a tool mainly for analyzing code, but not mainly for normal
> reading main code.
> 
> So for me, the coding styles need not consider about git.

You are mistaken here. It's very helpful when debugging; usually you want 
to find the commit that introduced particular change, and read its 
changelog (at least). Having to cross rather pointless changes just adds 
time (need to restart git-blame with commit~1 as a base) for no really 
good reason.

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[SeongJae Park]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2079 bytes --]



On Fri, 26 Feb 2016, Chen Gang wrote:

> On 2/26/16 00:07, Mel Gorman wrote:
>>>> On Thu, Feb 25, 2016 at 06:26:31AM +0800, chengang@emindsoft.com.cn wrote:
>>
>> I do not want this patch to go through the trivial tree. It still adds
>> another step to identifying relevant commits through git blame and has
>> limited, if any, benefit to maintainability.
>>
>>>   "it's preferable to preserve blame than go through a layer of cleanup
>>>   when looking for the commit that defined particular flags".
>>>
>>
>> git blame identifies what commit last altered a line. If a cleanup patch
>> is encountered then the tree before that commit needs to be examined
>> which adds time. It's rare that cleanup patches on their own are useful
>> and this is one of those cases.
>>
>
> git is a tool mainly for analyzing code, but not mainly for normal
> reading main code.
>
> So for me, the coding styles need not consider about git.


It is common to see reject of trivial coding style fixup patch here and
there.  Those patches usually be merged for early stage files that only
few people read / write.  However, for files that are old and lots of
people read and write, those patches are rejected in usual.  I mean, the
negative opinions for this patches are usual in this community.

I agree that coding style is important and respect your effort.  However,
because the code will be seen and written by most kernel hackers, the file
should be maintained to be easily readable and writable by most kernel
hackers, especially, maintainers.  What I want to say is, we should
respect maintainers' opinion in usual.

As far as I remember, I have seen a document that saying same with others'
opinion but couldn't find it.


Thanks,
SeongJae Park

>
>
> Thanks.
> -- 
> Chen Gang (陈刚)
>
> Managing Natural Environments is the Duty of Human Beings.
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[SeongJae Park]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2079 bytes --]



On Fri, 26 Feb 2016, Chen Gang wrote:

> On 2/26/16 00:07, Mel Gorman wrote:
>>>> On Thu, Feb 25, 2016 at 06:26:31AM +0800, chengang@emindsoft.com.cn wrote:
>>
>> I do not want this patch to go through the trivial tree. It still adds
>> another step to identifying relevant commits through git blame and has
>> limited, if any, benefit to maintainability.
>>
>>>   "it's preferable to preserve blame than go through a layer of cleanup
>>>   when looking for the commit that defined particular flags".
>>>
>>
>> git blame identifies what commit last altered a line. If a cleanup patch
>> is encountered then the tree before that commit needs to be examined
>> which adds time. It's rare that cleanup patches on their own are useful
>> and this is one of those cases.
>>
>
> git is a tool mainly for analyzing code, but not mainly for normal
> reading main code.
>
> So for me, the coding styles need not consider about git.


It is common to see reject of trivial coding style fixup patch here and
there.  Those patches usually be merged for early stage files that only
few people read / write.  However, for files that are old and lots of
people read and write, those patches are rejected in usual.  I mean, the
negative opinions for this patches are usual in this community.

I agree that coding style is important and respect your effort.  However,
because the code will be seen and written by most kernel hackers, the file
should be maintained to be easily readable and writable by most kernel
hackers, especially, maintainers.  What I want to say is, we should
respect maintainers' opinion in usual.

As far as I remember, I have seen a document that saying same with others'
opinion but couldn't find it.


Thanks,
SeongJae Park

>
>
> Thanks.
> -- 
> Chen Gang (e??a??)
>
> Managing Natural Environments is the Duty of Human Beings.
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Jianyu Zhan]

On Fri, Feb 26, 2016 at 6:29 AM, Chen Gang <chengang@emindsoft.com.cn> wrote:
> git is a tool mainly for analyzing code, but not mainly for normal
> reading main code.
>
> So for me, the coding styles need not consider about git.

For you, maybe yes.

But for most of the developers/learners,  git blame does help a lot.
Kernel code was not as complicated as it is now, it is keeping evolving.

So basically a history chain is indispensable in studying such a complex system.
git blame fits in this role.  I benefited a lot from using it when I
started to learn the code,
And,  a pure coding style fix is sometimes really troublesome as I
have to use git blame
to go another step up along the history chain,  which is time
consuming and boring.

But after all, I bet you will be fond of using it if you dive deeper
into the kernel code studying.
And if you do,  you will know why so many developers in this thread
are so upset and allergic
to such coding-style fix.

As for coding style, actually IMHO this patch is even _not_ a coding
style, more like a code shuffle, indeed.

And for your commit history, I found actually you have already
contributed some quit good patches.
I don't think it is helpful for a non-layman contributor to keep
generating such code churn.



Thanks,
Jianyu Zhan

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Jianyu Zhan]

On Fri, Feb 26, 2016 at 6:29 AM, Chen Gang <chengang@emindsoft.com.cn> wrote:
> git is a tool mainly for analyzing code, but not mainly for normal
> reading main code.
>
> So for me, the coding styles need not consider about git.

For you, maybe yes.

But for most of the developers/learners,  git blame does help a lot.
Kernel code was not as complicated as it is now, it is keeping evolving.

So basically a history chain is indispensable in studying such a complex system.
git blame fits in this role.  I benefited a lot from using it when I
started to learn the code,
And,  a pure coding style fix is sometimes really troublesome as I
have to use git blame
to go another step up along the history chain,  which is time
consuming and boring.

But after all, I bet you will be fond of using it if you dive deeper
into the kernel code studying.
And if you do,  you will know why so many developers in this thread
are so upset and allergic
to such coding-style fix.

As for coding style, actually IMHO this patch is even _not_ a coding
style, more like a code shuffle, indeed.

And for your commit history, I found actually you have already
contributed some quit good patches.
I don't think it is helpful for a non-layman contributor to keep
generating such code churn.



Thanks,
Jianyu Zhan

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/26/16 06:39, Jiri Kosina wrote:
> On Fri, 26 Feb 2016, Chen Gang wrote:
> 
>> git is a tool mainly for analyzing code, but not mainly for normal
>> reading main code.
>>
>> So for me, the coding styles need not consider about git.
> 
> You are mistaken here. It's very helpful when debugging;

For me, 'debugging' is related with debugger (e.g. kdb or kgdb), and
'tracing' is related with dumping log, and code analyzing is related
with "git diff" and "git blame".

And yes, for me, "git diff" and "git blame" is really very helpful for
code analyzing.

>                                                         usually you want 
> to find the commit that introduced particular change, and read its 
> changelog (at least). Having to cross rather pointless changes just adds 
> time (need to restart git-blame with commit~1 as a base) for no really 
> good reason.
> 

That is the reason why I am not quite care about body files, I often use
"git log -p filename", the cleanup code patch has negative effect with
code analyzing (although for me, it should still need to be cleanup).

But in our case, it is for the shared header file:

 - They are often the common base file, the main contents will not be
   changed quite often, and their contents are usually simple enough (
   e.g. gfp.h in our case), they are not often for "code analyzing".

 - But they are quite often read in normal reading ways by programmers
   (e.g. open with normal editors). For normal reading, programmers
   usually care about the contents, not the changes.

 - So for me, the common shared header files need always take care about
   coding styles, and need not consider about code analyzing.

And if we reject this kind of patch (in our case), I guess, that almost
mean: "for the common shared header files, their bad coding styles will
be remain for ever".


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/26/16 06:39, Jiri Kosina wrote:
> On Fri, 26 Feb 2016, Chen Gang wrote:
> 
>> git is a tool mainly for analyzing code, but not mainly for normal
>> reading main code.
>>
>> So for me, the coding styles need not consider about git.
> 
> You are mistaken here. It's very helpful when debugging;

For me, 'debugging' is related with debugger (e.g. kdb or kgdb), and
'tracing' is related with dumping log, and code analyzing is related
with "git diff" and "git blame".

And yes, for me, "git diff" and "git blame" is really very helpful for
code analyzing.

>                                                         usually you want 
> to find the commit that introduced particular change, and read its 
> changelog (at least). Having to cross rather pointless changes just adds 
> time (need to restart git-blame with commit~1 as a base) for no really 
> good reason.
> 

That is the reason why I am not quite care about body files, I often use
"git log -p filename", the cleanup code patch has negative effect with
code analyzing (although for me, it should still need to be cleanup).

But in our case, it is for the shared header file:

 - They are often the common base file, the main contents will not be
   changed quite often, and their contents are usually simple enough (
   e.g. gfp.h in our case), they are not often for "code analyzing".

 - But they are quite often read in normal reading ways by programmers
   (e.g. open with normal editors). For normal reading, programmers
   usually care about the contents, not the changes.

 - So for me, the common shared header files need always take care about
   coding styles, and need not consider about code analyzing.

And if we reject this kind of patch (in our case), I guess, that almost
mean: "for the common shared header files, their bad coding styles will
be remain for ever".


Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/26/16 07:12, SeongJae Park wrote:
> 
> On Fri, 26 Feb 2016, Chen Gang wrote:
> 
>>
>> git is a tool mainly for analyzing code, but not mainly for normal
>> reading main code.
>>
>> So for me, the coding styles need not consider about git.
> 
> 
> It is common to see reject of trivial coding style fixup patch here and
> there.  Those patches usually be merged for early stage files that only
> few people read / write.  However, for files that are old and lots of
> people read and write, those patches are rejected in usual.  I mean, the
> negative opinions for this patches are usual in this community.
> 
> I agree that coding style is important and respect your effort.  However,
> because the code will be seen and written by most kernel hackers, the file
> should be maintained to be easily readable and writable by most kernel
> hackers, especially, maintainers.  What I want to say is, we should
> respect maintainers' opinion in usual.
> 

Yes we need consider about the maintainers' options.

And my another ideas are replied in the other thread, please check, and
welcome any ideas, suggestion, and completions.


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/26/16 07:12, SeongJae Park wrote:
> 
> On Fri, 26 Feb 2016, Chen Gang wrote:
> 
>>
>> git is a tool mainly for analyzing code, but not mainly for normal
>> reading main code.
>>
>> So for me, the coding styles need not consider about git.
> 
> 
> It is common to see reject of trivial coding style fixup patch here and
> there.  Those patches usually be merged for early stage files that only
> few people read / write.  However, for files that are old and lots of
> people read and write, those patches are rejected in usual.  I mean, the
> negative opinions for this patches are usual in this community.
> 
> I agree that coding style is important and respect your effort.  However,
> because the code will be seen and written by most kernel hackers, the file
> should be maintained to be easily readable and writable by most kernel
> hackers, especially, maintainers.  What I want to say is, we should
> respect maintainers' opinion in usual.
> 

Yes we need consider about the maintainers' options.

And my another ideas are replied in the other thread, please check, and
welcome any ideas, suggestion, and completions.


Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/26/16 10:32, Jianyu Zhan wrote:
> On Fri, Feb 26, 2016 at 6:29 AM, Chen Gang <chengang@emindsoft.com.cn> wrote:
>> git is a tool mainly for analyzing code, but not mainly for normal
>> reading main code.
>>
>> So for me, the coding styles need not consider about git.
> 
> For you, maybe yes.
> 
> But for most of the developers/learners,  git blame does help a lot.
> Kernel code was not as complicated as it is now, it is keeping evolving.
> 

Yes.

> So basically a history chain is indispensable in studying such a complex system.
> git blame fits in this role.  I benefited a lot from using it when I
> started to learn the code,
> And,  a pure coding style fix is sometimes really troublesome as I
> have to use git blame
> to go another step up along the history chain,  which is time
> consuming and boring.
> 
> But after all, I bet you will be fond of using it if you dive deeper
> into the kernel code studying.
> And if you do,  you will know why so many developers in this thread
> are so upset and allergic
> to such coding-style fix.
> 

For me, for discussion, I don't care about "so many developers", I only
focus on the proof and the contribution.


> As for coding style, actually IMHO this patch is even _not_ a coding
> style, more like a code shuffle, indeed.
> 

"80 column limitation" is about coding style, I guess, all of us agree
with it.

> And for your commit history, I found actually you have already
> contributed some quit good patches.

For me, I don't care about my history -- except some members find issues
related with my original patches, I have duty to analyze the related
issues together with the finders.

> I don't think it is helpful for a non-layman contributor to keep
> generating such code churn.
> 

For me, we are discussing, so it is not quite suitable to make an early
conclusion (code churn).

For me, I don't care about layman or non-layman, I only focus on the
proof and the contribution.

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/26/16 10:32, Jianyu Zhan wrote:
> On Fri, Feb 26, 2016 at 6:29 AM, Chen Gang <chengang@emindsoft.com.cn> wrote:
>> git is a tool mainly for analyzing code, but not mainly for normal
>> reading main code.
>>
>> So for me, the coding styles need not consider about git.
> 
> For you, maybe yes.
> 
> But for most of the developers/learners,  git blame does help a lot.
> Kernel code was not as complicated as it is now, it is keeping evolving.
> 

Yes.

> So basically a history chain is indispensable in studying such a complex system.
> git blame fits in this role.  I benefited a lot from using it when I
> started to learn the code,
> And,  a pure coding style fix is sometimes really troublesome as I
> have to use git blame
> to go another step up along the history chain,  which is time
> consuming and boring.
> 
> But after all, I bet you will be fond of using it if you dive deeper
> into the kernel code studying.
> And if you do,  you will know why so many developers in this thread
> are so upset and allergic
> to such coding-style fix.
> 

For me, for discussion, I don't care about "so many developers", I only
focus on the proof and the contribution.


> As for coding style, actually IMHO this patch is even _not_ a coding
> style, more like a code shuffle, indeed.
> 

"80 column limitation" is about coding style, I guess, all of us agree
with it.

> And for your commit history, I found actually you have already
> contributed some quit good patches.

For me, I don't care about my history -- except some members find issues
related with my original patches, I have duty to analyze the related
issues together with the finders.

> I don't think it is helpful for a non-layman contributor to keep
> generating such code churn.
> 

For me, we are discussing, so it is not quite suitable to make an early
conclusion (code churn).

For me, I don't care about layman or non-layman, I only focus on the
proof and the contribution.

Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Theodore Ts'o]

On Fri, Feb 26, 2016 at 11:26:02PM +0800, Chen Gang wrote:
> > As for coding style, actually IMHO this patch is even _not_ a coding
> > style, more like a code shuffle, indeed.
> > 
> 
> "80 column limitation" is about coding style, I guess, all of us agree
> with it.

No, it's been accepted that checkpatch requiring people to reformat
code to within be 80 columns limitation was actively harmful, and it
no longer does that.

Worse, it now complains when you split a printf string across lines,
so there were patches that split a string across multiple lines to
make checkpatch shut up.  And now there are patches that join the
string back together.

And if you now start submitting patches to split them up again because
you think the 80 column restriction is so darned important, that would
be even ***more*** code churn.

Which is one of the reasons why some of us aren't terribly happy with
people who start running checkpatch -file on other people's code and
start submitting patches, either through the trivial patch portal or
not.

Mel, as an MM developer, has already NACK'ed the patch, which means
you should not send the patch to **any** upstream maintainer for
inclusion.

						- Ted
						

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Theodore Ts'o]

On Fri, Feb 26, 2016 at 11:26:02PM +0800, Chen Gang wrote:
> > As for coding style, actually IMHO this patch is even _not_ a coding
> > style, more like a code shuffle, indeed.
> > 
> 
> "80 column limitation" is about coding style, I guess, all of us agree
> with it.

No, it's been accepted that checkpatch requiring people to reformat
code to within be 80 columns limitation was actively harmful, and it
no longer does that.

Worse, it now complains when you split a printf string across lines,
so there were patches that split a string across multiple lines to
make checkpatch shut up.  And now there are patches that join the
string back together.

And if you now start submitting patches to split them up again because
you think the 80 column restriction is so darned important, that would
be even ***more*** code churn.

Which is one of the reasons why some of us aren't terribly happy with
people who start running checkpatch -file on other people's code and
start submitting patches, either through the trivial patch portal or
not.

Mel, as an MM developer, has already NACK'ed the patch, which means
you should not send the patch to **any** upstream maintainer for
inclusion.

						- Ted
						

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/27/16 10:45, Theodore Ts'o wrote:
> On Fri, Feb 26, 2016 at 11:26:02PM +0800, Chen Gang wrote:
>>> As for coding style, actually IMHO this patch is even _not_ a coding
>>> style, more like a code shuffle, indeed.
>>>
>>
>> "80 column limitation" is about coding style, I guess, all of us agree
>> with it.
> 
> No, it's been accepted that checkpatch requiring people to reformat
> code to within be 80 columns limitation was actively harmful, and it
> no longer does that.
> 
> Worse, it now complains when you split a printf string across lines,
> so there were patches that split a string across multiple lines to
> make checkpatch shut up.  And now there are patches that join the
> string back together.
> 
> And if you now start submitting patches to split them up again because
> you think the 80 column restriction is so darned important, that would
> be even ***more*** code churn.
> 

I don't think so. Of cause NOT the "CODE CHURN". It is not correct to
make an early decision during discussing.

"80 column limitation" is mentioned in "Documentation/CodingStyle", if
we have very good reason for it, we can break this limitation (for me,
what you said above are really some of good reasons).

But in our case (the patch), can anybody find any "good reasons" for it?
at least, at present, I can not find:

 - It is a common shared base header file, it is almost not used for
   code analyzing (e.g. git diff, git blame).

 - Is it helpful for "grep xxx filename | grep yyy"? Please check the
   patch, I can not find (maybe __GFP_MOVABL definition be? but it is
   still not obvious, if some member stick to, we can keep it no touch).

 - Could anyone find any good reasons for it within this patch?


> Which is one of the reasons why some of us aren't terribly happy with
> people who start running checkpatch -file on other people's code and
> start submitting patches, either through the trivial patch portal or
> not.
> 

For me, as a individual developer, I don't like this way, either. So of
cause, I don't care about this way.

I am just reading the common shared header files about mm. At least, I
can understand some common sense of mm, and also read through the whole
other headers to know what they are.

When I find something valuable more or less, I shall send related patch
for it.

> Mel, as an MM developer, has already NACK'ed the patch, which means
> you should not send the patch to **any** upstream maintainer for
> inclusion.

I don't think I "should not ...". I only care about correctness and
contribution, I don't care about any members ideas and their thinking.
When we have different ideas or thinking, we need discuss.

For common shared header files, for me, we should really take more care
about the coding styles.

 - If the common shared header files don't care about the coding styles,
   I guess any body files will have much more excuses for "do not care
   about coding styles".

 - That means our kernel whole source files need not care about coding
   styles at all!!

 - It is really really VERY BAD!!

If someone only dislike me to send the related patches, I suggest: Let
another member(s) "run checkpatch -file" on the whole "./include" sub-
directory, and fix all coding styles issues.


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/27/16 10:45, Theodore Ts'o wrote:
> On Fri, Feb 26, 2016 at 11:26:02PM +0800, Chen Gang wrote:
>>> As for coding style, actually IMHO this patch is even _not_ a coding
>>> style, more like a code shuffle, indeed.
>>>
>>
>> "80 column limitation" is about coding style, I guess, all of us agree
>> with it.
> 
> No, it's been accepted that checkpatch requiring people to reformat
> code to within be 80 columns limitation was actively harmful, and it
> no longer does that.
> 
> Worse, it now complains when you split a printf string across lines,
> so there were patches that split a string across multiple lines to
> make checkpatch shut up.  And now there are patches that join the
> string back together.
> 
> And if you now start submitting patches to split them up again because
> you think the 80 column restriction is so darned important, that would
> be even ***more*** code churn.
> 

I don't think so. Of cause NOT the "CODE CHURN". It is not correct to
make an early decision during discussing.

"80 column limitation" is mentioned in "Documentation/CodingStyle", if
we have very good reason for it, we can break this limitation (for me,
what you said above are really some of good reasons).

But in our case (the patch), can anybody find any "good reasons" for it?
at least, at present, I can not find:

 - It is a common shared base header file, it is almost not used for
   code analyzing (e.g. git diff, git blame).

 - Is it helpful for "grep xxx filename | grep yyy"? Please check the
   patch, I can not find (maybe __GFP_MOVABL definition be? but it is
   still not obvious, if some member stick to, we can keep it no touch).

 - Could anyone find any good reasons for it within this patch?


> Which is one of the reasons why some of us aren't terribly happy with
> people who start running checkpatch -file on other people's code and
> start submitting patches, either through the trivial patch portal or
> not.
> 

For me, as a individual developer, I don't like this way, either. So of
cause, I don't care about this way.

I am just reading the common shared header files about mm. At least, I
can understand some common sense of mm, and also read through the whole
other headers to know what they are.

When I find something valuable more or less, I shall send related patch
for it.

> Mel, as an MM developer, has already NACK'ed the patch, which means
> you should not send the patch to **any** upstream maintainer for
> inclusion.

I don't think I "should not ...". I only care about correctness and
contribution, I don't care about any members ideas and their thinking.
When we have different ideas or thinking, we need discuss.

For common shared header files, for me, we should really take more care
about the coding styles.

 - If the common shared header files don't care about the coding styles,
   I guess any body files will have much more excuses for "do not care
   about coding styles".

 - That means our kernel whole source files need not care about coding
   styles at all!!

 - It is really really VERY BAD!!

If someone only dislike me to send the related patches, I suggest: Let
another member(s) "run checkpatch -file" on the whole "./include" sub-
directory, and fix all coding styles issues.


Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Theodore Ts'o]

On Sat, Feb 27, 2016 at 10:32:04PM +0800, Chen Gang wrote:
> I don't think so. Of cause NOT the "CODE CHURN". It is not correct to
> make an early decision during discussing.

There is no discussion.  If the maintainer has NAK'ed it.  That's the
end of the dicsussion.  Period.  See:

ftp://ftp.kernel.org/pub/linux/kernel/people/rusty/trivial/template-index.html

Also note the comment from the above:

   NOTE: This means I'll only take whitespace/indentation fixes from the
   author or maintainer.

      	   	      	   			       	     - Ted

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Theodore Ts'o]

On Sat, Feb 27, 2016 at 10:32:04PM +0800, Chen Gang wrote:
> I don't think so. Of cause NOT the "CODE CHURN". It is not correct to
> make an early decision during discussing.

There is no discussion.  If the maintainer has NAK'ed it.  That's the
end of the dicsussion.  Period.  See:

ftp://ftp.kernel.org/pub/linux/kernel/people/rusty/trivial/template-index.html

Also note the comment from the above:

   NOTE: This means I'll only take whitespace/indentation fixes from the
   author or maintainer.

      	   	      	   			       	     - Ted

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Jiri Kosina]

On Sat, 27 Feb 2016, Chen Gang wrote:

> > Mel, as an MM developer, has already NACK'ed the patch, which means
> > you should not send the patch to **any** upstream maintainer for
> > inclusion.
> 
> I don't think I "should not ...". I only care about correctness and
> contribution, I don't care about any members ideas and their thinking.
> When we have different ideas or thinking, we need discuss.

If by "discuss" you mean "30+ email thread about where to put a line 
break", please drop me from CC next time this discussion is going to 
happen. Thanks.

> For common shared header files, for me, we should really take more care
> about the coding styles.
> 
>  - If the common shared header files don't care about the coding styles,
>    I guess any body files will have much more excuses for "do not care
>    about coding styles".
> 
>  - That means our kernel whole source files need not care about coding
>    styles at all!!
> 
>  - It is really really VERY BAD!!
> 
> If someone only dislike me to send the related patches, I suggest: Let
> another member(s) "run checkpatch -file" on the whole "./include" sub-
> directory, and fix all coding styles issues.

Which is exactly what you shouldn't do.

The ultimate goal of the Linux kernel is not 100% strict complicance to 
the CodingStyle document no matter what. The ultimate goal is to have a 
kernel that is under control. By polluting git blame, you are taking on 
aspect of the "under control" away.

Common sense needs to be used; horribly terrible coding style needs to be 
fixed, sure. Is 82-characters long line horribly terrible coding style? 
No, it's not.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Jiri Kosina]

On Sat, 27 Feb 2016, Chen Gang wrote:

> > Mel, as an MM developer, has already NACK'ed the patch, which means
> > you should not send the patch to **any** upstream maintainer for
> > inclusion.
> 
> I don't think I "should not ...". I only care about correctness and
> contribution, I don't care about any members ideas and their thinking.
> When we have different ideas or thinking, we need discuss.

If by "discuss" you mean "30+ email thread about where to put a line 
break", please drop me from CC next time this discussion is going to 
happen. Thanks.

> For common shared header files, for me, we should really take more care
> about the coding styles.
> 
>  - If the common shared header files don't care about the coding styles,
>    I guess any body files will have much more excuses for "do not care
>    about coding styles".
> 
>  - That means our kernel whole source files need not care about coding
>    styles at all!!
> 
>  - It is really really VERY BAD!!
> 
> If someone only dislike me to send the related patches, I suggest: Let
> another member(s) "run checkpatch -file" on the whole "./include" sub-
> directory, and fix all coding styles issues.

Which is exactly what you shouldn't do.

The ultimate goal of the Linux kernel is not 100% strict complicance to 
the CodingStyle document no matter what. The ultimate goal is to have a 
kernel that is under control. By polluting git blame, you are taking on 
aspect of the "under control" away.

Common sense needs to be used; horribly terrible coding style needs to be 
fixed, sure. Is 82-characters long line horribly terrible coding style? 
No, it's not.

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/28/16 00:53, Theodore Ts'o wrote:
> On Sat, Feb 27, 2016 at 10:32:04PM +0800, Chen Gang wrote:
>> I don't think so. Of cause NOT the "CODE CHURN". It is not correct to
>> make an early decision during discussing.
> 
> There is no discussion.  If the maintainer has NAK'ed it.  That's the
> end of the dicsussion.  Period.  See:
> 

For me, NAK also needs reasons.

And this issue is about "coding styles issue", I am not quite sure
whether trivial patch maintainer and mm maintainer are also the
maintainer for "coding styles issues".

I guess they are related with this patch, and their NAKs' reason are: mm
and trivial don't care about this coding style issue, is it correct?


> ftp://ftp.kernel.org/pub/linux/kernel/people/rusty/trivial/template-index.html
> 
> Also note the comment from the above:
> 
>    NOTE: This means I'll only take whitespace/indentation fixes from the
>    author or maintainer.

OK, thanks, it is really one proof for us. :-)

I guess, the file above almost means: except whitespace/indentation,
trivial patches don't consider about the coding styles issues. But can
we say coding styles issues are not issues in our kernel? (I guess not)

If we can not say, I guess one of your suggestion is useful (maybe be
as your suggestion): find one suitable member (I guess I am not), run
"checkpatch -file" under "./include", and fix all reported issues.

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/28/16 00:53, Theodore Ts'o wrote:
> On Sat, Feb 27, 2016 at 10:32:04PM +0800, Chen Gang wrote:
>> I don't think so. Of cause NOT the "CODE CHURN". It is not correct to
>> make an early decision during discussing.
> 
> There is no discussion.  If the maintainer has NAK'ed it.  That's the
> end of the dicsussion.  Period.  See:
> 

For me, NAK also needs reasons.

And this issue is about "coding styles issue", I am not quite sure
whether trivial patch maintainer and mm maintainer are also the
maintainer for "coding styles issues".

I guess they are related with this patch, and their NAKs' reason are: mm
and trivial don't care about this coding style issue, is it correct?


> ftp://ftp.kernel.org/pub/linux/kernel/people/rusty/trivial/template-index.html
> 
> Also note the comment from the above:
> 
>    NOTE: This means I'll only take whitespace/indentation fixes from the
>    author or maintainer.

OK, thanks, it is really one proof for us. :-)

I guess, the file above almost means: except whitespace/indentation,
trivial patches don't consider about the coding styles issues. But can
we say coding styles issues are not issues in our kernel? (I guess not)

If we can not say, I guess one of your suggestion is useful (maybe be
as your suggestion): find one suitable member (I guess I am not), run
"checkpatch -file" under "./include", and fix all reported issues.

Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/28/16 07:14, Jiri Kosina wrote:
> On Sat, 27 Feb 2016, Chen Gang wrote:
> 
>>> Mel, as an MM developer, has already NACK'ed the patch, which means
>>> you should not send the patch to **any** upstream maintainer for
>>> inclusion.
>>
>> I don't think I "should not ...". I only care about correctness and
>> contribution, I don't care about any members ideas and their thinking.
>> When we have different ideas or thinking, we need discuss.
> 
> If by "discuss" you mean "30+ email thread about where to put a line 
> break", please drop me from CC next time this discussion is going to 
> happen. Thanks.
> 

Excuse me, when I sent this patch, I did not know who I shall send to, I
have to reference to "./scripts/get_maintainer.pl".

If any members have no time to care about it (every members' time are
really expensive), please let me know (can reply directly).

Thanks.

>> For common shared header files, for me, we should really take more care
>> about the coding styles.
>>
>>  - If the common shared header files don't care about the coding styles,
>>    I guess any body files will have much more excuses for "do not care
>>    about coding styles".
>>
>>  - That means our kernel whole source files need not care about coding
>>    styles at all!!
>>
>>  - It is really really VERY BAD!!
>>
>> If someone only dislike me to send the related patches, I suggest: Let
>> another member(s) "run checkpatch -file" on the whole "./include" sub-
>> directory, and fix all coding styles issues.
> 
> Which is exactly what you shouldn't do.
> 

For me, I also guess, I am not the suitable member to do that (in fact,
I dislike to do like that - "run checkpath -file" on "./include").

> The ultimate goal of the Linux kernel is not 100% strict complicance to 
> the CodingStyle document no matter what. The ultimate goal is to have a 
> kernel that is under control. By polluting git blame, you are taking on 
> aspect of the "under control" away.
> 

Yes, the ultimate goal of CodingStyle is to have a kernel that is under
control.

For me, most of files in "./include" are common, simple, and shared
files, which are not quite related with code analyzing (e.g. git log -p,
or git blame), but they are read by others in most times. Is it correct?


> Common sense needs to be used; horribly terrible coding style needs to be 
> fixed, sure. Is 82-characters long line horribly terrible coding style? 
> No, it's not.
> 

For me, what you said above have effect on body files (in kernel, at
least, more than 95% source files are body files, I guess).

But in "./include", most of files are the interface inside and outside
of our kernel, we need take more care about their coding styles.

I often use vertical split window in vim in full screen mode to reading
source code, when I read c source files, I often split window vertically
for the related header files as reference.


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/28/16 07:14, Jiri Kosina wrote:
> On Sat, 27 Feb 2016, Chen Gang wrote:
> 
>>> Mel, as an MM developer, has already NACK'ed the patch, which means
>>> you should not send the patch to **any** upstream maintainer for
>>> inclusion.
>>
>> I don't think I "should not ...". I only care about correctness and
>> contribution, I don't care about any members ideas and their thinking.
>> When we have different ideas or thinking, we need discuss.
> 
> If by "discuss" you mean "30+ email thread about where to put a line 
> break", please drop me from CC next time this discussion is going to 
> happen. Thanks.
> 

Excuse me, when I sent this patch, I did not know who I shall send to, I
have to reference to "./scripts/get_maintainer.pl".

If any members have no time to care about it (every members' time are
really expensive), please let me know (can reply directly).

Thanks.

>> For common shared header files, for me, we should really take more care
>> about the coding styles.
>>
>>  - If the common shared header files don't care about the coding styles,
>>    I guess any body files will have much more excuses for "do not care
>>    about coding styles".
>>
>>  - That means our kernel whole source files need not care about coding
>>    styles at all!!
>>
>>  - It is really really VERY BAD!!
>>
>> If someone only dislike me to send the related patches, I suggest: Let
>> another member(s) "run checkpatch -file" on the whole "./include" sub-
>> directory, and fix all coding styles issues.
> 
> Which is exactly what you shouldn't do.
> 

For me, I also guess, I am not the suitable member to do that (in fact,
I dislike to do like that - "run checkpath -file" on "./include").

> The ultimate goal of the Linux kernel is not 100% strict complicance to 
> the CodingStyle document no matter what. The ultimate goal is to have a 
> kernel that is under control. By polluting git blame, you are taking on 
> aspect of the "under control" away.
> 

Yes, the ultimate goal of CodingStyle is to have a kernel that is under
control.

For me, most of files in "./include" are common, simple, and shared
files, which are not quite related with code analyzing (e.g. git log -p,
or git blame), but they are read by others in most times. Is it correct?


> Common sense needs to be used; horribly terrible coding style needs to be 
> fixed, sure. Is 82-characters long line horribly terrible coding style? 
> No, it's not.
> 

For me, what you said above have effect on body files (in kernel, at
least, more than 95% source files are body files, I guess).

But in "./include", most of files are the interface inside and outside
of our kernel, we need take more care about their coding styles.

I often use vertical split window in vim in full screen mode to reading
source code, when I read c source files, I often split window vertically
for the related header files as reference.


Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Mel Gorman]

On Sun, Feb 28, 2016 at 08:21:40AM +0800, Chen Gang wrote:
> 
> On 2/28/16 00:53, Theodore Ts'o wrote:
> > On Sat, Feb 27, 2016 at 10:32:04PM +0800, Chen Gang wrote:
> >> I don't think so. Of cause NOT the "CODE CHURN". It is not correct to
> >> make an early decision during discussing.
> > 
> > There is no discussion.  If the maintainer has NAK'ed it.  That's the
> > end of the dicsussion.  Period.  See:
> > 
> 
> For me, NAK also needs reasons.
> 

You already got the reasons. Not only does a patch of this type interfere
with git blame which is important even in headers but I do not think the
patch actually improves the readability of the code. For example, the
comments move to the line after the defintions which to my eye at least
looks clumsy and weird.

> I guess they are related with this patch, and their NAKs' reason are: mm
> and trivial don't care about this coding style issue, is it correct?
> 

No. Coding style is important but it's a guideline not a law. There are
cases where breaking it results in perfectly readable code. At least one
my my own recent patches was flagged by checkpatch as having style issues
but fixing the style was considerably harder to read so I left it. If the
definitions in that header need to change again in the future and there
are style issues then they can be fixed in the context of a functional
change instead of patching style just for the sake of it.

-- 
Mel Gorman
SUSE Labs

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Mel Gorman]

On Sun, Feb 28, 2016 at 08:21:40AM +0800, Chen Gang wrote:
> 
> On 2/28/16 00:53, Theodore Ts'o wrote:
> > On Sat, Feb 27, 2016 at 10:32:04PM +0800, Chen Gang wrote:
> >> I don't think so. Of cause NOT the "CODE CHURN". It is not correct to
> >> make an early decision during discussing.
> > 
> > There is no discussion.  If the maintainer has NAK'ed it.  That's the
> > end of the dicsussion.  Period.  See:
> > 
> 
> For me, NAK also needs reasons.
> 

You already got the reasons. Not only does a patch of this type interfere
with git blame which is important even in headers but I do not think the
patch actually improves the readability of the code. For example, the
comments move to the line after the defintions which to my eye at least
looks clumsy and weird.

> I guess they are related with this patch, and their NAKs' reason are: mm
> and trivial don't care about this coding style issue, is it correct?
> 

No. Coding style is important but it's a guideline not a law. There are
cases where breaking it results in perfectly readable code. At least one
my my own recent patches was flagged by checkpatch as having style issues
but fixing the style was considerably harder to read so I left it. If the
definitions in that header need to change again in the future and there
are style issues then they can be fixed in the context of a functional
change instead of patching style just for the sake of it.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/28/16 21:27, Mel Gorman wrote:
> On Sun, Feb 28, 2016 at 08:21:40AM +0800, Chen Gang wrote:
>>
>> For me, NAK also needs reasons.
>>
> 
> You already got the reasons. Not only does a patch of this type interfere
> with git blame which is important even in headers but I do not think the
> patch actually improves the readability of the code. For example, the
> comments move to the line after the defintions which to my eye at least
> looks clumsy and weird.
>

For me, in local headers, they may be often modified, and also may be
complex, so the code analyzing maybe also be used often. But in common
shared headers in ./include (e.g. gfp.h), most of them are simple enough.

 - Since common shared headers are usually simple, code analyzing is
   still useful, but not like the body files or local headers (code
   analyzing are very useful for body files and local headers).
 
 - Common shared headers are quite often read by most programmers, so
   common shared headers need take more care about its coding styles.

 - Then for common shared headers, the coding style is 1st.

And for __GFP_MOVABLE definition (with ZONE_MOVABLE), I guess, we can
keep it no touch (like what I originally said: if the related member
stick to, we can keep it no touch).

And for me, the other macro definitions which out of 80 columns, can be
fixed in normal ways (let the related comments ahead of macro definition
), does this change also have negative effect?


>> I guess they are related with this patch, and their NAKs' reason are: mm
>> and trivial don't care about this coding style issue, is it correct?
>>
> 
> No. Coding style is important but it's a guideline not a law.

Yes.

For me, vertical split window in vim is very useful, I almost always use
this feature when read source code in full screen under Macbook client,
when columns are 86+, it will be wrapped (I feel really not quite good).

And occasionally (really not often), we may copy/past part of contents
in the header files (e.g. constant definition) to the pdf file as
appendix.

So except the string broken, or "grep -rn xxx * | grep yyy", 80 columns
limitation is always helpful to me.

>                                                               There are
> cases where breaking it results in perfectly readable code. At least one
> my my own recent patches was flagged by checkpatch as having style issues
> but fixing the style was considerably harder to read so I left it. If the
> definitions in that header need to change again in the future and there
> are style issues then they can be fixed in the context of a functional
> change instead of patching style just for the sake of it.
> 

For me, except just modify the related contents, usually, we need devide
the patch into 2: one for real modification, the other for coding styles.

And in some of common, base, shared headers in ./include (e.g. gfp.h), I
guess, most of contents *should* not be changed quite often, so the bad
coding styles probably will be alive in a long term.


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/28/16 21:27, Mel Gorman wrote:
> On Sun, Feb 28, 2016 at 08:21:40AM +0800, Chen Gang wrote:
>>
>> For me, NAK also needs reasons.
>>
> 
> You already got the reasons. Not only does a patch of this type interfere
> with git blame which is important even in headers but I do not think the
> patch actually improves the readability of the code. For example, the
> comments move to the line after the defintions which to my eye at least
> looks clumsy and weird.
>

For me, in local headers, they may be often modified, and also may be
complex, so the code analyzing maybe also be used often. But in common
shared headers in ./include (e.g. gfp.h), most of them are simple enough.

 - Since common shared headers are usually simple, code analyzing is
   still useful, but not like the body files or local headers (code
   analyzing are very useful for body files and local headers).
 
 - Common shared headers are quite often read by most programmers, so
   common shared headers need take more care about its coding styles.

 - Then for common shared headers, the coding style is 1st.

And for __GFP_MOVABLE definition (with ZONE_MOVABLE), I guess, we can
keep it no touch (like what I originally said: if the related member
stick to, we can keep it no touch).

And for me, the other macro definitions which out of 80 columns, can be
fixed in normal ways (let the related comments ahead of macro definition
), does this change also have negative effect?


>> I guess they are related with this patch, and their NAKs' reason are: mm
>> and trivial don't care about this coding style issue, is it correct?
>>
> 
> No. Coding style is important but it's a guideline not a law.

Yes.

For me, vertical split window in vim is very useful, I almost always use
this feature when read source code in full screen under Macbook client,
when columns are 86+, it will be wrapped (I feel really not quite good).

And occasionally (really not often), we may copy/past part of contents
in the header files (e.g. constant definition) to the pdf file as
appendix.

So except the string broken, or "grep -rn xxx * | grep yyy", 80 columns
limitation is always helpful to me.

>                                                               There are
> cases where breaking it results in perfectly readable code. At least one
> my my own recent patches was flagged by checkpatch as having style issues
> but fixing the style was considerably harder to read so I left it. If the
> definitions in that header need to change again in the future and there
> are style issues then they can be fixed in the context of a functional
> change instead of patching style just for the sake of it.
> 

For me, except just modify the related contents, usually, we need devide
the patch into 2: one for real modification, the other for coding styles.

And in some of common, base, shared headers in ./include (e.g. gfp.h), I
guess, most of contents *should* not be changed quite often, so the bad
coding styles probably will be alive in a long term.


Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Theodore Ts'o]

On Sun, Feb 28, 2016 at 08:47:23AM +0800, Chen Gang wrote:
> 
> Excuse me, when I sent this patch, I did not know who I shall send to, I
> have to reference to "./scripts/get_maintainer.pl".
> 
> If any members have no time to care about it (every members' time are
> really expensive), please let me know (can reply directly).

Yes, everybody's time is very expensive.  So why are you wasting it
all with a "last post wins" style of argumentation?  A maintainer has
NAK'ed it.  Please drop this.

There is a reason why whitespace fixes are often consider to have
extreme negative value, and a deep suspicion that people are doing
this just to say that they have a patch in the kernel, perhaps in the
misapprehension that this will help them get a job.

Let me say that if I were a hiring manager, and I did a Google search
on a potential job application, and came across a thread like this, my
reaction would be extremely negative.

					- Ted

Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles

[Chen Gang]

On 2/29/16 06:23, Theodore Ts'o wrote:
> On Sun, Feb 28, 2016 at 08:47:23AM +0800, Chen Gang wrote:
>>
>> Excuse me, when I sent this patch, I did not know who I shall send to, I
>> have to reference to "./scripts/get_maintainer.pl".
>>
>> If any members have no time to care about it (every members' time are
>> really expensive), please let me know (can reply directly).
> 
> Yes, everybody's time is very expensive.  So why are you wasting it
> all with a "last post wins" style of argumentation?  A maintainer has
> NAK'ed it.  Please drop this.
> 

For me, I don't care about "last post wins".

But I care about the technical correctness, and for me, we (all of us)
need try our best to let the email reply as correct as we can, so can
avoid to mislead another readers.

So for me, if any members have new ideas, suggestions, or completions,
they can still reply at any time (may be next day, next week, or next
month ...).


> There is a reason why whitespace fixes are often consider to have
> extreme negative value, and a deep suspicion that people are doing
> this just to say that they have a patch in the kernel, perhaps in the
> misapprehension that this will help them get a job.
> 

For me, I don't think so, at least for me, contribution and learning is
my main goal in open source community, so I mainly focus on correctness.

All of us know when some related maintainer NAK'ed, the related patch,
of cause, must be dropped, the reason why I still reply the mail is: I
shall try to make the discussion/communication as correct as I can.

For "get a job", I guess, the open source community is helpful, but I
also suggest: if someone wants to "get a job", he/she should not depend
on the open source community (community has no duty for it).


> Let me say that if I were a hiring manager, and I did a Google search
> on a potential job application, and came across a thread like this, my
> reaction would be extremely negative.
> 

For me, job hunter and HR hunter can use open source community, but the
open source community's main goal is not for job hunter or HR hunter.  I
guess, the open source community's goal should be:

 - Develop the relate product (we can treat it as urgent thing, e.g.
   new features, bug fix).

 - Learning and discussing the product related technical issues (we can
   treat it as important thing, I guess, "coding styles issues" should
   be one of these issues).

Welcome any members ideas, suggestions, and completions for it.

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.