Message-ID: <Y7wev+uJTAeWj4j4@kernel.org>
Date:   Mon, 9 Jan 2023 16:03:43 +0200
From:   Mike Rapoport <rppt@...nel.org>
To:     Bagas Sanjaya <bagasdotme@...il.com>
Cc:     Jonathan Corbet <corbet@....net>,
        Andrew Morton <akpm@...ux-foundation.org>,
        David Hildenbrand <david@...hat.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Lorenzo Stoakes <lstoakes@...il.com>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Mel Gorman <mgorman@...e.de>, Michal Hocko <mhocko@...nel.org>,
        Vlastimil Babka <vbabka@...e.cz>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 2/2] docs/mm: Physical Memory: add structure,
 introduction and nodes description

On Sat, Jan 07, 2023 at 10:55:26AM +0700, Bagas Sanjaya wrote:
> On Sun, Jan 01, 2023 at 11:45:23AM +0200, Mike Rapoport wrote:
> > From: "Mike Rapoport (IBM)" <rppt@...nel.org>
> > 
> 
> No patch description really?

The subject says it all, but I can copy it to the description as well.
 
> > +Each node may be divided up into a number of blocks called zones which
> > +represent ranges within memory. These ranges are usually determined by
> > +architectural constraints for accessing the physical memory. A zone is
> > +described by a ``struct zone_struct``, typedeffed to ``zone_t`` and each zone
> > +has one of the types described below.
> > +
> > +`ZONE_DMA` and `ZONE_DMA32`
> > +  represent memory suitable for DMA by peripheral devices that cannot
> > +  access all of the addressable memory. Depending on the architecture,
> > +  either of these zone types or even they both can be disabled at build
> > +  time using ``CONFIG_ZONE_DMA`` and ``CONFIG_ZONE_DMA32`` configuration
> > +  options. Some 64-bit platforms may need both zones as they support
> > +  peripherals with different DMA addressing limitations.
> > +
> > +`ZONE_NORMAL`
> > +  is for normal memory that can be accessed by the kernel all the time. DMA
> > +  operations can be performed on pages in this zone if the DMA devices support
> > +  transfers to all addressable memory. ZONE_NORMAL is always enabled.
> > +
> > +`ZONE_HIGHMEM`
> > +  is the part of the physical memory that is not covered by a permanent mapping
> > +  in the kernel page tables. The memory in this zone is only accessible to the
> > +  kernel using temporary mappings. This zone is available only some 32-bit
> > +  architectures and is enabled with ``CONFIG_HIGHMEM``.
> > +
> > +`ZONE_MOVABLE`
> > +  is for normal accessible memory, just like ZONE_NORMAL. The difference is
> > +  that most pages in ZONE_MOVABLE are movable. That means that while virtual
> > +  addresses of these pages do not change, their content may move between
> > +  different physical pages. ZONE_MOVABLE is only enabled when one of
> > +  `kernelcore`, `movablecore` and `movable_node` parameters is present in the
> > +  kernel command line. See :ref:`Page migration <page_migration>` for
> > +  additional details.
> > +
> > +`ZONE_DEVICE`
> > +  represents memory residing on devices such as PMEM and GPU. It has different
> > +  characteristics than RAM zone types and it exists to provide :ref:`struct
> > +  page <Pages>` and memory map services for device driver identified physical
> > +  address ranges. ZONE_DEVICE is enabled with configuration option
> > +  ``CONFIG_ZONE_DEVICE``.
> 
> I think bullet lists should do the job better, since the zone names are
> connected directly to their representations:

Agree.
 
> > +For example, with 32-bit kernel on an x86 UMA machine with 2 Gbytes of RAM the
> > +entire memory will be on node 0 and there will be three zones: ZONE_DMA,
> > +ZONE_NORMAL and ZONE_HIGHMEM::
> > +
> > +  0                                                            2G
> > +  +-------------------------------------------------------------+
> > +  |                            node 0                           |
> > +  +-------------------------------------------------------------+
> > +
> > +  0         16M                    896M                        2G
> > +  +----------+-----------------------+--------------------------+
> > +  | ZONE_DMA |      ZONE_NORMAL      |       ZONE_HIGHMEM       |
> > +  +----------+-----------------------+--------------------------+
> > +
> > +
> > +With a kernel built with ZONE_DMA disabled and ZONE_DMA32 enabled and booted
> > +with `movablecore=80%` parameter on an arm64 machine with 16 Gbytes of RAM
> > +equally split between two nodes, there will be ZONE_DMA32, ZONE_NORMAL and
> > +ZONE_MOVABLE on node 0, and ZONE_NORMAL and ZONE_MOVABLE on node 1::
> > +
> > +
> > +  1G                                9G                         17G
> > +  +--------------------------------+ +--------------------------+
> > +  |              node 0            | |          node 1          |
> > +  +--------------------------------+ +--------------------------+
> > +
> > +  1G       4G        4200M          9G          9320M          17G
> > +  +---------+----------+-----------+ +------------+-------------+
> > +  |  DMA32  |  NORMAL  |  MOVABLE  | |   NORMAL   |   MOVABLE   |
> > +  +---------+----------+-----------+ +------------+-------------+
> 
> I see inconsistency of formatting keywords: some are in inline code and some
> are not. I'm leaning towards inlining them all:

Sure, thanks for the patch :)
 
> > +For various operations possible with nodemasks please refer to
> > +`include/linux/nodemask.h
> > +<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/nodemask.h>`_.
> 
> Instead of linking to Linus's tree, just inline the source path:

Ok.
 
> > +.. _zones:
> > +
> > +Zones
> > +=====
> > +
> > +.. _pages:
> > +
> > +Pages
> > +=====
> > +
> > +.. _folios:
> > +
> > +Folios
> > +======
> > +
> > +.. _initialization:
> > +
> > +Initialization
> > +==============
> 
> Are these sections stubs (no fields list for each types)? If so, add
> admonitions to inform readers:

Ok.
 
-- 
Sincerely yours,
Mike.

Powered by blists - more mailing lists