[v1,0/4] mm: convert totalram_pages, totalhigh_pages and managed pages to atomic
mbox series

Message ID 1540551662-26458-1-git-send-email-arunks@codeaurora.org
Headers show
Series
  • mm: convert totalram_pages, totalhigh_pages and managed pages to atomic
Related show

Message

Arun KS Oct. 26, 2018, 11 a.m. UTC
This series convert totalram_pages, totalhigh_pages and
zone->managed_pages to atomic variables.

The patch was comiple tested on x86(x86_64_defconfig & i386_defconfig)
on tip of linux-mmotm. And memory hotplug tested on arm64, but on an
older version of kernel.

Arun KS (4):
  mm: Fix multiple evaluvations of totalram_pages and managed_pages
  mm: Convert zone->managed_pages to atomic variable
  mm: convert totalram_pages and totalhigh_pages variables to atomic
  mm: Remove managed_page_count spinlock

 arch/csky/mm/init.c                           |  4 +-
 arch/powerpc/platforms/pseries/cmm.c          | 10 ++--
 arch/s390/mm/init.c                           |  2 +-
 arch/um/kernel/mem.c                          |  3 +-
 arch/x86/kernel/cpu/microcode/core.c          |  5 +-
 drivers/char/agp/backend.c                    |  4 +-
 drivers/gpu/drm/amd/amdkfd/kfd_crat.c         |  2 +-
 drivers/gpu/drm/i915/i915_gem.c               |  2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
 drivers/hv/hv_balloon.c                       | 19 +++----
 drivers/md/dm-bufio.c                         |  2 +-
 drivers/md/dm-crypt.c                         |  2 +-
 drivers/md/dm-integrity.c                     |  2 +-
 drivers/md/dm-stats.c                         |  2 +-
 drivers/media/platform/mtk-vpu/mtk_vpu.c      |  2 +-
 drivers/misc/vmw_balloon.c                    |  2 +-
 drivers/parisc/ccio-dma.c                     |  4 +-
 drivers/parisc/sba_iommu.c                    |  4 +-
 drivers/staging/android/ion/ion_system_heap.c |  2 +-
 drivers/xen/xen-selfballoon.c                 |  6 +--
 fs/ceph/super.h                               |  2 +-
 fs/file_table.c                               |  7 +--
 fs/fuse/inode.c                               |  2 +-
 fs/nfs/write.c                                |  2 +-
 fs/nfsd/nfscache.c                            |  2 +-
 fs/ntfs/malloc.h                              |  2 +-
 fs/proc/base.c                                |  2 +-
 include/linux/highmem.h                       | 28 ++++++++++-
 include/linux/mm.h                            | 27 +++++++++-
 include/linux/mmzone.h                        | 15 +++---
 include/linux/swap.h                          |  1 -
 kernel/fork.c                                 |  5 +-
 kernel/kexec_core.c                           |  5 +-
 kernel/power/snapshot.c                       |  2 +-
 lib/show_mem.c                                |  2 +-
 mm/highmem.c                                  |  4 +-
 mm/huge_memory.c                              |  2 +-
 mm/kasan/quarantine.c                         |  2 +-
 mm/memblock.c                                 |  6 +--
 mm/memory_hotplug.c                           |  4 +-
 mm/mm_init.c                                  |  2 +-
 mm/oom_kill.c                                 |  2 +-
 mm/page_alloc.c                               | 71 +++++++++++++--------------
 mm/shmem.c                                    |  7 +--
 mm/slab.c                                     |  2 +-
 mm/swap.c                                     |  2 +-
 mm/util.c                                     |  2 +-
 mm/vmalloc.c                                  |  4 +-
 mm/vmstat.c                                   |  4 +-
 mm/workingset.c                               |  2 +-
 mm/zswap.c                                    |  4 +-
 net/dccp/proto.c                              |  7 +--
 net/decnet/dn_route.c                         |  2 +-
 net/ipv4/tcp_metrics.c                        |  2 +-
 net/netfilter/nf_conntrack_core.c             |  7 +--
 net/netfilter/xt_hashlimit.c                  |  5 +-
 net/sctp/protocol.c                           |  7 +--
 security/integrity/ima/ima_kexec.c            |  2 +-
 58 files changed, 195 insertions(+), 144 deletions(-)

Comments

Arun KS Nov. 6, 2018, 5:38 a.m. UTC | #1
Any comments?

Regards,
Arun

