[PATCH 1/1] bitops.h: move out get_count_order[_long]() from __KERNEL__ scope

[PATCH 1/1] bitops.h: move out get_count_order[_long]() from __KERNEL__ scope

[zijun_hu]

From: zijun_hu <zijun_hu@htc.com>

move out get_count_order[_long]() definitions from scope limited
by macro __KERNEL__

Signed-off-by: zijun_hu <zijun_hu@htc.com>
---
 this patch is based on the newest mmotm/linux-next tree and can
 be applied directly

 include/linux/bitops.h | 52 +++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 6f202c8fe4a6..a83c822c35c2 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -181,6 +181,32 @@ static inline unsigned fls_long(unsigned long l)
 	return fls64(l);
 }
 
+static inline int get_count_order(unsigned int count)
+{
+	int order;
+
+	order = fls(count) - 1;
+	if (count & (count - 1))
+		order++;
+	return order;
+}
+
+/**
+ * get_count_order_long - get order after rounding @l up to power of 2
+ * @l: parameter
+ *
+ * it is same as get_count_order() but with long type parameter
+ */
+static inline int get_count_order_long(unsigned long l)
+{
+	if (l == 0UL)
+		return -1;
+	else if (l & (l - 1UL))
+		return (int)fls_long(l);
+	else
+		return (int)fls_long(l) - 1;
+}
+
 /**
  * __ffs64 - find first set bit in a 64 bit word
  * @word: The 64 bit word
@@ -233,32 +259,6 @@ static inline unsigned long __ffs64(u64 word)
 })
 #endif
 
-static inline int get_count_order(unsigned int count)
-{
-	int order;
-
-	order = fls(count) - 1;
-	if (count & (count - 1))
-		order++;
-	return order;
-}
-
-/**
- * get_count_order_long - get order after rounding @l up to power of 2
- * @l: parameter
- *
- * it is same as get_count_order() but with long type parameter
- */
-static inline int get_count_order_long(unsigned long l)
-{
-	if (l == 0UL)
-		return -1;
-	else if (l & (l - 1UL))
-		return (int)fls_long(l);
-	else
-		return (int)fls_long(l) - 1;
-}
-
 #ifndef find_last_bit
 /**
  * find_last_bit - find the last set bit in a memory region
-- 
1.9.1

Re: [PATCH 1/1] bitops.h: move out get_count_order[_long]() from __KERNEL__ scope

[Stephen Rothwell]

Hi,

On Tue, 16 Aug 2016 14:27:04 +0800 zijun_hu <zijun_hu@zoho.com> wrote:
>
> From: zijun_hu <zijun_hu@htc.com>
> 
> move out get_count_order[_long]() definitions from scope limited
> by macro __KERNEL__

Why do you need to do this?  You say why in the commit message.

-- 
Cheers,
Stephen Rothwell

Re: [PATCH 1/1] bitops.h: move out get_count_order[_long]() from __KERNEL__ scope

[zijun_hu]

On 08/16/2016 02:50 PM, Stephen Rothwell wrote:
> Hi,
> 
> On Tue, 16 Aug 2016 14:27:04 +0800 zijun_hu <zijun_hu@zoho.com> wrote:
>>
>> From: zijun_hu <zijun_hu@htc.com>
>>
>> move out get_count_order[_long]() definitions from scope limited
>> by macro __KERNEL__
> 
> Why do you need to do this?  You say why in the commit message.
> 
1, it make both functions available in wider region regardless of
   whether macro __KERNEL__ is defined
2, get_count_order() locates out off region limited by macro __KERNEL__
   before the recent commit c513b4cd2fe9
   ("mm-vmalloc-fix-align-value-calculation-error-v2-fix-fix")
   it maybe more perfect to keep its original region and place its counterpart
   get_count_order_long() nearly

thanks for your reply

[RESEND PATCH 1/1] bitops.h: move out get_count_order[_long]() from __KERNEL__ scope

[zijun_hu]

From: zijun_hu <zijun_hu@htc.com>

move out get_count_order[_long]() definitions from scope limited
by macro __KERNEL__

it not only make both functions available in wider region regardless
of whether __KERNEL__ is defined but also keep original region for
get_count_order() before the recent commit c513b4cd2fe9
("mm-vmalloc-fix-align-value-calculation-error-v2-fix-fix") 

Signed-off-by: zijun_hu <zijun_hu@htc.com>
---
 this patch is based on the newest mmotm/linux-next tree and can
 be applied directly

 include/linux/bitops.h | 52 +++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 6f202c8fe4a6..a83c822c35c2 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -181,6 +181,32 @@ static inline unsigned fls_long(unsigned long l)
 	return fls64(l);
 }
 
+static inline int get_count_order(unsigned int count)
+{
+	int order;
+
+	order = fls(count) - 1;
+	if (count & (count - 1))
+		order++;
+	return order;
+}
+
+/**
+ * get_count_order_long - get order after rounding @l up to power of 2
+ * @l: parameter
+ *
+ * it is same as get_count_order() but with long type parameter
+ */
+static inline int get_count_order_long(unsigned long l)
+{
+	if (l == 0UL)
+		return -1;
+	else if (l & (l - 1UL))
+		return (int)fls_long(l);
+	else
+		return (int)fls_long(l) - 1;
+}
+
 /**
  * __ffs64 - find first set bit in a 64 bit word
  * @word: The 64 bit word
@@ -233,32 +259,6 @@ static inline unsigned long __ffs64(u64 word)
 })
 #endif
 
-static inline int get_count_order(unsigned int count)
-{
-	int order;
-
-	order = fls(count) - 1;
-	if (count & (count - 1))
-		order++;
-	return order;
-}
-
-/**
- * get_count_order_long - get order after rounding @l up to power of 2
- * @l: parameter
- *
- * it is same as get_count_order() but with long type parameter
- */
-static inline int get_count_order_long(unsigned long l)
-{
-	if (l == 0UL)
-		return -1;
-	else if (l & (l - 1UL))
-		return (int)fls_long(l);
-	else
-		return (int)fls_long(l) - 1;
-}
-
 #ifndef find_last_bit
 /**
  * find_last_bit - find the last set bit in a memory region
-- 
1.9.1

Re: [RESEND PATCH 1/1] bitops.h: move out get_count_order[_long]() from __KERNEL__ scope

[Al Viro]

On Tue, Aug 16, 2016 at 03:46:22PM +0800, zijun_hu wrote:
> From: zijun_hu <zijun_hu@htc.com>
> 
> move out get_count_order[_long]() definitions from scope limited
> by macro __KERNEL__
> 
> it not only make both functions available in wider region regardless
> of whether __KERNEL__ is defined but also keep original region for
> get_count_order() before the recent commit c513b4cd2fe9
> ("mm-vmalloc-fix-align-value-calculation-error-v2-fix-fix") 

What the hell is anything without __KERNEL__ doing with linux/bitops.h in
the first place?  IOW, why do we have those ifdefs at all?

Re: [RESEND PATCH 1/1] bitops.h: move out get_count_order[_long]() from __KERNEL__ scope

[zijun_hu]

On 2016/8/18 1:20, Al Viro wrote:
> On Tue, Aug 16, 2016 at 03:46:22PM +0800, zijun_hu wrote:
>> From: zijun_hu <zijun_hu@htc.com>
>>
>> move out get_count_order[_long]() definitions from scope limited
>> by macro __KERNEL__
>>
>> it not only make both functions available in wider region regardless
>> of whether __KERNEL__ is defined but also keep original region for
>> get_count_order() before the recent commit c513b4cd2fe9
>> ("mm-vmalloc-fix-align-value-calculation-error-v2-fix-fix") 
> 
> What the hell is anything without __KERNEL__ doing with linux/bitops.h in
> the first place?  IOW, why do we have those ifdefs at all?
> 

__KERNEL__ is used to indicate the relevant sections within kernel
headers can't be exported to or used by user space

let me illuminate this patch background firstly
i and Andrew develop another patch to fix a mm issue recently, that
patch move get_count_order() into __KERNEL__ scope, so touch the
function's scope property we don't need to touch or care

the aim of this patch is undoing our unnecessary changes

Re: [RESEND PATCH 1/1] bitops.h: move out get_count_order[_long]() from __KERNEL__ scope

[Al Viro]

On Thu, Aug 18, 2016 at 07:51:19AM +0800, zijun_hu wrote:
> > What the hell is anything without __KERNEL__ doing with linux/bitops.h in
> > the first place?  IOW, why do we have those ifdefs at all?
> > 
> 
> __KERNEL__ is used to indicate the relevant sections within kernel
> headers can't be exported to or used by user space

ITYM "used to be used".  These days it's "everything outside of */uapi/*.h
can't be exported"...

Re: [RESEND PATCH 1/1] bitops.h: move out get_count_order[_long]() from __KERNEL__ scope

[zijun_hu]

On 2016/8/18 7:59, Al Viro wrote:
> On Thu, Aug 18, 2016 at 07:51:19AM +0800, zijun_hu wrote:
>>> What the hell is anything without __KERNEL__ doing with linux/bitops.h in
>>> the first place?  IOW, why do we have those ifdefs at all?
>>>
>>
>> __KERNEL__ is used to indicate the relevant sections within kernel
>> headers can't be exported to or used by user space
> 
> ITYM "used to be used".  These days it's "everything outside of */uapi/*.h
> can't be exported"...
> 
i conclude one purpose of __KERNEL__ from the following kernel files

scripts/headers_install.sh:
echo "Usage: headers_install.sh OUTDIR SRCDIR [FILES...]"
echo
echo "Prepares kernel header files for use by user space, by removing"
echo "all compiler.h definitions and #includes, removing any"
echo "#ifdef __KERNEL__ sections, and putting __underscores__ around"
echo "asm/inline/volatile keywords."
echo
......
scripts/unifdef -U__KERNEL__ -D__EXPORTED_HEADERS__ "$OUTDIR/$FILE.sed"

Documentation/kbuild/makefiles.txt：
The kernel includes a set of headers that is exported to userspace.
Many headers can be exported as-is but other headers require a
minimal pre-processing before they are ready for user-space.
The pre-processing does:
- drop kernel-specific annotations
- drop include of compiler.h
- drop all sections that are kernel internal (guarded by ifdef __KERNEL__)

scripts/Makefile.headersinst:
# ==========================================================================
# Installing headers
#
# header-y  - list files to be installed. They are preprocessed
#             to remove __KERNEL__ section of the file
# genhdr-y  - Same as header-y but in a generated/ directory
#
# ==========================================================================

Re: [RESEND PATCH 1/1] bitops.h: move out get_count_order[_long]() from __KERNEL__ scope

[Al Viro]

On Thu, Aug 18, 2016 at 08:10:19AM +0800, zijun_hu wrote:

> Documentation/kbuild/makefiles.txt：
> The kernel includes a set of headers that is exported to userspace.
> Many headers can be exported as-is but other headers require a
> minimal pre-processing before they are ready for user-space.
> The pre-processing does:
> - drop kernel-specific annotations
> - drop include of compiler.h
> - drop all sections that are kernel internal (guarded by ifdef __KERNEL__)
> 
> scripts/Makefile.headersinst:
> # ==========================================================================
> # Installing headers
> #
> # header-y  - list files to be installed. They are preprocessed
> #             to remove __KERNEL__ section of the file
> # genhdr-y  - Same as header-y but in a generated/ directory

Quite.  Now, could you show me where bitops.h is mentioned in any header-y
assignments?

Stuff outside of */uapi/* is not exported at all.

Re: [RESEND PATCH 1/1] bitops.h: move out get_count_order[_long]() from __KERNEL__ scope

[zijun_hu]

On 2016/8/18 8:28, Al Viro wrote:
> On Thu, Aug 18, 2016 at 08:10:19AM +0800, zijun_hu wrote:
> 
>> Documentation/kbuild/makefiles.txt：
>> The kernel includes a set of headers that is exported to userspace.
>> Many headers can be exported as-is but other headers require a
>> minimal pre-processing before they are ready for user-space.
>> The pre-processing does:
>> - drop kernel-specific annotations
>> - drop include of compiler.h
>> - drop all sections that are kernel internal (guarded by ifdef __KERNEL__)
>>
>> scripts/Makefile.headersinst:
>> # ==========================================================================
>> # Installing headers
>> #
>> # header-y  - list files to be installed. They are preprocessed
>> #             to remove __KERNEL__ section of the file
>> # genhdr-y  - Same as header-y but in a generated/ directory
> 
> Quite.  Now, could you show me where bitops.h is mentioned in any header-y
> assignments?
> 
you are right bitops.h isn't mentioned in any header-y
my aim is making macro __KERNEL__ purpose or history purpose clear
it isn't a bad thing to follow __KERNEL__ purpose
it maybe look nicer to keep get_counter_order() original scope 

that don't matter since the current scope works well also
> Stuff outside of */uapi/* is not exported at all.
>