linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: Define KB, MB, GB, TB in core VM
@ 2017-05-22 11:17 Anshuman Khandual
  2017-05-22 21:11 ` Andrew Morton
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Anshuman Khandual @ 2017-05-22 11:17 UTC (permalink / raw)
  To: linux-mm, linux-kernel; +Cc: akpm

There are many places where we define size either left shifting integers
or multiplying 1024s without any generic definition to fall back on. But
there are couples of (powerpc and lz4) attempts to define these standard
memory sizes. Lets move these definitions to core VM to make sure that
all new usage come from these definitions eventually standardizing it
across all places.

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
 arch/powerpc/mm/hash_utils_64.c | 4 ----
 include/linux/mm.h              | 5 +++++
 lib/lz4/lz4defs.h               | 5 +----
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index f2095ce..ef64040 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -74,10 +74,6 @@
 #define DBG_LOW(fmt...)
 #endif
 
-#define KB (1024)
-#define MB (1024*KB)
-#define GB (1024L*MB)
-
 /*
  * Note:  pte   --> Linux PTE
  *        HPTE  --> PowerPC Hashed Page Table Entry
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7cb17c6..9f5779f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2549,5 +2549,10 @@ static inline bool page_is_guard(struct page *page)
 static inline void setup_nr_node_ids(void) {}
 #endif
 
+#define KB (1UL << 10)
+#define MB (1UL << 20)
+#define GB (1UL << 30)
+#define TB (1UL << 40)
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
diff --git a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h
index 00a0b58..67a0f6d 100644
--- a/lib/lz4/lz4defs.h
+++ b/lib/lz4/lz4defs.h
@@ -37,6 +37,7 @@
 
 #include <asm/unaligned.h>
 #include <linux/string.h>	 /* memset, memcpy */
+#include <linux/mm.h>
 
 #define FORCE_INLINE __always_inline
 
@@ -81,10 +82,6 @@
 
 #define HASH_UNIT sizeof(size_t)
 
-#define KB (1 << 10)
-#define MB (1 << 20)
-#define GB (1U << 30)
-
 #define MAXD_LOG 16
 #define MAX_DISTANCE ((1 << MAXD_LOG) - 1)
 #define STEPSIZE sizeof(size_t)
-- 
1.8.5.2

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-22 11:17 [PATCH] mm: Define KB, MB, GB, TB in core VM Anshuman Khandual
@ 2017-05-22 21:11 ` Andrew Morton
  2017-05-23  7:02   ` Christoph Hellwig
  2017-05-23 11:13   ` Anshuman Khandual
  2017-05-23  6:41 ` kbuild test robot
  2017-05-23  7:24 ` kbuild test robot
  2 siblings, 2 replies; 14+ messages in thread
From: Andrew Morton @ 2017-05-22 21:11 UTC (permalink / raw)
  To: Anshuman Khandual; +Cc: linux-mm, linux-kernel

On Mon, 22 May 2017 16:47:42 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:

> There are many places where we define size either left shifting integers
> or multiplying 1024s without any generic definition to fall back on. But
> there are couples of (powerpc and lz4) attempts to define these standard
> memory sizes. Lets move these definitions to core VM to make sure that
> all new usage come from these definitions eventually standardizing it
> across all places.

Grep further - there are many more definitions and some may now
generate warnings.