On 2018-10-26 16:30, Arun KS wrote:
> This series convert totalram_pages, totalhigh_pages and
> zone->managed_pages to atomic variables.
> 
> The patch was comiple tested on x86(x86_64_defconfig & i386_defconfig)
> on tip of linux-mmotm. And memory hotplug tested on arm64, but on an
> older version of kernel.
> 
> Arun KS (4):
>   mm: Fix multiple evaluvations of totalram_pages and managed_pages
>   mm: Convert zone->managed_pages to atomic variable
>   mm: convert totalram_pages and totalhigh_pages variables to atomic
>   mm: Remove managed_page_count spinlock
> 
>  arch/csky/mm/init.c                           |  4 +-
>  arch/powerpc/platforms/pseries/cmm.c          | 10 ++--
>  arch/s390/mm/init.c                           |  2 +-
>  arch/um/kernel/mem.c                          |  3 +-
>  arch/x86/kernel/cpu/microcode/core.c          |  5 +-
>  drivers/char/agp/backend.c                    |  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c         |  2 +-
>  drivers/gpu/drm/i915/i915_gem.c               |  2 +-
>  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
>  drivers/hv/hv_balloon.c                       | 19 +++----
>  drivers/md/dm-bufio.c                         |  2 +-
>  drivers/md/dm-crypt.c                         |  2 +-
>  drivers/md/dm-integrity.c                     |  2 +-
>  drivers/md/dm-stats.c                         |  2 +-
>  drivers/media/platform/mtk-vpu/mtk_vpu.c      |  2 +-
>  drivers/misc/vmw_balloon.c                    |  2 +-
>  drivers/parisc/ccio-dma.c                     |  4 +-
>  drivers/parisc/sba_iommu.c                    |  4 +-
>  drivers/staging/android/ion/ion_system_heap.c |  2 +-
>  drivers/xen/xen-selfballoon.c                 |  6 +--
>  fs/ceph/super.h                               |  2 +-
>  fs/file_table.c                               |  7 +--
>  fs/fuse/inode.c                               |  2 +-
>  fs/nfs/write.c                                |  2 +-
>  fs/nfsd/nfscache.c                            |  2 +-
>  fs/ntfs/malloc.h                              |  2 +-
>  fs/proc/base.c                                |  2 +-
>  include/linux/highmem.h                       | 28 ++++++++++-
>  include/linux/mm.h                            | 27 +++++++++-
>  include/linux/mmzone.h                        | 15 +++---
>  include/linux/swap.h                          |  1 -
>  kernel/fork.c                                 |  5 +-
>  kernel/kexec_core.c                           |  5 +-
>  kernel/power/snapshot.c                       |  2 +-
>  lib/show_mem.c                                |  2 +-
>  mm/highmem.c                                  |  4 +-
>  mm/huge_memory.c                              |  2 +-
>  mm/kasan/quarantine.c                         |  2 +-
>  mm/memblock.c                                 |  6 +--
>  mm/memory_hotplug.c                           |  4 +-
>  mm/mm_init.c                                  |  2 +-
>  mm/oom_kill.c                                 |  2 +-
>  mm/page_alloc.c                               | 71 
> +++++++++++++--------------
>  mm/shmem.c                                    |  7 +--
>  mm/slab.c                                     |  2 +-
>  mm/swap.c                                     |  2 +-
>  mm/util.c                                     |  2 +-
>  mm/vmalloc.c                                  |  4 +-
>  mm/vmstat.c                                   |  4 +-
>  mm/workingset.c                               |  2 +-
>  mm/zswap.c                                    |  4 +-
>  net/dccp/proto.c                              |  7 +--
>  net/decnet/dn_route.c                         |  2 +-
>  net/ipv4/tcp_metrics.c                        |  2 +-
>  net/netfilter/nf_conntrack_core.c             |  7 +--
>  net/netfilter/xt_hashlimit.c                  |  5 +-
>  net/sctp/protocol.c                           |  7 +--
>  security/integrity/ima/ima_kexec.c            |  2 +-
>  58 files changed, 195 insertions(+), 144 deletions(-)
Konstantin Khlebnikov Nov. 6, 2018, 8:17 a.m. UTC | #2
On 06.11.2018 8:38, Arun KS wrote:
> Any comments?

Looks good.
Except unclear motivation behind this change.
This should be in comment of one of patch.

Reviewed-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>

