linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
To: Anshuman Khandual <khandual@linux.vnet.ibm.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Christoph Hellwig <hch@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
Date: Wed, 24 May 2017 12:10:13 +0530	[thread overview]
Message-ID: <7f85724c-6fc1-bb51-11e4-15fc2f89372b@linux.vnet.ibm.com> (raw)
In-Reply-To: <161638da-3b2b-7912-2ae2-3b2936ca1537@linux.vnet.ibm.com>

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 ?

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

  reply	other threads:[~2017-05-24  6:41 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7f85724c-6fc1-bb51-11e4-15fc2f89372b@linux.vnet.ibm.com \
    --to=khandual@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).