Newly including mm.h for these things seems a bit heavyweight.  I can't
immediately think of a more appropriate place.  Maybe printk.h or
kernel.h.

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-22 11:17 [PATCH] mm: Define KB, MB, GB, TB in core VM Anshuman Khandual
  2017-05-22 21:11 ` Andrew Morton
@ 2017-05-23  6:41 ` kbuild test robot
  2017-05-23  7:24 ` kbuild test robot
  2 siblings, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2017-05-23  6:41 UTC (permalink / raw)
  To: Anshuman Khandual; +Cc: kbuild-all, linux-mm, linux-kernel, akpm

[-- Attachment #1: Type: text/plain, Size: 3969 bytes --]

Hi Anshuman,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.12-rc2 next-20170522]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Anshuman-Khandual/mm-Define-KB-MB-GB-TB-in-core-VM/20170523-141359
config: i386-tinyconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

>> arch/x86/kernel/cpu/intel_cacheinfo.c:34:0: warning: "MB" redefined
    #define MB(x) ((x) * 1024)
    
   In file included from arch/x86/include/asm/pci.h:4:0,
                    from include/linux/pci.h:1618,
                    from arch/x86/kernel/cpu/intel_cacheinfo.c:16:
   include/linux/mm.h:2553:0: note: this is the location of the previous definition
    #define MB (1UL << 20)
    

vim +/MB +34 arch/x86/kernel/cpu/intel_cacheinfo.c

cd4d09ec arch/x86/kernel/cpu/intel_cacheinfo.c  Borislav Petkov  2016-01-26  18  #include <asm/cpufeature.h>
23ac4ae8 arch/x86/kernel/cpu/intel_cacheinfo.c  Andreas Herrmann 2010-09-17  19  #include <asm/amd_nb.h>
dcf39daf arch/x86/kernel/cpu/intel_cacheinfo.c  Borislav Petkov  2010-01-22  20  #include <asm/smp.h>
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  21  
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  22  #define LVL_1_INST	1
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  23  #define LVL_1_DATA	2
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  24  #define LVL_2		3
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  25  #define LVL_3		4
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  26  #define LVL_TRACE	5
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  27  
8bdbd962 arch/x86/kernel/cpu/intel_cacheinfo.c  Alan Cox         2009-07-04  28  struct _cache_table {
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  29  	unsigned char descriptor;
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  30  	char cache_type;
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  31  	short size;
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  32  };
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  33  
2ca49b2f arch/x86/kernel/cpu/intel_cacheinfo.c  Dave Jones       2010-01-04 @34  #define MB(x)	((x) * 1024)
2ca49b2f arch/x86/kernel/cpu/intel_cacheinfo.c  Dave Jones       2010-01-04  35  
8bdbd962 arch/x86/kernel/cpu/intel_cacheinfo.c  Alan Cox         2009-07-04  36  /* All the cache descriptor types we care about (no TLB or
8bdbd962 arch/x86/kernel/cpu/intel_cacheinfo.c  Alan Cox         2009-07-04  37     trace cache entries) */
8bdbd962 arch/x86/kernel/cpu/intel_cacheinfo.c  Alan Cox         2009-07-04  38  
148f9bb8 arch/x86/kernel/cpu/intel_cacheinfo.c  Paul Gortmaker   2013-06-18  39  static const struct _cache_table cache_table[] =
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  40  {
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  41  	{ 0x06, LVL_1_INST, 8 },	/* 4-way set assoc, 32 byte line size */
^1da177e arch/i386/kernel/cpu/intel_cacheinfo.c Linus Torvalds   2005-04-16  42  	{ 0x08, LVL_1_INST, 16 },	/* 4-way set assoc, 32 byte line size */

:::::: The code at line 34 was first introduced by commit
:::::: 2ca49b2fcf5813571663c3c4c894b78148c43690 x86: Macroise x86 cache descriptors

:::::: TO: Dave Jones <davej@redhat.com>
:::::: CC: Ingo Molnar <mingo@elte.hu>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6567 bytes --]

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-22 21:11 ` Andrew Morton
@ 2017-05-23  7:02   ` Christoph Hellwig
  2017-05-23  8:38     ` Vlastimil Babka
  2017-05-23 11:13   ` Anshuman Khandual
  1 sibling, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2017-05-23  7:02 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Anshuman Khandual, linux-mm, linux-kernel

On Mon, May 22, 2017 at 02:11:49PM -0700, Andrew Morton wrote:
> On Mon, 22 May 2017 16:47:42 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
> 
> > There are many places where we define size either left shifting integers
> > or multiplying 1024s without any generic definition to fall back on. But
> > there are couples of (powerpc and lz4) attempts to define these standard
> > memory sizes. Lets move these definitions to core VM to make sure that
> > all new usage come from these definitions eventually standardizing it
> > across all places.
> 
> Grep further - there are many more definitions and some may now
> generate warnings.
> 
> Newly including mm.h for these things seems a bit heavyweight.  I can't
> immediately think of a more appropriate place.  Maybe printk.h or
> kernel.h.

IFF we do these kernel.h is the right place.  And please also add the
MiB & co variants for the binary versions right next to the decimal
ones.

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-22 11:17 [PATCH] mm: Define KB, MB, GB, TB in core VM Anshuman Khandual
  2017-05-22 21:11 ` Andrew Morton
  2017-05-23  6:41 ` kbuild test robot
@ 2017-05-23  7:24 ` kbuild test robot
  2 siblings, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2017-05-23  7:24 UTC (permalink / raw)
  To: Anshuman Khandual; +Cc: kbuild-all, linux-mm, linux-kernel, akpm