> 
> Regards,
> Arun
> 
> On 2018-10-26 16:30, Arun KS wrote:
>> This series convert totalram_pages, totalhigh_pages and
>> zone->managed_pages to atomic variables.
>>
>> The patch was comiple tested on x86(x86_64_defconfig & i386_defconfig)
>> on tip of linux-mmotm. And memory hotplug tested on arm64, but on an
>> older version of kernel.
>>
>> Arun KS (4):
>>   mm: Fix multiple evaluvations of totalram_pages and managed_pages
>>   mm: Convert zone->managed_pages to atomic variable
>>   mm: convert totalram_pages and totalhigh_pages variables to atomic
>>   mm: Remove managed_page_count spinlock
>>
>>  arch/csky/mm/init.c                           |  4 +-
>>  arch/powerpc/platforms/pseries/cmm.c          | 10 ++--
>>  arch/s390/mm/init.c                           |  2 +-
>>  arch/um/kernel/mem.c                          |  3 +-
>>  arch/x86/kernel/cpu/microcode/core.c          |  5 +-
>>  drivers/char/agp/backend.c                    |  4 +-
>>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c         |  2 +-
>>  drivers/gpu/drm/i915/i915_gem.c               |  2 +-
>>  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
>>  drivers/hv/hv_balloon.c                       | 19 +++----
>>  drivers/md/dm-bufio.c                         |  2 +-
>>  drivers/md/dm-crypt.c                         |  2 +-
>>  drivers/md/dm-integrity.c                     |  2 +-
>>  drivers/md/dm-stats.c                         |  2 +-
>>  drivers/media/platform/mtk-vpu/mtk_vpu.c      |  2 +-
>>  drivers/misc/vmw_balloon.c                    |  2 +-
>>  drivers/parisc/ccio-dma.c                     |  4 +-
>>  drivers/parisc/sba_iommu.c                    |  4 +-
>>  drivers/staging/android/ion/ion_system_heap.c |  2 +-
>>  drivers/xen/xen-selfballoon.c                 |  6 +--
>>  fs/ceph/super.h                               |  2 +-
>>  fs/file_table.c                               |  7 +--
>>  fs/fuse/inode.c                               |  2 +-
>>  fs/nfs/write.c                                |  2 +-
>>  fs/nfsd/nfscache.c                            |  2 +-
>>  fs/ntfs/malloc.h                              |  2 +-
>>  fs/proc/base.c                                |  2 +-
>>  include/linux/highmem.h                       | 28 ++++++++++-
>>  include/linux/mm.h                            | 27 +++++++++-
>>  include/linux/mmzone.h                        | 15 +++---
>>  include/linux/swap.h                          |  1 -
>>  kernel/fork.c                                 |  5 +-
>>  kernel/kexec_core.c                           |  5 +-
>>  kernel/power/snapshot.c                       |  2 +-
>>  lib/show_mem.c                                |  2 +-
>>  mm/highmem.c                                  |  4 +-
>>  mm/huge_memory.c                              |  2 +-
>>  mm/kasan/quarantine.c                         |  2 +-
>>  mm/memblock.c                                 |  6 +--
>>  mm/memory_hotplug.c                           |  4 +-
>>  mm/mm_init.c                                  |  2 +-
>>  mm/oom_kill.c                                 |  2 +-
>>  mm/page_alloc.c                               | 71 +++++++++++++--------------
>>  mm/shmem.c                                    |  7 +--
>>  mm/slab.c                                     |  2 +-
>>  mm/swap.c                                     |  2 +-
>>  mm/util.c                                     |  2 +-
>>  mm/vmalloc.c                                  |  4 +-
>>  mm/vmstat.c                                   |  4 +-
>>  mm/workingset.c                               |  2 +-
>>  mm/zswap.c                                    |  4 +-
>>  net/dccp/proto.c                              |  7 +--
>>  net/decnet/dn_route.c                         |  2 +-
>>  net/ipv4/tcp_metrics.c                        |  2 +-
>>  net/netfilter/nf_conntrack_core.c             |  7 +--
>>  net/netfilter/xt_hashlimit.c                  |  5 +-
>>  net/sctp/protocol.c                           |  7 +--
>>  security/integrity/ima/ima_kexec.c            |  2 +-
>>  58 files changed, 195 insertions(+), 144 deletions(-)
Arun KS Nov. 6, 2018, 8:30 a.m. UTC | #3
On 2018-11-06 13:47, Konstantin Khlebnikov wrote:
> On 06.11.2018 8:38, Arun KS wrote:
>> Any comments?
> 
> Looks good.
> Except unclear motivation behind this change.
> This should be in comment of one of patch.

totalram_pages, zone->managed_pages and totalhigh_pages are sometimes 
modified outside managed_page_count_lock. Hence convert these variable 
to atomic to avoid readers potentially seeing a store tear.

Will update the comment.

Regards,
Arun

> 
> Reviewed-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> 
>> 
>> Regards,
>> Arun
>> 
>> On 2018-10-26 16:30, Arun KS wrote:
>>> This series convert totalram_pages, totalhigh_pages and
>>> zone->managed_pages to atomic variables.
>>> 
>>> The patch was comiple tested on x86(x86_64_defconfig & 
>>> i386_defconfig)
>>> on tip of linux-mmotm. And memory hotplug tested on arm64, but on an
>>> older version of kernel.
>>> 
>>> Arun KS (4):
>>>   mm: Fix multiple evaluvations of totalram_pages and managed_pages
>>>   mm: Convert zone->managed_pages to atomic variable
>>>   mm: convert totalram_pages and totalhigh_pages variables to atomic
>>>   mm: Remove managed_page_count spinlock
>>> 
>>>  arch/csky/mm/init.c                           |  4 +-
>>>  arch/powerpc/platforms/pseries/cmm.c          | 10 ++--
>>>  arch/s390/mm/init.c                           |  2 +-
>>>  arch/um/kernel/mem.c                          |  3 +-
>>>  arch/x86/kernel/cpu/microcode/core.c          |  5 +-
>>>  drivers/char/agp/backend.c                    |  4 +-
>>>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c         |  2 +-
>>>  drivers/gpu/drm/i915/i915_gem.c               |  2 +-
>>>  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
>>>  drivers/hv/hv_balloon.c                       | 19 +++----
>>>  drivers/md/dm-bufio.c                         |  2 +-
>>>  drivers/md/dm-crypt.c                         |  2 +-
>>>  drivers/md/dm-integrity.c                     |  2 +-
>>>  drivers/md/dm-stats.c                         |  2 +-
>>>  drivers/media/platform/mtk-vpu/mtk_vpu.c      |  2 +-
>>>  drivers/misc/vmw_balloon.c                    |  2 +-
>>>  drivers/parisc/ccio-dma.c                     |  4 +-
>>>  drivers/parisc/sba_iommu.c                    |  4 +-
>>>  drivers/staging/android/ion/ion_system_heap.c |  2 +-
>>>  drivers/xen/xen-selfballoon.c                 |  6 +--
>>>  fs/ceph/super.h                               |  2 +-
>>>  fs/file_table.c                               |  7 +--
>>>  fs/fuse/inode.c                               |  2 +-
>>>  fs/nfs/write.c                                |  2 +-
>>>  fs/nfsd/nfscache.c                            |  2 +-
>>>  fs/ntfs/malloc.h                              |  2 +-
>>>  fs/proc/base.c                                |  2 +-
>>>  include/linux/highmem.h                       | 28 ++++++++++-
>>>  include/linux/mm.h                            | 27 +++++++++-
>>>  include/linux/mmzone.h                        | 15 +++---
>>>  include/linux/swap.h                          |  1 -
>>>  kernel/fork.c                                 |  5 +-
>>>  kernel/kexec_core.c                           |  5 +-
>>>  kernel/power/snapshot.c                       |  2 +-
>>>  lib/show_mem.c                                |  2 +-
>>>  mm/highmem.c                                  |  4 +-
>>>  mm/huge_memory.c                              |  2 +-
>>>  mm/kasan/quarantine.c                         |  2 +-
>>>  mm/memblock.c                                 |  6 +--
>>>  mm/memory_hotplug.c                           |  4 +-
>>>  mm/mm_init.c                                  |  2 +-
>>>  mm/oom_kill.c                                 |  2 +-
>>>  mm/page_alloc.c                               | 71 
>>> +++++++++++++--------------
>>>  mm/shmem.c                                    |  7 +--
>>>  mm/slab.c                                     |  2 +-
>>>  mm/swap.c                                     |  2 +-
>>>  mm/util.c                                     |  2 +-
>>>  mm/vmalloc.c                                  |  4 +-
>>>  mm/vmstat.c                                   |  4 +-
>>>  mm/workingset.c                               |  2 +-
>>>  mm/zswap.c                                    |  4 +-
>>>  net/dccp/proto.c                              |  7 +--
>>>  net/decnet/dn_route.c                         |  2 +-
>>>  net/ipv4/tcp_metrics.c                        |  2 +-
>>>  net/netfilter/nf_conntrack_core.c             |  7 +--
>>>  net/netfilter/xt_hashlimit.c                  |  5 +-
>>>  net/sctp/protocol.c                           |  7 +--
>>>  security/integrity/ima/ima_kexec.c            |  2 +-
>>>  58 files changed, 195 insertions(+), 144 deletions(-)
Konstantin Khlebnikov Nov. 6, 2018, 8:37 a.m. UTC | #4
On 06.11.2018 11:30, Arun KS wrote:
> On 2018-11-06 13:47, Konstantin Khlebnikov wrote:
>> On 06.11.2018 8:38, Arun KS wrote:
>>> Any comments?
>>
>> Looks good.
>> Except unclear motivation behind this change.
>> This should be in comment of one of patch.
> 
> totalram_pages, zone->managed_pages and totalhigh_pages are sometimes modified outside managed_page_count_lock. Hence convert these variable 
> to atomic to avoid readers potentially seeing a store tear.

So, this is just theoretical issue or splat from sanitizer.
After boot memory online\offline are strictly serialized by rw-semaphore.