[-- Attachment #1: Type: text/plain, Size: 4206 bytes --]

Hi Anshuman,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.12-rc2 next-20170523]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Anshuman-Khandual/mm-Define-KB-MB-GB-TB-in-core-VM/20170523-141359
config: x86_64-nfsroot+CONFIG_DEBUG_INFO_REDUCED (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from drivers/char/agp/backend.c:41:0:
>> drivers/char/agp/agp.h:158:0: warning: "KB" redefined
    #define KB(x) ((x) * 1024)
    
   In file included from include/linux/scatterlist.h:7:0,
                    from include/linux/dmapool.h:14,
                    from include/linux/pci.h:1281,
                    from drivers/char/agp/backend.c:31:
   include/linux/mm.h:2552:0: note: this is the location of the previous definition
    #define KB (1UL << 10)
    
   In file included from drivers/char/agp/backend.c:41:0:
>> drivers/char/agp/agp.h:159:0: warning: "MB" redefined
    #define MB(x) (KB (KB (x)))
    
   In file included from include/linux/scatterlist.h:7:0,
                    from include/linux/dmapool.h:14,
                    from include/linux/pci.h:1281,
                    from drivers/char/agp/backend.c:31:
   include/linux/mm.h:2553:0: note: this is the location of the previous definition
    #define MB (1UL << 20)
    
   In file included from drivers/char/agp/backend.c:41:0:
>> drivers/char/agp/agp.h:160:0: warning: "GB" redefined
    #define GB(x) (MB (KB (x)))
    
   In file included from include/linux/scatterlist.h:7:0,
                    from include/linux/dmapool.h:14,
                    from include/linux/pci.h:1281,
                    from drivers/char/agp/backend.c:31:
   include/linux/mm.h:2554:0: note: this is the location of the previous definition
    #define GB (1UL << 30)
    
--
>> drivers/gpu/drm/i915/i915_gem_stolen.c:33:0: warning: "KB" redefined
    #define KB(x) ((x) * 1024)
    
   In file included from include/linux/scatterlist.h:7:0,
                    from include/linux/dma-mapping.h:10,
                    from include/drm/drmP.h:37,
                    from drivers/gpu/drm/i915/i915_gem_stolen.c:29:
   include/linux/mm.h:2552:0: note: this is the location of the previous definition
    #define KB (1UL << 10)
    
>> drivers/gpu/drm/i915/i915_gem_stolen.c:34:0: warning: "MB" redefined
    #define MB(x) (KB(x) * 1024)
    
   In file included from include/linux/scatterlist.h:7:0,
                    from include/linux/dma-mapping.h:10,
                    from include/drm/drmP.h:37,
                    from drivers/gpu/drm/i915/i915_gem_stolen.c:29:
   include/linux/mm.h:2553:0: note: this is the location of the previous definition
    #define MB (1UL << 20)
    

vim +/KB +158 drivers/char/agp/agp.h

b0825488 Matthew Garrett 2005-07-29  152  	u32 apbase_config;
a8c84df9 Keith Packard   2008-07-31  153  	/* list of agp_memory mapped to the aperture */
a8c84df9 Keith Packard   2008-07-31  154  	struct list_head mapped_list;
a8c84df9 Keith Packard   2008-07-31  155  	spinlock_t mapped_lock;
^1da177e Linus Torvalds  2005-04-16  156  };
^1da177e Linus Torvalds  2005-04-16  157  
^1da177e Linus Torvalds  2005-04-16 @158  #define KB(x)	((x) * 1024)
^1da177e Linus Torvalds  2005-04-16 @159  #define MB(x)	(KB (KB (x)))
^1da177e Linus Torvalds  2005-04-16 @160  #define GB(x)	(MB (KB (x)))
^1da177e Linus Torvalds  2005-04-16  161  
^1da177e Linus Torvalds  2005-04-16  162  #define A_SIZE_8(x)	((struct aper_size_info_8 *) x)
^1da177e Linus Torvalds  2005-04-16  163  #define A_SIZE_16(x)	((struct aper_size_info_16 *) x)

:::::: The code at line 158 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28961 bytes --]

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-23  7:02   ` Christoph Hellwig
@ 2017-05-23  8:38     ` Vlastimil Babka
  2017-05-23  8:40       ` Christoph Hellwig
  2017-05-23 11:19       ` Anshuman Khandual
  0 siblings, 2 replies; 14+ messages in thread
From: Vlastimil Babka @ 2017-05-23  8:38 UTC (permalink / raw)
  To: Christoph Hellwig, Andrew Morton
  Cc: Anshuman Khandual, linux-mm, linux-kernel

On 05/23/2017 09:02 AM, Christoph Hellwig wrote:
> On Mon, May 22, 2017 at 02:11:49PM -0700, Andrew Morton wrote:
>> On Mon, 22 May 2017 16:47:42 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
>>
>>> There are many places where we define size either left shifting integers
>>> or multiplying 1024s without any generic definition to fall back on. But
>>> there are couples of (powerpc and lz4) attempts to define these standard
>>> memory sizes. Lets move these definitions to core VM to make sure that
>>> all new usage come from these definitions eventually standardizing it
>>> across all places.
>>
>> Grep further - there are many more definitions and some may now
>> generate warnings.
>>
>> Newly including mm.h for these things seems a bit heavyweight.  I can't
>> immediately think of a more appropriate place.  Maybe printk.h or
>> kernel.h.
> 
> IFF we do these kernel.h is the right place.  And please also add the
> MiB & co variants for the binary versions right next to the decimal
> ones.

Those defined in the patch are binary, not decimal. Do we even need
decimal ones?

> --
> 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>
> 

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-23  8:38     ` Vlastimil Babka
@ 2017-05-23  8:40       ` Christoph Hellwig
  2017-05-23 11:19       ` Anshuman Khandual
  1 sibling, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2017-05-23  8:40 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Christoph Hellwig, Andrew Morton, Anshuman Khandual, linux-mm,
	linux-kernel

On Tue, May 23, 2017 at 10:38:17AM +0200, Vlastimil Babka wrote:
> Those defined in the patch are binary, not decimal. Do we even need
> decimal ones?

Oh, good point.  In which case the names should change to avoid the
confusion.

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-22 21:11 ` Andrew Morton
  2017-05-23  7:02   ` Christoph Hellwig
@ 2017-05-23 11:13   ` Anshuman Khandual
  1 sibling, 0 replies; 14+ messages in thread
From: Anshuman Khandual @ 2017-05-23 11:13 UTC (permalink / raw)
  To: Andrew Morton, Anshuman Khandual; +Cc: linux-mm, linux-kernel

On 05/23/2017 02:41 AM, Andrew Morton wrote:
> On Mon, 22 May 2017 16:47:42 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
> 
>> There are many places where we define size either left shifting integers
>> or multiplying 1024s without any generic definition to fall back on. But
>> there are couples of (powerpc and lz4) attempts to define these standard
>> memory sizes. Lets move these definitions to core VM to make sure that
>> all new usage come from these definitions eventually standardizing it
>> across all places.
> Grep further - there are many more definitions and some may now
> generate warnings.