> 
> Will update the comment.
> 
> Regards,
> Arun
> 
>>
>> Reviewed-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>>
>>>
>>> Regards,
>>> Arun
>>>
>>> On 2018-10-26 16:30, Arun KS wrote:
>>>> This series convert totalram_pages, totalhigh_pages and
>>>> zone->managed_pages to atomic variables.
>>>>
>>>> The patch was comiple tested on x86(x86_64_defconfig & i386_defconfig)
>>>> on tip of linux-mmotm. And memory hotplug tested on arm64, but on an
>>>> older version of kernel.
>>>>
>>>> Arun KS (4):
>>>>   mm: Fix multiple evaluvations of totalram_pages and managed_pages
>>>>   mm: Convert zone->managed_pages to atomic variable
>>>>   mm: convert totalram_pages and totalhigh_pages variables to atomic
>>>>   mm: Remove managed_page_count spinlock
>>>>
>>>>  arch/csky/mm/init.c                           |  4 +-
>>>>  arch/powerpc/platforms/pseries/cmm.c          | 10 ++--
>>>>  arch/s390/mm/init.c                           |  2 +-
>>>>  arch/um/kernel/mem.c                          |  3 +-
>>>>  arch/x86/kernel/cpu/microcode/core.c          |  5 +-
>>>>  drivers/char/agp/backend.c                    |  4 +-
>>>>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c         |  2 +-
>>>>  drivers/gpu/drm/i915/i915_gem.c               |  2 +-
>>>>  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
>>>>  drivers/hv/hv_balloon.c                       | 19 +++----
>>>>  drivers/md/dm-bufio.c                         |  2 +-
>>>>  drivers/md/dm-crypt.c                         |  2 +-
>>>>  drivers/md/dm-integrity.c                     |  2 +-
>>>>  drivers/md/dm-stats.c                         |  2 +-
>>>>  drivers/media/platform/mtk-vpu/mtk_vpu.c      |  2 +-
>>>>  drivers/misc/vmw_balloon.c                    |  2 +-
>>>>  drivers/parisc/ccio-dma.c                     |  4 +-
>>>>  drivers/parisc/sba_iommu.c                    |  4 +-
>>>>  drivers/staging/android/ion/ion_system_heap.c |  2 +-
>>>>  drivers/xen/xen-selfballoon.c                 |  6 +--
>>>>  fs/ceph/super.h                               |  2 +-
>>>>  fs/file_table.c                               |  7 +--
>>>>  fs/fuse/inode.c                               |  2 +-
>>>>  fs/nfs/write.c                                |  2 +-
>>>>  fs/nfsd/nfscache.c                            |  2 +-
>>>>  fs/ntfs/malloc.h                              |  2 +-
>>>>  fs/proc/base.c                                |  2 +-
>>>>  include/linux/highmem.h                       | 28 ++++++++++-
>>>>  include/linux/mm.h                            | 27 +++++++++-
>>>>  include/linux/mmzone.h                        | 15 +++---
>>>>  include/linux/swap.h                          |  1 -
>>>>  kernel/fork.c                                 |  5 +-
>>>>  kernel/kexec_core.c                           |  5 +-
>>>>  kernel/power/snapshot.c                       |  2 +-
>>>>  lib/show_mem.c                                |  2 +-
>>>>  mm/highmem.c                                  |  4 +-
>>>>  mm/huge_memory.c                              |  2 +-
>>>>  mm/kasan/quarantine.c                         |  2 +-
>>>>  mm/memblock.c                                 |  6 +--
>>>>  mm/memory_hotplug.c                           |  4 +-
>>>>  mm/mm_init.c                                  |  2 +-
>>>>  mm/oom_kill.c                                 |  2 +-
>>>>  mm/page_alloc.c                               | 71 +++++++++++++--------------
>>>>  mm/shmem.c                                    |  7 +--
>>>>  mm/slab.c                                     |  2 +-
>>>>  mm/swap.c                                     |  2 +-
>>>>  mm/util.c                                     |  2 +-
>>>>  mm/vmalloc.c                                  |  4 +-
>>>>  mm/vmstat.c                                   |  4 +-
>>>>  mm/workingset.c                               |  2 +-
>>>>  mm/zswap.c                                    |  4 +-
>>>>  net/dccp/proto.c                              |  7 +--
>>>>  net/decnet/dn_route.c                         |  2 +-
>>>>  net/ipv4/tcp_metrics.c                        |  2 +-
>>>>  net/netfilter/nf_conntrack_core.c             |  7 +--
>>>>  net/netfilter/xt_hashlimit.c                  |  5 +-
>>>>  net/sctp/protocol.c                           |  7 +--
>>>>  security/integrity/ima/ima_kexec.c            |  2 +-
>>>>  58 files changed, 195 insertions(+), 144 deletions(-)
Arun KS Nov. 6, 2018, 8:43 a.m. UTC | #5
On 2018-11-06 14:07, Konstantin Khlebnikov wrote:
> On 06.11.2018 11:30, Arun KS wrote:
>> On 2018-11-06 13:47, Konstantin Khlebnikov wrote:
>>> On 06.11.2018 8:38, Arun KS wrote:
>>>> Any comments?
>>> 
>>> Looks good.
>>> Except unclear motivation behind this change.
>>> This should be in comment of one of patch.
>> 
>> totalram_pages, zone->managed_pages and totalhigh_pages are sometimes 
>> modified outside managed_page_count_lock. Hence convert these variable 
>> to atomic to avoid readers potentially seeing a store tear.
> 
> So, this is just theoretical issue or splat from sanitizer.
> After boot memory online\offline are strictly serialized by 
> rw-semaphore.

Few instances which can race with hot add. Please see below,
https://patchwork.kernel.org/patch/10627521/

Regards,
Arun