Yeah, warning reports started coming in. Will try to change
all of those to follow the new definitions added.

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-23  8:38     ` Vlastimil Babka
  2017-05-23  8:40       ` Christoph Hellwig
@ 2017-05-23 11:19       ` Anshuman Khandual
  2017-05-24  6:40         ` Anshuman Khandual
  1 sibling, 1 reply; 14+ messages in thread
From: Anshuman Khandual @ 2017-05-23 11:19 UTC (permalink / raw)
  To: Vlastimil Babka, Christoph Hellwig, Andrew Morton
  Cc: Anshuman Khandual, linux-mm, linux-kernel

On 05/23/2017 02:08 PM, Vlastimil Babka wrote:
> On 05/23/2017 09:02 AM, Christoph Hellwig wrote:
>> On Mon, May 22, 2017 at 02:11:49PM -0700, Andrew Morton wrote:
>>> On Mon, 22 May 2017 16:47:42 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
>>>
>>>> There are many places where we define size either left shifting integers
>>>> or multiplying 1024s without any generic definition to fall back on. But
>>>> there are couples of (powerpc and lz4) attempts to define these standard
>>>> memory sizes. Lets move these definitions to core VM to make sure that
>>>> all new usage come from these definitions eventually standardizing it
>>>> across all places.
>>> Grep further - there are many more definitions and some may now
>>> generate warnings.
>>>
>>> Newly including mm.h for these things seems a bit heavyweight.  I can't
>>> immediately think of a more appropriate place.  Maybe printk.h or
>>> kernel.h.
>> IFF we do these kernel.h is the right place.  And please also add the
>> MiB & co variants for the binary versions right next to the decimal
>> ones.
> Those defined in the patch are binary, not decimal. Do we even need
> decimal ones?
> 