> 
>> 
>> Will update the comment.
>> 
>> Regards,
>> Arun
>> 
>>> 
>>> Reviewed-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>>> 
>>>> 
>>>> Regards,
>>>> Arun
>>>> 
>>>> On 2018-10-26 16:30, Arun KS wrote:
>>>>> This series convert totalram_pages, totalhigh_pages and
>>>>> zone->managed_pages to atomic variables.
>>>>> 
>>>>> The patch was comiple tested on x86(x86_64_defconfig & 
>>>>> i386_defconfig)
>>>>> on tip of linux-mmotm. And memory hotplug tested on arm64, but on 
>>>>> an
>>>>> older version of kernel.
>>>>> 
>>>>> Arun KS (4):
>>>>>   mm: Fix multiple evaluvations of totalram_pages and managed_pages
>>>>>   mm: Convert zone->managed_pages to atomic variable
>>>>>   mm: convert totalram_pages and totalhigh_pages variables to 
>>>>> atomic
>>>>>   mm: Remove managed_page_count spinlock
>>>>> 
>>>>>  arch/csky/mm/init.c                           |  4 +-
>>>>>  arch/powerpc/platforms/pseries/cmm.c          | 10 ++--
>>>>>  arch/s390/mm/init.c                           |  2 +-
>>>>>  arch/um/kernel/mem.c                          |  3 +-
>>>>>  arch/x86/kernel/cpu/microcode/core.c          |  5 +-
>>>>>  drivers/char/agp/backend.c                    |  4 +-
>>>>>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c         |  2 +-
>>>>>  drivers/gpu/drm/i915/i915_gem.c               |  2 +-
>>>>>  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
>>>>>  drivers/hv/hv_balloon.c                       | 19 +++----
>>>>>  drivers/md/dm-bufio.c                         |  2 +-
>>>>>  drivers/md/dm-crypt.c                         |  2 +-
>>>>>  drivers/md/dm-integrity.c                     |  2 +-
>>>>>  drivers/md/dm-stats.c                         |  2 +-
>>>>>  drivers/media/platform/mtk-vpu/mtk_vpu.c      |  2 +-
>>>>>  drivers/misc/vmw_balloon.c                    |  2 +-
>>>>>  drivers/parisc/ccio-dma.c                     |  4 +-
>>>>>  drivers/parisc/sba_iommu.c                    |  4 +-
>>>>>  drivers/staging/android/ion/ion_system_heap.c |  2 +-
>>>>>  drivers/xen/xen-selfballoon.c                 |  6 +--
>>>>>  fs/ceph/super.h                               |  2 +-
>>>>>  fs/file_table.c                               |  7 +--
>>>>>  fs/fuse/inode.c                               |  2 +-
>>>>>  fs/nfs/write.c                                |  2 +-
>>>>>  fs/nfsd/nfscache.c                            |  2 +-
>>>>>  fs/ntfs/malloc.h                              |  2 +-
>>>>>  fs/proc/base.c                                |  2 +-
>>>>>  include/linux/highmem.h                       | 28 ++++++++++-
>>>>>  include/linux/mm.h                            | 27 +++++++++-
>>>>>  include/linux/mmzone.h                        | 15 +++---
>>>>>  include/linux/swap.h                          |  1 -
>>>>>  kernel/fork.c                                 |  5 +-
>>>>>  kernel/kexec_core.c                           |  5 +-
>>>>>  kernel/power/snapshot.c                       |  2 +-
>>>>>  lib/show_mem.c                                |  2 +-
>>>>>  mm/highmem.c                                  |  4 +-
>>>>>  mm/huge_memory.c                              |  2 +-
>>>>>  mm/kasan/quarantine.c                         |  2 +-
>>>>>  mm/memblock.c                                 |  6 +--
>>>>>  mm/memory_hotplug.c                           |  4 +-
>>>>>  mm/mm_init.c                                  |  2 +-
>>>>>  mm/oom_kill.c                                 |  2 +-
>>>>>  mm/page_alloc.c                               | 71 
>>>>> +++++++++++++--------------
>>>>>  mm/shmem.c                                    |  7 +--
>>>>>  mm/slab.c                                     |  2 +-
>>>>>  mm/swap.c                                     |  2 +-
>>>>>  mm/util.c                                     |  2 +-
>>>>>  mm/vmalloc.c                                  |  4 +-
>>>>>  mm/vmstat.c                                   |  4 +-
>>>>>  mm/workingset.c                               |  2 +-
>>>>>  mm/zswap.c                                    |  4 +-
>>>>>  net/dccp/proto.c                              |  7 +--
>>>>>  net/decnet/dn_route.c                         |  2 +-
>>>>>  net/ipv4/tcp_metrics.c                        |  2 +-
>>>>>  net/netfilter/nf_conntrack_core.c             |  7 +--
>>>>>  net/netfilter/xt_hashlimit.c                  |  5 +-
>>>>>  net/sctp/protocol.c                           |  7 +--
>>>>>  security/integrity/ima/ima_kexec.c            |  2 +-
>>>>>  58 files changed, 195 insertions(+), 144 deletions(-)
Andrew Morton Nov. 7, 2018, 12:22 a.m. UTC | #6
On Fri, 26 Oct 2018 16:30:58 +0530 Arun KS <arunks@codeaurora.org> wrote:

> This series convert totalram_pages, totalhigh_pages and
> zone->managed_pages to atomic variables.

The whole point appears to be removal of managed_page_count_lock, yes?

Why?  What is the value of this patchset?  If "performance" then are any
measurements available?
Arun KS Nov. 7, 2018, 6:19 a.m. UTC | #7
On 2018-11-07 05:52, Andrew Morton wrote:
> On Fri, 26 Oct 2018 16:30:58 +0530 Arun KS <arunks@codeaurora.org> 
> wrote:
> 
>> This series convert totalram_pages, totalhigh_pages and
>> zone->managed_pages to atomic variables.
> 
> The whole point appears to be removal of managed_page_count_lock, yes?
> 
> Why?  What is the value of this patchset?  If "performance" then are 
> any
> measurements available?

Hello Andrew,

https://patchwork.kernel.org/patch/10670787/
In version 2, I have added motivation behind this conversion. Pasting 
same here,

totalram_pages, zone->managed_pages and totalhigh_pages updates are 
protected by managed_page_count_lock, but readers never care about it. 
Convert these variables to atomic to avoid readers potentially seeing a 
store tear. I don't think we have a performance improvement here.

Regards,
Arun
Konstantin Khlebnikov Nov. 7, 2018, 7:02 a.m. UTC | #8
On 06.11.2018 11:43, Arun KS wrote:
> On 2018-11-06 14:07, Konstantin Khlebnikov wrote:
>> On 06.11.2018 11:30, Arun KS wrote:
>>> On 2018-11-06 13:47, Konstantin Khlebnikov wrote:
>>>> On 06.11.2018 8:38, Arun KS wrote:
>>>>> Any comments?
>>>>
>>>> Looks good.
>>>> Except unclear motivation behind this change.
>>>> This should be in comment of one of patch.
>>>
>>> totalram_pages, zone->managed_pages and totalhigh_pages are sometimes modified outside managed_page_count_lock. Hence convert these 
>>> variable to atomic to avoid readers potentially seeing a store tear.
>>
>> So, this is just theoretical issue or splat from sanitizer.
>> After boot memory online\offline are strictly serialized by rw-semaphore.
> 
> Few instances which can race with hot add. Please see below,
> https://patchwork.kernel.org/patch/10627521/
Could you point what exactly are you fixing with this set?