I can define KiB, MiB, .... with the same values as binary.
Did not get about the decimal ones, we need different names
for them holding values which are multiple of 1024 ?

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-23 11:19       ` Anshuman Khandual
@ 2017-05-24  6:40         ` Anshuman Khandual
  2017-05-24 14:31           ` Michal Hocko
                             ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Anshuman Khandual @ 2017-05-24  6:40 UTC (permalink / raw)
  To: Anshuman Khandual, Vlastimil Babka, Christoph Hellwig, Andrew Morton
  Cc: linux-mm, linux-kernel

On 05/23/2017 04:49 PM, Anshuman Khandual wrote:
> On 05/23/2017 02:08 PM, Vlastimil Babka wrote:
>> On 05/23/2017 09:02 AM, Christoph Hellwig wrote:
>>> On Mon, May 22, 2017 at 02:11:49PM -0700, Andrew Morton wrote:
>>>> On Mon, 22 May 2017 16:47:42 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
>>>>
>>>>> There are many places where we define size either left shifting integers
>>>>> or multiplying 1024s without any generic definition to fall back on. But
>>>>> there are couples of (powerpc and lz4) attempts to define these standard
>>>>> memory sizes. Lets move these definitions to core VM to make sure that
>>>>> all new usage come from these definitions eventually standardizing it
>>>>> across all places.
>>>> Grep further - there are many more definitions and some may now
>>>> generate warnings.
>>>>
>>>> Newly including mm.h for these things seems a bit heavyweight.  I can't
>>>> immediately think of a more appropriate place.  Maybe printk.h or
>>>> kernel.h.
>>> IFF we do these kernel.h is the right place.  And please also add the
>>> MiB & co variants for the binary versions right next to the decimal
>>> ones.
>> Those defined in the patch are binary, not decimal. Do we even need
>> decimal ones?
>>
> 
> I can define KiB, MiB, .... with the same values as binary.
> Did not get about the decimal ones, we need different names
> for them holding values which are multiple of 1024 ?

Now it seems little bit complicated than I initially thought.
There are three different kind of definitions scattered across
the tree.

(1) Constant defines like these which can be unified across
    with little effort.

+#define KB (1UL << 10)
+#define MB (1UL << 20)
+#define GB (1UL << 30)
+#define TB (1UL << 40)

(2) Function type defines like these which need to be renamed
    first because of the static defines already added above.

#define KB(x) ((x) * 1024)
#define MB(x) (KB(x) * 1024)

    Does these sound good as a rename ?

+#define KBN(x) ((x) * KB)
+#define MBN(x) ((x) * MB)
+#define GBN(x) ((x) * GB)
+#define TBN(x) ((x) * TB)

    And these need to be replaced across the tree.

(3) Then there are many defines for MB, KB, GB which have nothing
    to do with memory size and they need to be changed as well to
    something else more appropriately to something they actually
    do.

#define MB CRB

* Defined inside arch/powerpc/xmon/ppc-opc.c

#define GB(p,n,s) gf2k_get_bits(data, p, n, s)

* Defined inside drivers/input/joystick/gf2k.c

#define GB(pos,num) sw_get_bits(buf, pos, num, sw->bits)

* Defined inside drivers/input/joystick/sidewinder.c 

So the question is are we willing to do all these changes across
the tree to achieve common definitions of KB, MB, GB, TB in the
kernel ? Is it worth ?

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-24  6:40         ` Anshuman Khandual
@ 2017-05-24 14:31           ` Michal Hocko
  2017-05-29 10:55           ` Michael Ellerman
  2017-05-29 11:07           ` Geert Uytterhoeven
  2 siblings, 0 replies; 14+ messages in thread
From: Michal Hocko @ 2017-05-24 14:31 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Vlastimil Babka, Christoph Hellwig, Andrew Morton, linux-mm,
	linux-kernel

On Wed 24-05-17 12:10:13, Anshuman Khandual wrote:
[...]
> So the question is are we willing to do all these changes across
> the tree to achieve common definitions of KB, MB, GB, TB in the
> kernel ? Is it worth ?

I do not think this is worth losing time. Any tree wide change should
have a considerable advantage in the end. These macro helpers do not
sound overly important to care. But that is just my 2c
-- 
Michal Hocko
SUSE Labs

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-24  6:40         ` Anshuman Khandual
  2017-05-24 14:31           ` Michal Hocko
@ 2017-05-29 10:55           ` Michael Ellerman
  2017-06-09  2:54             ` Anshuman Khandual
  2017-05-29 11:07           ` Geert Uytterhoeven
  2 siblings, 1 reply; 14+ messages in thread
From: Michael Ellerman @ 2017-05-29 10:55 UTC (permalink / raw)
  To: Anshuman Khandual, Anshuman Khandual, Vlastimil Babka,
	Christoph Hellwig, Andrew Morton
  Cc: linux-mm, linux-kernel

Anshuman Khandual <khandual@linux.vnet.ibm.com> writes:
>
> So the question is are we willing to do all these changes across
> the tree to achieve common definitions of KB, MB, GB, TB in the
> kernel ? Is it worth ?

No I don't think it's worth the churn.

But have you looked at using the "proper" names, ie. KiB, MiB, GiB?

AFAICS the only clash is:

drivers/mtd/ssfdc.c:#define KiB(x)	( (x) * 1024L )
drivers/mtd/ssfdc.c:#define MiB(x)	( KiB(x) * 1024L )

Which would be easy to convert.

cheers

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-24  6:40         ` Anshuman Khandual
  2017-05-24 14:31           ` Michal Hocko
  2017-05-29 10:55           ` Michael Ellerman
@ 2017-05-29 11:07           ` Geert Uytterhoeven
  2 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2017-05-29 11:07 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Vlastimil Babka, Christoph Hellwig, Andrew Morton, Linux MM,
	linux-kernel

Hi Anshuman,