from v2:

 > totalram_pages, zone->managed_pages and totalhigh_pages updates
 > are protected by managed_page_count_lock, but readers never care
 > about it. Convert these variables to atomic to avoid readers
 > potentially seeing a store tear.

This?


Aligned unsigned long almost always stored at once.

To make it completely correct you could replace

a += b;

with

WRITE_ONCE(a, a + b);

> 
> Regards,
> Arun
> 
>>
>>>
>>> Will update the comment.
>>>
>>> Regards,
>>> Arun
>>>
>>>>
>>>> Reviewed-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>>>>
>>>>>
>>>>> Regards,
>>>>> Arun
>>>>>
>>>>> On 2018-10-26 16:30, Arun KS wrote:
>>>>>> This series convert totalram_pages, totalhigh_pages and
>>>>>> zone->managed_pages to atomic variables.
>>>>>>
>>>>>> The patch was comiple tested on x86(x86_64_defconfig & i386_defconfig)
>>>>>> on tip of linux-mmotm. And memory hotplug tested on arm64, but on an
>>>>>> older version of kernel.
>>>>>>
>>>>>> Arun KS (4):
>>>>>>   mm: Fix multiple evaluvations of totalram_pages and managed_pages
>>>>>>   mm: Convert zone->managed_pages to atomic variable
>>>>>>   mm: convert totalram_pages and totalhigh_pages variables to atomic
>>>>>>   mm: Remove managed_page_count spinlock
>>>>>>
>>>>>>  arch/csky/mm/init.c                           |  4 +-
>>>>>>  arch/powerpc/platforms/pseries/cmm.c          | 10 ++--
>>>>>>  arch/s390/mm/init.c                           |  2 +-
>>>>>>  arch/um/kernel/mem.c                          |  3 +-
>>>>>>  arch/x86/kernel/cpu/microcode/core.c          |  5 +-
>>>>>>  drivers/char/agp/backend.c                    |  4 +-
>>>>>>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c         |  2 +-
>>>>>>  drivers/gpu/drm/i915/i915_gem.c               |  2 +-
>>>>>>  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
>>>>>>  drivers/hv/hv_balloon.c                       | 19 +++----
>>>>>>  drivers/md/dm-bufio.c                         |  2 +-
>>>>>>  drivers/md/dm-crypt.c                         |  2 +-
>>>>>>  drivers/md/dm-integrity.c                     |  2 +-
>>>>>>  drivers/md/dm-stats.c                         |  2 +-
>>>>>>  drivers/media/platform/mtk-vpu/mtk_vpu.c      |  2 +-
>>>>>>  drivers/misc/vmw_balloon.c                    |  2 +-
>>>>>>  drivers/parisc/ccio-dma.c                     |  4 +-
>>>>>>  drivers/parisc/sba_iommu.c                    |  4 +-
>>>>>>  drivers/staging/android/ion/ion_system_heap.c |  2 +-
>>>>>>  drivers/xen/xen-selfballoon.c                 |  6 +--
>>>>>>  fs/ceph/super.h                               |  2 +-
>>>>>>  fs/file_table.c                               |  7 +--
>>>>>>  fs/fuse/inode.c                               |  2 +-
>>>>>>  fs/nfs/write.c                                |  2 +-
>>>>>>  fs/nfsd/nfscache.c                            |  2 +-
>>>>>>  fs/ntfs/malloc.h                              |  2 +-
>>>>>>  fs/proc/base.c                                |  2 +-
>>>>>>  include/linux/highmem.h                       | 28 ++++++++++-
>>>>>>  include/linux/mm.h                            | 27 +++++++++-
>>>>>>  include/linux/mmzone.h                        | 15 +++---
>>>>>>  include/linux/swap.h                          |  1 -
>>>>>>  kernel/fork.c                                 |  5 +-
>>>>>>  kernel/kexec_core.c                           |  5 +-
>>>>>>  kernel/power/snapshot.c                       |  2 +-
>>>>>>  lib/show_mem.c                                |  2 +-
>>>>>>  mm/highmem.c                                  |  4 +-
>>>>>>  mm/huge_memory.c                              |  2 +-
>>>>>>  mm/kasan/quarantine.c                         |  2 +-
>>>>>>  mm/memblock.c                                 |  6 +--
>>>>>>  mm/memory_hotplug.c                           |  4 +-
>>>>>>  mm/mm_init.c                                  |  2 +-
>>>>>>  mm/oom_kill.c                                 |  2 +-
>>>>>>  mm/page_alloc.c                               | 71 +++++++++++++--------------
>>>>>>  mm/shmem.c                                    |  7 +--
>>>>>>  mm/slab.c                                     |  2 +-
>>>>>>  mm/swap.c                                     |  2 +-
>>>>>>  mm/util.c                                     |  2 +-
>>>>>>  mm/vmalloc.c                                  |  4 +-
>>>>>>  mm/vmstat.c                                   |  4 +-
>>>>>>  mm/workingset.c                               |  2 +-
>>>>>>  mm/zswap.c                                    |  4 +-
>>>>>>  net/dccp/proto.c                              |  7 +--
>>>>>>  net/decnet/dn_route.c                         |  2 +-
>>>>>>  net/ipv4/tcp_metrics.c                        |  2 +-
>>>>>>  net/netfilter/nf_conntrack_core.c             |  7 +--
>>>>>>  net/netfilter/xt_hashlimit.c                  |  5 +-
>>>>>>  net/sctp/protocol.c                           |  7 +--
>>>>>>  security/integrity/ima/ima_kexec.c            |  2 +-
>>>>>>  58 files changed, 195 insertions(+), 144 deletions(-)
Vlastimil Babka Nov. 7, 2018, 8:50 a.m. UTC | #9
On 11/7/18 8:02 AM, Konstantin Khlebnikov wrote:
> On 06.11.2018 11:43, Arun KS wrote:
>> On 2018-11-06 14:07, Konstantin Khlebnikov wrote:
>>> On 06.11.2018 11:30, Arun KS wrote:
>>>> On 2018-11-06 13:47, Konstantin Khlebnikov wrote:
>>>>> On 06.11.2018 8:38, Arun KS wrote:
>>>>>> Any comments?
>>>>>
>>>>> Looks good.
>>>>> Except unclear motivation behind this change.
>>>>> This should be in comment of one of patch.
>>>>
>>>> totalram_pages, zone->managed_pages and totalhigh_pages are sometimes modified outside managed_page_count_lock. Hence convert these 
>>>> variable to atomic to avoid readers potentially seeing a store tear.
>>>
>>> So, this is just theoretical issue or splat from sanitizer.
>>> After boot memory online\offline are strictly serialized by rw-semaphore.
>>
>> Few instances which can race with hot add. Please see below,
>> https://patchwork.kernel.org/patch/10627521/
> Could you point what exactly are you fixing with this set?
> 
> from v2:
> 
>  > totalram_pages, zone->managed_pages and totalhigh_pages updates
>  > are protected by managed_page_count_lock, but readers never care
>  > about it. Convert these variables to atomic to avoid readers
>  > potentially seeing a store tear.
> 
> This?
> 
> 
> Aligned unsigned long almost always stored at once.