On Wed, May 24, 2017 at 8:40 AM, Anshuman Khandual
<khandual@linux.vnet.ibm.com> wrote:
> On 05/23/2017 04:49 PM, Anshuman Khandual wrote:
>> On 05/23/2017 02:08 PM, Vlastimil Babka wrote:
>>> On 05/23/2017 09:02 AM, Christoph Hellwig wrote:
>>>> On Mon, May 22, 2017 at 02:11:49PM -0700, Andrew Morton wrote:
>>>>> On Mon, 22 May 2017 16:47:42 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
>>>>>> There are many places where we define size either left shifting integers
>>>>>> or multiplying 1024s without any generic definition to fall back on. But
>>>>>> there are couples of (powerpc and lz4) attempts to define these standard
>>>>>> memory sizes. Lets move these definitions to core VM to make sure that
>>>>>> all new usage come from these definitions eventually standardizing it
>>>>>> across all places.
>>>>> Grep further - there are many more definitions and some may now
>>>>> generate warnings.
>>>>>
>>>>> Newly including mm.h for these things seems a bit heavyweight.  I can't
>>>>> immediately think of a more appropriate place.  Maybe printk.h or
>>>>> kernel.h.
>>>> IFF we do these kernel.h is the right place.  And please also add the
>>>> MiB & co variants for the binary versions right next to the decimal
>>>> ones.
>>> Those defined in the patch are binary, not decimal. Do we even need
>>> decimal ones?
>>
>> I can define KiB, MiB, .... with the same values as binary.
>> Did not get about the decimal ones, we need different names
>> for them holding values which are multiple of 1024 ?
>
> Now it seems little bit complicated than I initially thought.
> There are three different kind of definitions scattered across
> the tree.
>
> (1) Constant defines like these which can be unified across
>     with little effort.
>
> +#define KB (1UL << 10)
> +#define MB (1UL << 20)
> +#define GB (1UL << 30)
> +#define TB (1UL << 40)

Please don't add more/generalize (ab)users of decimal prefixes where
binary prefixes are needed/intended.

https://en.wikipedia.org/wiki/Binary_prefix

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
  2017-05-29 10:55           ` Michael Ellerman
@ 2017-06-09  2:54             ` Anshuman Khandual
  0 siblings, 0 replies; 14+ messages in thread
From: Anshuman Khandual @ 2017-06-09  2:54 UTC (permalink / raw)
  To: Michael Ellerman, Anshuman Khandual, Vlastimil Babka,
	Christoph Hellwig, Andrew Morton
  Cc: linux-mm, linux-kernel

On 05/29/2017 04:25 PM, Michael Ellerman wrote:
> Anshuman Khandual <khandual@linux.vnet.ibm.com> writes:
>>
>> So the question is are we willing to do all these changes across
>> the tree to achieve common definitions of KB, MB, GB, TB in the
>> kernel ? Is it worth ?
> 
> No I don't think it's worth the churn.
> 
> But have you looked at using the "proper" names, ie. KiB, MiB, GiB?
> 
> AFAICS the only clash is:
> 
> drivers/mtd/ssfdc.c:#define KiB(x)	( (x) * 1024L )
> drivers/mtd/ssfdc.c:#define MiB(x)	( KiB(x) * 1024L )
> 
> Which would be easy to convert.

Sure, will take a look into generalizing KiB/MiB/GiB instead of
current proposal for KB/MB/GB.

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

end of thread, other threads:[~2017-06-09  2:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-22 11:17 [PATCH] mm: Define KB, MB, GB, TB in core VM Anshuman Khandual
2017-05-22 21:11 ` Andrew Morton
2017-05-23  7:02   ` Christoph Hellwig
2017-05-23  8:38     ` Vlastimil Babka
2017-05-23  8:40       ` Christoph Hellwig
2017-05-23 11:19       ` Anshuman Khandual
2017-05-24  6:40         ` Anshuman Khandual
2017-05-24 14:31           ` Michal Hocko
2017-05-29 10:55           ` Michael Ellerman
2017-06-09  2:54             ` Anshuman Khandual
2017-05-29 11:07           ` Geert Uytterhoeven
2017-05-23 11:13   ` Anshuman Khandual
2017-05-23  6:41 ` kbuild test robot
2017-05-23  7:24 ` kbuild test robot

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