The point is "almost always", so better not rely on it :) But the main
motivation was that managed_page_count_lock handling was complicating
Arun's "memory_hotplug: Free pages as higher order" patch and it seemed
a better idea to just remove and convert this to atomics, with
preventing potential store-to-read tearing as a bonus.

It would be nice to mention it in the changelogs though.

> To make it completely correct you could replace
> 
> a += b;
> 
> with
> 
> WRITE_ONCE(a, a + b);

Wouldn't be enough to get rid of the locks.
Michal Hocko Nov. 7, 2018, 10:28 a.m. UTC | #10
On Wed 07-11-18 09:50:10, Vlastimil Babka wrote:
> On 11/7/18 8:02 AM, Konstantin Khlebnikov wrote:
[...]
> > Could you point what exactly are you fixing with this set?
> > 
> > from v2:
> > 
> >  > totalram_pages, zone->managed_pages and totalhigh_pages updates
> >  > are protected by managed_page_count_lock, but readers never care
> >  > about it. Convert these variables to atomic to avoid readers
> >  > potentially seeing a store tear.
> > 
> > This?
> > 
> > 
> > Aligned unsigned long almost always stored at once.
> 
> The point is "almost always", so better not rely on it :) But the main
> motivation was that managed_page_count_lock handling was complicating
> Arun's "memory_hotplug: Free pages as higher order" patch and it seemed
> a better idea to just remove and convert this to atomics, with
> preventing potential store-to-read tearing as a bonus.

And more importantly the lock itself seems bogus as mentioned here
http://lkml.kernel.org/r/20181106141732.GR27423@dhcp22.suse.cz

> It would be nice to mention it in the changelogs though.

agreed
Michal Hocko Nov. 7, 2018, 10:39 a.m. UTC | #11
On Wed 07-11-18 11:28:37, Michal Hocko wrote:
> On Wed 07-11-18 09:50:10, Vlastimil Babka wrote:
> > On 11/7/18 8:02 AM, Konstantin Khlebnikov wrote:
> [...]
> > > Could you point what exactly are you fixing with this set?
> > > 
> > > from v2:
> > > 
> > >  > totalram_pages, zone->managed_pages and totalhigh_pages updates
> > >  > are protected by managed_page_count_lock, but readers never care
> > >  > about it. Convert these variables to atomic to avoid readers
> > >  > potentially seeing a store tear.
> > > 
> > > This?
> > > 
> > > 
> > > Aligned unsigned long almost always stored at once.
> > 
> > The point is "almost always", so better not rely on it :) But the main
> > motivation was that managed_page_count_lock handling was complicating
> > Arun's "memory_hotplug: Free pages as higher order" patch and it seemed
> > a better idea to just remove and convert this to atomics, with
> > preventing potential store-to-read tearing as a bonus.
> 
> And more importantly the lock itself seems bogus as mentioned here
> http://lkml.kernel.org/r/20181106141732.GR27423@dhcp22.suse.cz

Should be http://lkml.kernel.org/r/20181107103630.GF2453@dhcp22.suse.cz