[GIT PULL] libnvdimm for 4.18

[GIT PULL] libnvdimm for 4.18

[Williams, Dan J]

Hi Linus, please pull from:

  git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/libnvdimm-for-4.18

...to receive the first part of the libnvdimm / persistent memory
support update for 4.18. These patches have all been in -next for
several releases. The 'DAX DMA vs Truncate' work has had extra soak
time as it nearly missed 4.17.

This pull request adds a user for the new 'bytes-remaining' updates to
memcpy_mcsafe() that you already received through Ingo via the x86-dax-
for-linus pull. There is a minor collision with bdev_dax_supported()
reworks that you pulled from xfs, and another minor collision of the
vm_fault_t conversion you received from Andrew. A potential merge
resolution is here: https://git.kernel.org/pub/scm/linux/kernel/git/nvd
imm/nvdimm.git/log/?h=libnvdimm-for-4.18-merge

Not included in this pull, but still targeting this cycle, is support
for handling memory media errors (poison) consumed via userspace dax
mappings.

---

The following changes since commit b04e217704b7f879c6b91222b066983a44a7a09f:

  Linux 4.17-rc7 (2018-05-27 13:01:47 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/libnvdimm-for-4.18

for you to fetch changes up to 930218affeadd1325ea17e053f0dcecf218f5a4f:

  Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next (2018-06-08 15:16:44 -0700)

----------------------------------------------------------------
libnvdimm for 4.18

* DAX broke a fundamental assumption of truncate of file mapped pages.
  The truncate path assumed that it is safe to disconnect a pinned page
  from a file and let the filesystem reclaim the physical block. With DAX
  the page is equivalent to the filesystem block. Introduce
  dax_layout_busy_page() to enable filesystems to wait for pinned DAX
  pages to be released. Without this wait a filesystem could allocate
  blocks under active device-DMA to a new file.

* DAX arranges for the block layer to be bypassed and uses
  dax_direct_access() + copy_to_iter() to satisfy read(2) calls.
  However, the memcpy_mcsafe() facility is available through the pmem
  block driver. In order to safely handle media errors, via the DAX
  block-layer bypass, introduce copy_to_iter_mcsafe().

* Fix cache management policy relative to the ACPI NFIT Platform
  Capabilities Structure to properly elide cache flushes when they are not
  necessary. The table indicates whether CPU caches are power-fail
  protected. Clarify that a deep flush is always performed on
  REQ_{FUA,PREFLUSH} requests.

----------------------------------------------------------------
Dan Williams (17):
      memremap: split devm_memremap_pages() and memremap() infrastructure
      mm: introduce MEMORY_DEVICE_FS_DAX and CONFIG_DEV_PAGEMAP_OPS
      mm: fix __gup_device_huge vs unmap
      mm, fs, dax: handle layout changes to pinned dax mappings
      xfs: prepare xfs_break_layouts() to be called with XFS_MMAPLOCK_EXCL
      xfs: prepare xfs_break_layouts() for another layout type
      xfs, dax: introduce xfs_break_dax_layouts()
      uio, lib: Fix CONFIG_ARCH_HAS_UACCESS_MCSAFE compilation
      dax: Introduce a ->copy_to_iter dax operation
      dax: Report bytes remaining in dax_iomap_actor()
      pmem: Switch to copy_to_iter_mcsafe()
      x86, nfit_test: Add unit test for memcpy_mcsafe()
      libnvdimm: Debug probe times
      libnvdimm, e820: Register all pmem resources
      acpi, nfit: Remove ecc_unit_size
      Merge branch 'for-4.18/dax' into libnvdimm-for-next
      Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next

Matthew Wilcox (1):
      dax: dax_insert_mapping_entry always succeeds

Robert Elliott (1):
      linvdimm, pmem: Preserve read-only setting for pmem devices

Ross Zwisler (4):
      libnvdimm, pmem: Complete REQ_FLUSH => REQ_PREFLUSH
      libnvdimm, pmem: Unconditionally deep flush on *sync
      libnvdimm, pmem: Do not flush power-fail protected CPU caches
      dax: Use dax_write_cache* helpers

 Documentation/ABI/removed/sysfs-bus-nfit |  17 +++
 Documentation/ABI/testing/sysfs-bus-nfit |  19 ---
 arch/x86/Kconfig                         |   1 +
 arch/x86/Kconfig.debug                   |   3 +
 arch/x86/include/asm/mcsafe_test.h       |  75 +++++++++++
 arch/x86/include/asm/string_64.h         |  10 +-
 arch/x86/include/asm/uaccess_64.h        |  14 +++
 arch/x86/lib/memcpy_64.S                 | 112 ++++++++---------
 arch/x86/lib/usercopy_64.c               |  21 ++++
 drivers/acpi/nfit/core.c                 |  11 --
 drivers/dax/super.c                      |  33 +++--
 drivers/md/dm-linear.c                   |  16 +++
 drivers/md/dm-log-writes.c               |  15 +++
 drivers/md/dm-stripe.c                   |  21 ++++
 drivers/md/dm.c                          |  25 ++++
 drivers/nvdimm/bus.c                     |  19 ++-
 drivers/nvdimm/claim.c                   |   3 +-
 drivers/nvdimm/e820.c                    |  41 +++---
 drivers/nvdimm/pfn_devs.c                |   2 -
 drivers/nvdimm/pmem.c                    |  52 ++++++--
 drivers/nvdimm/region_devs.c             |   3 +-
 drivers/s390/block/dcssblk.c             |   7 ++
 fs/Kconfig                               |   1 +
 fs/dax.c                                 | 136 ++++++++++++++++----
 fs/xfs/xfs_file.c                        |  72 ++++++++++-
 fs/xfs/xfs_inode.h                       |  16 +++
 fs/xfs/xfs_ioctl.c                       |   8 +-
 fs/xfs/xfs_iops.c                        |  16 ++-
 fs/xfs/xfs_pnfs.c                        |  15 +--
 fs/xfs/xfs_pnfs.h                        |   5 +-
 include/linux/dax.h                      |  12 ++
 include/linux/device-mapper.h            |   5 +-
 include/linux/memremap.h                 |  36 ++----
 include/linux/mm.h                       |  71 ++++++++---
 include/linux/string.h                   |   4 +-
 include/linux/uio.h                      |  15 +++
 kernel/Makefile                          |   3 +-
 kernel/iomem.c                           | 167 ++++++++++++++++++++++++
 kernel/memremap.c                        | 210 +++++--------------------------
 kernel/resource.c                        |   1 +
 lib/Kconfig                              |   3 +
 lib/iov_iter.c                           |  61 +++++++++
 mm/Kconfig                               |   5 +
 mm/gup.c                                 |  36 ++++--
 mm/hmm.c                                 |  13 +-
 mm/swap.c                                |   3 +-
 tools/testing/nvdimm/test/nfit.c         | 104 +++++++++++++++
 47 files changed, 1092 insertions(+), 446 deletions(-)
 create mode 100644 Documentation/ABI/removed/sysfs-bus-nfit
 create mode 100644 arch/x86/include/asm/mcsafe_test.h
 create mode 100644 kernel/iomem.c
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[GIT PULL] libnvdimm for 4.18

[Williams, Dan J]

Hi Linus, please pull from:

  git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/libnvdimm-for-4.18

...to receive the first part of the libnvdimm / persistent memory
support update for 4.18. These patches have all been in -next for
several releases. The 'DAX DMA vs Truncate' work has had extra soak
time as it nearly missed 4.17.

This pull request adds a user for the new 'bytes-remaining' updates to
memcpy_mcsafe() that you already received through Ingo via the x86-dax-
for-linus pull. There is a minor collision with bdev_dax_supported()
reworks that you pulled from xfs, and another minor collision of the
vm_fault_t conversion you received from Andrew. A potential merge
resolution is here: https://git.kernel.org/pub/scm/linux/kernel/git/nvd
imm/nvdimm.git/log/?h=libnvdimm-for-4.18-merge

Not included in this pull, but still targeting this cycle, is support
for handling memory media errors (poison) consumed via userspace dax
mappings.

---

The following changes since commit b04e217704b7f879c6b91222b066983a44a7a09f:

  Linux 4.17-rc7 (2018-05-27 13:01:47 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/libnvdimm-for-4.18

for you to fetch changes up to 930218affeadd1325ea17e053f0dcecf218f5a4f:

  Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next (2018-06-08 15:16:44 -0700)

----------------------------------------------------------------
libnvdimm for 4.18

* DAX broke a fundamental assumption of truncate of file mapped pages.
  The truncate path assumed that it is safe to disconnect a pinned page
  from a file and let the filesystem reclaim the physical block. With DAX
  the page is equivalent to the filesystem block. Introduce
  dax_layout_busy_page() to enable filesystems to wait for pinned DAX
  pages to be released. Without this wait a filesystem could allocate
  blocks under active device-DMA to a new file.

* DAX arranges for the block layer to be bypassed and uses
  dax_direct_access() + copy_to_iter() to satisfy read(2) calls.
  However, the memcpy_mcsafe() facility is available through the pmem
  block driver. In order to safely handle media errors, via the DAX
  block-layer bypass, introduce copy_to_iter_mcsafe().

* Fix cache management policy relative to the ACPI NFIT Platform
  Capabilities Structure to properly elide cache flushes when they are not
  necessary. The table indicates whether CPU caches are power-fail
  protected. Clarify that a deep flush is always performed on
  REQ_{FUA,PREFLUSH} requests.

----------------------------------------------------------------
Dan Williams (17):
      memremap: split devm_memremap_pages() and memremap() infrastructure
      mm: introduce MEMORY_DEVICE_FS_DAX and CONFIG_DEV_PAGEMAP_OPS
      mm: fix __gup_device_huge vs unmap
      mm, fs, dax: handle layout changes to pinned dax mappings
      xfs: prepare xfs_break_layouts() to be called with XFS_MMAPLOCK_EXCL
      xfs: prepare xfs_break_layouts() for another layout type
      xfs, dax: introduce xfs_break_dax_layouts()
      uio, lib: Fix CONFIG_ARCH_HAS_UACCESS_MCSAFE compilation
      dax: Introduce a ->copy_to_iter dax operation
      dax: Report bytes remaining in dax_iomap_actor()
      pmem: Switch to copy_to_iter_mcsafe()
      x86, nfit_test: Add unit test for memcpy_mcsafe()
      libnvdimm: Debug probe times
      libnvdimm, e820: Register all pmem resources
      acpi, nfit: Remove ecc_unit_size
      Merge branch 'for-4.18/dax' into libnvdimm-for-next
      Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next

Matthew Wilcox (1):
      dax: dax_insert_mapping_entry always succeeds

Robert Elliott (1):
      linvdimm, pmem: Preserve read-only setting for pmem devices

Ross Zwisler (4):
      libnvdimm, pmem: Complete REQ_FLUSH => REQ_PREFLUSH
      libnvdimm, pmem: Unconditionally deep flush on *sync
      libnvdimm, pmem: Do not flush power-fail protected CPU caches
      dax: Use dax_write_cache* helpers

 Documentation/ABI/removed/sysfs-bus-nfit |  17 +++
 Documentation/ABI/testing/sysfs-bus-nfit |  19 ---
 arch/x86/Kconfig                         |   1 +
 arch/x86/Kconfig.debug                   |   3 +
 arch/x86/include/asm/mcsafe_test.h       |  75 +++++++++++
 arch/x86/include/asm/string_64.h         |  10 +-
 arch/x86/include/asm/uaccess_64.h        |  14 +++
 arch/x86/lib/memcpy_64.S                 | 112 ++++++++---------
 arch/x86/lib/usercopy_64.c               |  21 ++++
 drivers/acpi/nfit/core.c                 |  11 --
 drivers/dax/super.c                      |  33 +++--
 drivers/md/dm-linear.c                   |  16 +++
 drivers/md/dm-log-writes.c               |  15 +++
 drivers/md/dm-stripe.c                   |  21 ++++
 drivers/md/dm.c                          |  25 ++++
 drivers/nvdimm/bus.c                     |  19 ++-
 drivers/nvdimm/claim.c                   |   3 +-
 drivers/nvdimm/e820.c                    |  41 +++---
 drivers/nvdimm/pfn_devs.c                |   2 -
 drivers/nvdimm/pmem.c                    |  52 ++++++--
 drivers/nvdimm/region_devs.c             |   3 +-
 drivers/s390/block/dcssblk.c             |   7 ++
 fs/Kconfig                               |   1 +
 fs/dax.c                                 | 136 ++++++++++++++++----
 fs/xfs/xfs_file.c                        |  72 ++++++++++-
 fs/xfs/xfs_inode.h                       |  16 +++
 fs/xfs/xfs_ioctl.c                       |   8 +-
 fs/xfs/xfs_iops.c                        |  16 ++-
 fs/xfs/xfs_pnfs.c                        |  15 +--
 fs/xfs/xfs_pnfs.h                        |   5 +-
 include/linux/dax.h                      |  12 ++
 include/linux/device-mapper.h            |   5 +-
 include/linux/memremap.h                 |  36 ++----
 include/linux/mm.h                       |  71 ++++++++---
 include/linux/string.h                   |   4 +-
 include/linux/uio.h                      |  15 +++
 kernel/Makefile                          |   3 +-
 kernel/iomem.c                           | 167 ++++++++++++++++++++++++
 kernel/memremap.c                        | 210 +++++--------------------------
 kernel/resource.c                        |   1 +
 lib/Kconfig                              |   3 +
 lib/iov_iter.c                           |  61 +++++++++
 mm/Kconfig                               |   5 +
 mm/gup.c                                 |  36 ++++--
 mm/hmm.c                                 |  13 +-
 mm/swap.c                                |   3 +-
 tools/testing/nvdimm/test/nfit.c         | 104 +++++++++++++++
 47 files changed, 1092 insertions(+), 446 deletions(-)
 create mode 100644 Documentation/ABI/removed/sysfs-bus-nfit
 create mode 100644 arch/x86/include/asm/mcsafe_test.h
 create mode 100644 kernel/iomem.c

[GIT PULL] libnvdimm for 4.18

[Williams, Dan J]

Hi Linus, please pull from:

  git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/libnvdimm-for-4.18

...to receive the first part of the libnvdimm / persistent memory
support update for 4.18. These patches have all been in -next for
several releases. The 'DAX DMA vs Truncate' work has had extra soak
time as it nearly missed 4.17.

This pull request adds a user for the new 'bytes-remaining' updates to
memcpy_mcsafe() that you already received through Ingo via the x86-dax-
for-linus pull. There is a minor collision with bdev_dax_supported()
reworks that you pulled from xfs, and another minor collision of the
vm_fault_t conversion you received from Andrew. A potential merge
resolution is here: https://git.kernel.org/pub/scm/linux/kernel/git/nvd
imm/nvdimm.git/log/?h=libnvdimm-for-4.18-merge

Not included in this pull, but still targeting this cycle, is support
for handling memory media errors (poison) consumed via userspace dax
mappings.

---

The following changes since commit b04e217704b7f879c6b91222b066983a44a7a09f:

  Linux 4.17-rc7 (2018-05-27 13:01:47 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/libnvdimm-for-4.18

for you to fetch changes up to 930218affeadd1325ea17e053f0dcecf218f5a4f:

  Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next (2018-06-08 15:16:44 -0700)

----------------------------------------------------------------
libnvdimm for 4.18

* DAX broke a fundamental assumption of truncate of file mapped pages.
  The truncate path assumed that it is safe to disconnect a pinned page
  from a file and let the filesystem reclaim the physical block. With DAX
  the page is equivalent to the filesystem block. Introduce
  dax_layout_busy_page() to enable filesystems to wait for pinned DAX
  pages to be released. Without this wait a filesystem could allocate
  blocks under active device-DMA to a new file.

* DAX arranges for the block layer to be bypassed and uses
  dax_direct_access() + copy_to_iter() to satisfy read(2) calls.
  However, the memcpy_mcsafe() facility is available through the pmem
  block driver. In order to safely handle media errors, via the DAX
  block-layer bypass, introduce copy_to_iter_mcsafe().

* Fix cache management policy relative to the ACPI NFIT Platform
  Capabilities Structure to properly elide cache flushes when they are not
  necessary. The table indicates whether CPU caches are power-fail
  protected. Clarify that a deep flush is always performed on
  REQ_{FUA,PREFLUSH} requests.

----------------------------------------------------------------
Dan Williams (17):
      memremap: split devm_memremap_pages() and memremap() infrastructure
      mm: introduce MEMORY_DEVICE_FS_DAX and CONFIG_DEV_PAGEMAP_OPS
      mm: fix __gup_device_huge vs unmap
      mm, fs, dax: handle layout changes to pinned dax mappings
      xfs: prepare xfs_break_layouts() to be called with XFS_MMAPLOCK_EXCL
      xfs: prepare xfs_break_layouts() for another layout type
      xfs, dax: introduce xfs_break_dax_layouts()
      uio, lib: Fix CONFIG_ARCH_HAS_UACCESS_MCSAFE compilation
      dax: Introduce a ->copy_to_iter dax operation
      dax: Report bytes remaining in dax_iomap_actor()
      pmem: Switch to copy_to_iter_mcsafe()
      x86, nfit_test: Add unit test for memcpy_mcsafe()
      libnvdimm: Debug probe times
      libnvdimm, e820: Register all pmem resources
      acpi, nfit: Remove ecc_unit_size
      Merge branch 'for-4.18/dax' into libnvdimm-for-next
      Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next

Matthew Wilcox (1):
      dax: dax_insert_mapping_entry always succeeds

Robert Elliott (1):
      linvdimm, pmem: Preserve read-only setting for pmem devices

Ross Zwisler (4):
      libnvdimm, pmem: Complete REQ_FLUSH => REQ_PREFLUSH
      libnvdimm, pmem: Unconditionally deep flush on *sync
      libnvdimm, pmem: Do not flush power-fail protected CPU caches
      dax: Use dax_write_cache* helpers

 Documentation/ABI/removed/sysfs-bus-nfit |  17 +++
 Documentation/ABI/testing/sysfs-bus-nfit |  19 ---
 arch/x86/Kconfig                         |   1 +
 arch/x86/Kconfig.debug                   |   3 +
 arch/x86/include/asm/mcsafe_test.h       |  75 +++++++++++
 arch/x86/include/asm/string_64.h         |  10 +-
 arch/x86/include/asm/uaccess_64.h        |  14 +++
 arch/x86/lib/memcpy_64.S                 | 112 ++++++++---------
 arch/x86/lib/usercopy_64.c               |  21 ++++
 drivers/acpi/nfit/core.c                 |  11 --
 drivers/dax/super.c                      |  33 +++--
 drivers/md/dm-linear.c                   |  16 +++
 drivers/md/dm-log-writes.c               |  15 +++
 drivers/md/dm-stripe.c                   |  21 ++++
 drivers/md/dm.c                          |  25 ++++
 drivers/nvdimm/bus.c                     |  19 ++-
 drivers/nvdimm/claim.c                   |   3 +-
 drivers/nvdimm/e820.c                    |  41 +++---
 drivers/nvdimm/pfn_devs.c                |   2 -
 drivers/nvdimm/pmem.c                    |  52 ++++++--
 drivers/nvdimm/region_devs.c             |   3 +-
 drivers/s390/block/dcssblk.c             |   7 ++
 fs/Kconfig                               |   1 +
 fs/dax.c                                 | 136 ++++++++++++++++----
 fs/xfs/xfs_file.c                        |  72 ++++++++++-
 fs/xfs/xfs_inode.h                       |  16 +++
 fs/xfs/xfs_ioctl.c                       |   8 +-
 fs/xfs/xfs_iops.c                        |  16 ++-
 fs/xfs/xfs_pnfs.c                        |  15 +--
 fs/xfs/xfs_pnfs.h                        |   5 +-
 include/linux/dax.h                      |  12 ++
 include/linux/device-mapper.h            |   5 +-
 include/linux/memremap.h                 |  36 ++----
 include/linux/mm.h                       |  71 ++++++++---
 include/linux/string.h                   |   4 +-
 include/linux/uio.h                      |  15 +++
 kernel/Makefile                          |   3 +-
 kernel/iomem.c                           | 167 ++++++++++++++++++++++++
 kernel/memremap.c                        | 210 +++++--------------------------
 kernel/resource.c                        |   1 +
 lib/Kconfig                              |   3 +
 lib/iov_iter.c                           |  61 +++++++++
 mm/Kconfig                               |   5 +
 mm/gup.c                                 |  36 ++++--
 mm/hmm.c                                 |  13 +-
 mm/swap.c                                |   3 +-
 tools/testing/nvdimm/test/nfit.c         | 104 +++++++++++++++
 47 files changed, 1092 insertions(+), 446 deletions(-)
 create mode 100644 Documentation/ABI/removed/sysfs-bus-nfit
 create mode 100644 arch/x86/include/asm/mcsafe_test.h
 create mode 100644 kernel/iomem.c

Re: [GIT PULL] libnvdimm for 4.18

[Linus Torvalds]

On Fri, Jun 8, 2018 at 4:58 PM Williams, Dan J <dan.j.williams@intel.com> wrote:
>
> Not included in this pull, but still targeting this cycle, is support
> for handling memory media errors (poison) consumed via userspace dax
> mappings.

Note that since I'm going to be traveling much of next week, I'm going
to be even less open to late pull requests than usual.

If I get stuff that looks at all complex at the end of the merge
window, I will just cackle unpleasantly while I press the big 'D' key
on my keyboard.

          Linus
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [GIT PULL] libnvdimm for 4.18

[Linus Torvalds]

On Fri, Jun 8, 2018 at 4:58 PM Williams, Dan J <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>
> Not included in this pull, but still targeting this cycle, is support
> for handling memory media errors (poison) consumed via userspace dax
> mappings.

Note that since I'm going to be traveling much of next week, I'm going
to be even less open to late pull requests than usual.

If I get stuff that looks at all complex at the end of the merge
window, I will just cackle unpleasantly while I press the big 'D' key
on my keyboard.

          Linus

Re: [GIT PULL] libnvdimm for 4.18

[Linus Torvalds]

On Fri, Jun 8, 2018 at 4:58 PM Williams, Dan J <dan.j.williams@intel.com> wrote:
>
> Not included in this pull, but still targeting this cycle, is support
> for handling memory media errors (poison) consumed via userspace dax
> mappings.

Note that since I'm going to be traveling much of next week, I'm going
to be even less open to late pull requests than usual.

If I get stuff that looks at all complex at the end of the merge
window, I will just cackle unpleasantly while I press the big 'D' key
on my keyboard.

          Linus

Re: [GIT PULL] libnvdimm for 4.18

[Linus Torvalds]

On Fri, Jun 8, 2018 at 5:19 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> If I get stuff that looks at all complex at the end of the merge
> window, I will just cackle unpleasantly while I press the big 'D' key
> on my keyboard.

Side note: looking at what I just pulled, there was close to a D key here too.

Dammit, the top commit in your tree is a merge. And the merge message
for that merge is this:

    Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next

That's it. One line. That doesn't say anything at all.

That kind of uninformative commit message wouldn't be remotely
acceptable for a regular simple one-liner patch.

WHY THE HELL DO PEOPLE CONTINUE TO THINK THAT IT'S OK FOR MERGES?

Dammit. Merges are *more* complex than random usual patches. They need
proper commit messages. Yet you have two merges there with absolutely
*no* information in the commit message.

If you can't be bothered to write an informative commit message for a
merge, you damn well shouldn't do the merge.

It really is that simple.

                     Linus
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [GIT PULL] libnvdimm for 4.18

[Linus Torvalds]

On Fri, Jun 8, 2018 at 5:19 PM Linus Torvalds
<torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
>
> If I get stuff that looks at all complex at the end of the merge
> window, I will just cackle unpleasantly while I press the big 'D' key
> on my keyboard.

Side note: looking at what I just pulled, there was close to a D key here too.

Dammit, the top commit in your tree is a merge. And the merge message
for that merge is this:

    Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next

That's it. One line. That doesn't say anything at all.

That kind of uninformative commit message wouldn't be remotely
acceptable for a regular simple one-liner patch.

WHY THE HELL DO PEOPLE CONTINUE TO THINK THAT IT'S OK FOR MERGES?

Dammit. Merges are *more* complex than random usual patches. They need
proper commit messages. Yet you have two merges there with absolutely
*no* information in the commit message.

If you can't be bothered to write an informative commit message for a
merge, you damn well shouldn't do the merge.

It really is that simple.

                     Linus

Re: [GIT PULL] libnvdimm for 4.18

[Linus Torvalds]

On Fri, Jun 8, 2018 at 5:19 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> If I get stuff that looks at all complex at the end of the merge
> window, I will just cackle unpleasantly while I press the big 'D' key
> on my keyboard.

Side note: looking at what I just pulled, there was close to a D key here too.

Dammit, the top commit in your tree is a merge. And the merge message
for that merge is this:

    Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next

That's it. One line. That doesn't say anything at all.

That kind of uninformative commit message wouldn't be remotely
acceptable for a regular simple one-liner patch.

WHY THE HELL DO PEOPLE CONTINUE TO THINK THAT IT'S OK FOR MERGES?

Dammit. Merges are *more* complex than random usual patches. They need
proper commit messages. Yet you have two merges there with absolutely
*no* information in the commit message.

If you can't be bothered to write an informative commit message for a
merge, you damn well shouldn't do the merge.

It really is that simple.

                     Linus

Re: [GIT PULL] libnvdimm for 4.18

[Dan Williams]

On Fri, Jun 8, 2018 at 5:26 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Jun 8, 2018 at 5:19 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> If I get stuff that looks at all complex at the end of the merge
>> window, I will just cackle unpleasantly while I press the big 'D' key
>> on my keyboard.
>
> Side note: looking at what I just pulled, there was close to a D key here too.
>
> Dammit, the top commit in your tree is a merge. And the merge message
> for that merge is this:
>
>     Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next
>
> That's it. One line. That doesn't say anything at all.
>
> That kind of uninformative commit message wouldn't be remotely
> acceptable for a regular simple one-liner patch.
>
> WHY THE HELL DO PEOPLE CONTINUE TO THINK THAT IT'S OK FOR MERGES?
>
> Dammit. Merges are *more* complex than random usual patches. They need
> proper commit messages. Yet you have two merges there with absolutely
> *no* information in the commit message.
>
> If you can't be bothered to write an informative commit message for a
> merge, you damn well shouldn't do the merge.
>
> It really is that simple.

Well, crap. I've been doing it the wrong way for a while. Do you have
a preference for more pull requests or just splitting what is now a
top level tag message into a summary changelog per branch when I merge
the ready branches for the merge window? I had been assuming that the
arrangement you have with Ingo / Thomas to pull individual topics was
a privilege for the tip tree and not necessarily something everyone
that sends you pulls should be doing.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [GIT PULL] libnvdimm for 4.18

[Dan Williams]

On Fri, Jun 8, 2018 at 5:26 PM, Linus Torvalds
<torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
> On Fri, Jun 8, 2018 at 5:19 PM Linus Torvalds
> <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
>>
>> If I get stuff that looks at all complex at the end of the merge
>> window, I will just cackle unpleasantly while I press the big 'D' key
>> on my keyboard.
>
> Side note: looking at what I just pulled, there was close to a D key here too.
>
> Dammit, the top commit in your tree is a merge. And the merge message
> for that merge is this:
>
>     Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next
>
> That's it. One line. That doesn't say anything at all.
>
> That kind of uninformative commit message wouldn't be remotely
> acceptable for a regular simple one-liner patch.
>
> WHY THE HELL DO PEOPLE CONTINUE TO THINK THAT IT'S OK FOR MERGES?
>
> Dammit. Merges are *more* complex than random usual patches. They need
> proper commit messages. Yet you have two merges there with absolutely
> *no* information in the commit message.
>
> If you can't be bothered to write an informative commit message for a
> merge, you damn well shouldn't do the merge.
>
> It really is that simple.

Well, crap. I've been doing it the wrong way for a while. Do you have
a preference for more pull requests or just splitting what is now a
top level tag message into a summary changelog per branch when I merge
the ready branches for the merge window? I had been assuming that the
arrangement you have with Ingo / Thomas to pull individual topics was
a privilege for the tip tree and not necessarily something everyone
that sends you pulls should be doing.

Re: [GIT PULL] libnvdimm for 4.18

[Dan Williams]

On Fri, Jun 8, 2018 at 5:26 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Jun 8, 2018 at 5:19 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> If I get stuff that looks at all complex at the end of the merge
>> window, I will just cackle unpleasantly while I press the big 'D' key
>> on my keyboard.
>
> Side note: looking at what I just pulled, there was close to a D key here too.
>
> Dammit, the top commit in your tree is a merge. And the merge message
> for that merge is this:
>
>     Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next
>
> That's it. One line. That doesn't say anything at all.
>
> That kind of uninformative commit message wouldn't be remotely
> acceptable for a regular simple one-liner patch.
>
> WHY THE HELL DO PEOPLE CONTINUE TO THINK THAT IT'S OK FOR MERGES?
>
> Dammit. Merges are *more* complex than random usual patches. They need
> proper commit messages. Yet you have two merges there with absolutely
> *no* information in the commit message.
>
> If you can't be bothered to write an informative commit message for a
> merge, you damn well shouldn't do the merge.
>
> It really is that simple.

Well, crap. I've been doing it the wrong way for a while. Do you have
a preference for more pull requests or just splitting what is now a
top level tag message into a summary changelog per branch when I merge
the ready branches for the merge window? I had been assuming that the
arrangement you have with Ingo / Thomas to pull individual topics was
a privilege for the tip tree and not necessarily something everyone
that sends you pulls should be doing.

Re: [GIT PULL] libnvdimm for 4.18

[Linus Torvalds]

On Sat, Jun 9, 2018 at 8:17 AM Dan Williams <dan.j.williams@intel.com> wrote:
>
> Well, crap. I've been doing it the wrong way for a while.

.. you're not the only one ;*(

I only really catch it when it's very obvious, like it was now when
the last merge was just before so it stood out like a sore thumb when
I looked at the resulting "git log".

.. or then I catch it in the (happily rare) case when a merge causes
issues, and then I curse.

> Do you have a preference for more pull requests or just splitting what
> is now a top level tag message into a summary changelog per branch
> when I merge the ready branches for the merge window? I had been
> assuming that the arrangement you have with Ingo / Thomas to pull
> individual topics was a privilege for the tip tree and not necessarily
> something everyone that sends you pulls should be doing.

Oh, the -tip tree arrangement is absolutely not a "privilege" for the
-tip tree. In fact, there are a few other trees that I have basically
_required_ do the same because not doing it was too messy. So now I
pull the security subsystem trees as 4-5 different separate branches
too, for example. (And obviously the ARM tree is the "historically
nasty" one that hasn't actually been a problem for several years, but
the solution was the same: split it up.

So I'm perfectly happy getting more than one pull request from everybody.

That said, I also don't want to get _pointless_ pull requests. I don't
want people to split their pulls "just because". It needs to make
sense.

And I *am* perfectly happy with you doing your merges too - I actually
like seeing submaintainers using topic branches for the different
things they do, and do their own merges. Because if I see splits, I
want them to be along some "broader" more generic thing, not - for
example - some "per driver pull request".

But I do ask that when people do their own merges, they actually write
a merge message for that local merge. I will still want the pull
request to talk about the *whole* thing I'm pulling (so you having
details in a local small merge doesn't mean that they shouldn't be
mentioned when then asking me to pull the whole result), so there may
be some duplication in commit messages as things get merged "up the
stack", but that's fine.

Also, the merge message really doesn't have to be extensive. But it
should at least talk about what the topic branch you merged was doing.
Just a sentence or two is fine, unless there's something subtle going
on, in which case that subtle thing needs to be explained.

Examples of "something subtle going on" is when the merge isn't just
for normal development on your own, but is (for example) merging a
common branch that you are also sharing with some other tree, or if
the merge is a back-merge that has some important reason for it.
Hopefully those back-merges don't even happen in the first place, but
if they do, I _really_ want an explanation for _why_ they happened.

For a regular topic branch merge, the "why" is not needed, because
it's "obvious". You're merging development. Then just a short sentence
of two of what the development is. Sometimes the topic branch name
itself *might* be enough, but usually just a _bit_ more information.

So for example, in that

    Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next

merge, even just then adding below that a few sentences saying something like

 "Introduce new machine-check safe copy iterator to dax and make pmem
  use it.

  Add a test-case"

would have been appreciated.

It really doesn't need to be much. Just a "when people do 'git log',
they see what each commit does without having to know the big
picture".

Because imagine doing just 'git log' on my tree. You won't see just
the libnvdimm commits - you'll see tons of random other commits too,
and the commits that get merged by that merge commit are *not* obvious
(because they are probably way down deep in the history, and it's not
so easy to find them in the output of "git log" among all the other
thousands of  commits).

So just making each commit - whether it's a regular commit or a merge
- say what it does without the reader having to know the whole context
really makes it much more understandable.

So to re-iterate: you *can* send those branches individually to me,
and I'll just do three merges instead of one. That works too. But
libnvdimm isn't big enough of a subsystem for me to really necessarily
care at that level, so you doing your own topic branch merges is
*also* good.

But when you do your topic branch merges, imagine that you are me, and
that you're a maintainer that is merging something from a
submaintainer, and write the commit message as if you were a bit of an
outsider that had the submaintainer explain to you what you're
merging.

And simplifying and abstracting is good. Don't try to explain the
commits that get merged at an individual commit level - that's what
the commit messages _in_ those commits are for. The merge should have
a more high-level view of what gets merged.

             Linus
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [GIT PULL] libnvdimm for 4.18

[Linus Torvalds]

On Sat, Jun 9, 2018 at 8:17 AM Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>
> Well, crap. I've been doing it the wrong way for a while.

.. you're not the only one ;*(

I only really catch it when it's very obvious, like it was now when
the last merge was just before so it stood out like a sore thumb when
I looked at the resulting "git log".

.. or then I catch it in the (happily rare) case when a merge causes
issues, and then I curse.

> Do you have a preference for more pull requests or just splitting what
> is now a top level tag message into a summary changelog per branch
> when I merge the ready branches for the merge window? I had been
> assuming that the arrangement you have with Ingo / Thomas to pull
> individual topics was a privilege for the tip tree and not necessarily
> something everyone that sends you pulls should be doing.

Oh, the -tip tree arrangement is absolutely not a "privilege" for the
-tip tree. In fact, there are a few other trees that I have basically
_required_ do the same because not doing it was too messy. So now I
pull the security subsystem trees as 4-5 different separate branches
too, for example. (And obviously the ARM tree is the "historically
nasty" one that hasn't actually been a problem for several years, but
the solution was the same: split it up.

So I'm perfectly happy getting more than one pull request from everybody.

That said, I also don't want to get _pointless_ pull requests. I don't
want people to split their pulls "just because". It needs to make
sense.

And I *am* perfectly happy with you doing your merges too - I actually
like seeing submaintainers using topic branches for the different
things they do, and do their own merges. Because if I see splits, I
want them to be along some "broader" more generic thing, not - for
example - some "per driver pull request".

But I do ask that when people do their own merges, they actually write
a merge message for that local merge. I will still want the pull
request to talk about the *whole* thing I'm pulling (so you having
details in a local small merge doesn't mean that they shouldn't be
mentioned when then asking me to pull the whole result), so there may
be some duplication in commit messages as things get merged "up the
stack", but that's fine.

Also, the merge message really doesn't have to be extensive. But it
should at least talk about what the topic branch you merged was doing.
Just a sentence or two is fine, unless there's something subtle going
on, in which case that subtle thing needs to be explained.

Examples of "something subtle going on" is when the merge isn't just
for normal development on your own, but is (for example) merging a
common branch that you are also sharing with some other tree, or if
the merge is a back-merge that has some important reason for it.
Hopefully those back-merges don't even happen in the first place, but
if they do, I _really_ want an explanation for _why_ they happened.

For a regular topic branch merge, the "why" is not needed, because
it's "obvious". You're merging development. Then just a short sentence
of two of what the development is. Sometimes the topic branch name
itself *might* be enough, but usually just a _bit_ more information.

So for example, in that

    Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next

merge, even just then adding below that a few sentences saying something like

 "Introduce new machine-check safe copy iterator to dax and make pmem
  use it.

  Add a test-case"

would have been appreciated.

It really doesn't need to be much. Just a "when people do 'git log',
they see what each commit does without having to know the big
picture".

Because imagine doing just 'git log' on my tree. You won't see just
the libnvdimm commits - you'll see tons of random other commits too,
and the commits that get merged by that merge commit are *not* obvious
(because they are probably way down deep in the history, and it's not
so easy to find them in the output of "git log" among all the other
thousands of  commits).

So just making each commit - whether it's a regular commit or a merge
- say what it does without the reader having to know the whole context
really makes it much more understandable.

So to re-iterate: you *can* send those branches individually to me,
and I'll just do three merges instead of one. That works too. But
libnvdimm isn't big enough of a subsystem for me to really necessarily
care at that level, so you doing your own topic branch merges is
*also* good.

But when you do your topic branch merges, imagine that you are me, and
that you're a maintainer that is merging something from a
submaintainer, and write the commit message as if you were a bit of an
outsider that had the submaintainer explain to you what you're
merging.

And simplifying and abstracting is good. Don't try to explain the
commits that get merged at an individual commit level - that's what
the commit messages _in_ those commits are for. The merge should have
a more high-level view of what gets merged.

             Linus

Re: [GIT PULL] libnvdimm for 4.18

[Linus Torvalds]

On Sat, Jun 9, 2018 at 8:17 AM Dan Williams <dan.j.williams@intel.com> wrote:
>
> Well, crap. I've been doing it the wrong way for a while.

.. you're not the only one ;*(

I only really catch it when it's very obvious, like it was now when
the last merge was just before so it stood out like a sore thumb when
I looked at the resulting "git log".

.. or then I catch it in the (happily rare) case when a merge causes
issues, and then I curse.

> Do you have a preference for more pull requests or just splitting what
> is now a top level tag message into a summary changelog per branch
> when I merge the ready branches for the merge window? I had been
> assuming that the arrangement you have with Ingo / Thomas to pull
> individual topics was a privilege for the tip tree and not necessarily
> something everyone that sends you pulls should be doing.

Oh, the -tip tree arrangement is absolutely not a "privilege" for the
-tip tree. In fact, there are a few other trees that I have basically
_required_ do the same because not doing it was too messy. So now I
pull the security subsystem trees as 4-5 different separate branches
too, for example. (And obviously the ARM tree is the "historically
nasty" one that hasn't actually been a problem for several years, but
the solution was the same: split it up.

So I'm perfectly happy getting more than one pull request from everybody.

That said, I also don't want to get _pointless_ pull requests. I don't
want people to split their pulls "just because". It needs to make
sense.

And I *am* perfectly happy with you doing your merges too - I actually
like seeing submaintainers using topic branches for the different
things they do, and do their own merges. Because if I see splits, I
want them to be along some "broader" more generic thing, not - for
example - some "per driver pull request".

But I do ask that when people do their own merges, they actually write
a merge message for that local merge. I will still want the pull
request to talk about the *whole* thing I'm pulling (so you having
details in a local small merge doesn't mean that they shouldn't be
mentioned when then asking me to pull the whole result), so there may
be some duplication in commit messages as things get merged "up the
stack", but that's fine.

Also, the merge message really doesn't have to be extensive. But it
should at least talk about what the topic branch you merged was doing.
Just a sentence or two is fine, unless there's something subtle going
on, in which case that subtle thing needs to be explained.

Examples of "something subtle going on" is when the merge isn't just
for normal development on your own, but is (for example) merging a
common branch that you are also sharing with some other tree, or if
the merge is a back-merge that has some important reason for it.
Hopefully those back-merges don't even happen in the first place, but
if they do, I _really_ want an explanation for _why_ they happened.

For a regular topic branch merge, the "why" is not needed, because
it's "obvious". You're merging development. Then just a short sentence
of two of what the development is. Sometimes the topic branch name
itself *might* be enough, but usually just a _bit_ more information.

So for example, in that

    Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next

merge, even just then adding below that a few sentences saying something like

 "Introduce new machine-check safe copy iterator to dax and make pmem
  use it.

  Add a test-case"

would have been appreciated.

It really doesn't need to be much. Just a "when people do 'git log',
they see what each commit does without having to know the big
picture".

Because imagine doing just 'git log' on my tree. You won't see just
the libnvdimm commits - you'll see tons of random other commits too,
and the commits that get merged by that merge commit are *not* obvious
(because they are probably way down deep in the history, and it's not
so easy to find them in the output of "git log" among all the other
thousands of  commits).

So just making each commit - whether it's a regular commit or a merge
- say what it does without the reader having to know the whole context
really makes it much more understandable.

So to re-iterate: you *can* send those branches individually to me,
and I'll just do three merges instead of one. That works too. But
libnvdimm isn't big enough of a subsystem for me to really necessarily
care at that level, so you doing your own topic branch merges is
*also* good.

But when you do your topic branch merges, imagine that you are me, and
that you're a maintainer that is merging something from a
submaintainer, and write the commit message as if you were a bit of an
outsider that had the submaintainer explain to you what you're
merging.

And simplifying and abstracting is good. Don't try to explain the
commits that get merged at an individual commit level - that's what
the commit messages _in_ those commits are for. The merge should have
a more high-level view of what gets merged.

             Linus

Re: [GIT PULL] libnvdimm for 4.18

[Dan Williams]

On Sat, Jun 9, 2018 at 9:26 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Sat, Jun 9, 2018 at 8:17 AM Dan Williams <dan.j.williams@intel.com> wrote:
>>
>> Well, crap. I've been doing it the wrong way for a while.
>
> .. you're not the only one ;*(
>
> I only really catch it when it's very obvious, like it was now when
> the last merge was just before so it stood out like a sore thumb when
> I looked at the resulting "git log".
>
> .. or then I catch it in the (happily rare) case when a merge causes
> issues, and then I curse.
>
>> Do you have a preference for more pull requests or just splitting what
>> is now a top level tag message into a summary changelog per branch
>> when I merge the ready branches for the merge window? I had been
>> assuming that the arrangement you have with Ingo / Thomas to pull
>> individual topics was a privilege for the tip tree and not necessarily
>> something everyone that sends you pulls should be doing.
>
> Oh, the -tip tree arrangement is absolutely not a "privilege" for the
> -tip tree. In fact, there are a few other trees that I have basically
> _required_ do the same because not doing it was too messy. So now I
> pull the security subsystem trees as 4-5 different separate branches
> too, for example. (And obviously the ARM tree is the "historically
> nasty" one that hasn't actually been a problem for several years, but
> the solution was the same: split it up.
>
> So I'm perfectly happy getting more than one pull request from everybody.
>
> That said, I also don't want to get _pointless_ pull requests. I don't
> want people to split their pulls "just because". It needs to make
> sense.
>
> And I *am* perfectly happy with you doing your merges too - I actually
> like seeing submaintainers using topic branches for the different
> things they do, and do their own merges. Because if I see splits, I
> want them to be along some "broader" more generic thing, not - for
> example - some "per driver pull request".
>
> But I do ask that when people do their own merges, they actually write
> a merge message for that local merge. I will still want the pull
> request to talk about the *whole* thing I'm pulling (so you having
> details in a local small merge doesn't mean that they shouldn't be
> mentioned when then asking me to pull the whole result), so there may
> be some duplication in commit messages as things get merged "up the
> stack", but that's fine.
>
> Also, the merge message really doesn't have to be extensive. But it
> should at least talk about what the topic branch you merged was doing.
> Just a sentence or two is fine, unless there's something subtle going
> on, in which case that subtle thing needs to be explained.
>
> Examples of "something subtle going on" is when the merge isn't just
> for normal development on your own, but is (for example) merging a
> common branch that you are also sharing with some other tree, or if
> the merge is a back-merge that has some important reason for it.
> Hopefully those back-merges don't even happen in the first place, but
> if they do, I _really_ want an explanation for _why_ they happened.
>
> For a regular topic branch merge, the "why" is not needed, because
> it's "obvious". You're merging development. Then just a short sentence
> of two of what the development is. Sometimes the topic branch name
> itself *might* be enough, but usually just a _bit_ more information.
>
> So for example, in that
>
>     Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next
>
> merge, even just then adding below that a few sentences saying something like
>
>  "Introduce new machine-check safe copy iterator to dax and make pmem
>   use it.
>
>   Add a test-case"
>
> would have been appreciated.
>
> It really doesn't need to be much. Just a "when people do 'git log',
> they see what each commit does without having to know the big
> picture".
>
> Because imagine doing just 'git log' on my tree. You won't see just
> the libnvdimm commits - you'll see tons of random other commits too,
> and the commits that get merged by that merge commit are *not* obvious
> (because they are probably way down deep in the history, and it's not
> so easy to find them in the output of "git log" among all the other
> thousands of  commits).
>
> So just making each commit - whether it's a regular commit or a merge
> - say what it does without the reader having to know the whole context
> really makes it much more understandable.
>
> So to re-iterate: you *can* send those branches individually to me,
> and I'll just do three merges instead of one. That works too. But
> libnvdimm isn't big enough of a subsystem for me to really necessarily
> care at that level, so you doing your own topic branch merges is
> *also* good.
>
> But when you do your topic branch merges, imagine that you are me, and
> that you're a maintainer that is merging something from a
> submaintainer, and write the commit message as if you were a bit of an
> outsider that had the submaintainer explain to you what you're
> merging.
>
> And simplifying and abstracting is good. Don't try to explain the
> commits that get merged at an individual commit level - that's what
> the commit messages _in_ those commits are for. The merge should have
> a more high-level view of what gets merged.

Thanks Linus. This is great, I'm going to collect this and some your
other advice into the Maintainer Guide so we have a central document.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [GIT PULL] libnvdimm for 4.18

[Dan Williams]

On Sat, Jun 9, 2018 at 9:26 AM, Linus Torvalds
<torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
> On Sat, Jun 9, 2018 at 8:17 AM Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>>
>> Well, crap. I've been doing it the wrong way for a while.
>
> .. you're not the only one ;*(
>
> I only really catch it when it's very obvious, like it was now when
> the last merge was just before so it stood out like a sore thumb when
> I looked at the resulting "git log".
>
> .. or then I catch it in the (happily rare) case when a merge causes
> issues, and then I curse.
>
>> Do you have a preference for more pull requests or just splitting what
>> is now a top level tag message into a summary changelog per branch
>> when I merge the ready branches for the merge window? I had been
>> assuming that the arrangement you have with Ingo / Thomas to pull
>> individual topics was a privilege for the tip tree and not necessarily
>> something everyone that sends you pulls should be doing.
>
> Oh, the -tip tree arrangement is absolutely not a "privilege" for the
> -tip tree. In fact, there are a few other trees that I have basically
> _required_ do the same because not doing it was too messy. So now I
> pull the security subsystem trees as 4-5 different separate branches
> too, for example. (And obviously the ARM tree is the "historically
> nasty" one that hasn't actually been a problem for several years, but
> the solution was the same: split it up.
>
> So I'm perfectly happy getting more than one pull request from everybody.
>
> That said, I also don't want to get _pointless_ pull requests. I don't
> want people to split their pulls "just because". It needs to make
> sense.
>
> And I *am* perfectly happy with you doing your merges too - I actually
> like seeing submaintainers using topic branches for the different
> things they do, and do their own merges. Because if I see splits, I
> want them to be along some "broader" more generic thing, not - for
> example - some "per driver pull request".
>
> But I do ask that when people do their own merges, they actually write
> a merge message for that local merge. I will still want the pull
> request to talk about the *whole* thing I'm pulling (so you having
> details in a local small merge doesn't mean that they shouldn't be
> mentioned when then asking me to pull the whole result), so there may
> be some duplication in commit messages as things get merged "up the
> stack", but that's fine.
>
> Also, the merge message really doesn't have to be extensive. But it
> should at least talk about what the topic branch you merged was doing.
> Just a sentence or two is fine, unless there's something subtle going
> on, in which case that subtle thing needs to be explained.
>
> Examples of "something subtle going on" is when the merge isn't just
> for normal development on your own, but is (for example) merging a
> common branch that you are also sharing with some other tree, or if
> the merge is a back-merge that has some important reason for it.
> Hopefully those back-merges don't even happen in the first place, but
> if they do, I _really_ want an explanation for _why_ they happened.
>
> For a regular topic branch merge, the "why" is not needed, because
> it's "obvious". You're merging development. Then just a short sentence
> of two of what the development is. Sometimes the topic branch name
> itself *might* be enough, but usually just a _bit_ more information.
>
> So for example, in that
>
>     Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next
>
> merge, even just then adding below that a few sentences saying something like
>
>  "Introduce new machine-check safe copy iterator to dax and make pmem
>   use it.
>
>   Add a test-case"
>
> would have been appreciated.
>
> It really doesn't need to be much. Just a "when people do 'git log',
> they see what each commit does without having to know the big
> picture".
>
> Because imagine doing just 'git log' on my tree. You won't see just
> the libnvdimm commits - you'll see tons of random other commits too,
> and the commits that get merged by that merge commit are *not* obvious
> (because they are probably way down deep in the history, and it's not
> so easy to find them in the output of "git log" among all the other
> thousands of  commits).
>
> So just making each commit - whether it's a regular commit or a merge
> - say what it does without the reader having to know the whole context
> really makes it much more understandable.
>
> So to re-iterate: you *can* send those branches individually to me,
> and I'll just do three merges instead of one. That works too. But
> libnvdimm isn't big enough of a subsystem for me to really necessarily
> care at that level, so you doing your own topic branch merges is
> *also* good.
>
> But when you do your topic branch merges, imagine that you are me, and
> that you're a maintainer that is merging something from a
> submaintainer, and write the commit message as if you were a bit of an
> outsider that had the submaintainer explain to you what you're
> merging.
>
> And simplifying and abstracting is good. Don't try to explain the
> commits that get merged at an individual commit level - that's what
> the commit messages _in_ those commits are for. The merge should have
> a more high-level view of what gets merged.

Thanks Linus. This is great, I'm going to collect this and some your
other advice into the Maintainer Guide so we have a central document.

Re: [GIT PULL] libnvdimm for 4.18

[Dan Williams]

On Sat, Jun 9, 2018 at 9:26 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Sat, Jun 9, 2018 at 8:17 AM Dan Williams <dan.j.williams@intel.com> wrote:
>>
>> Well, crap. I've been doing it the wrong way for a while.
>
> .. you're not the only one ;*(
>
> I only really catch it when it's very obvious, like it was now when
> the last merge was just before so it stood out like a sore thumb when
> I looked at the resulting "git log".
>
> .. or then I catch it in the (happily rare) case when a merge causes
> issues, and then I curse.
>
>> Do you have a preference for more pull requests or just splitting what
>> is now a top level tag message into a summary changelog per branch
>> when I merge the ready branches for the merge window? I had been
>> assuming that the arrangement you have with Ingo / Thomas to pull
>> individual topics was a privilege for the tip tree and not necessarily
>> something everyone that sends you pulls should be doing.
>
> Oh, the -tip tree arrangement is absolutely not a "privilege" for the
> -tip tree. In fact, there are a few other trees that I have basically
> _required_ do the same because not doing it was too messy. So now I
> pull the security subsystem trees as 4-5 different separate branches
> too, for example. (And obviously the ARM tree is the "historically
> nasty" one that hasn't actually been a problem for several years, but
> the solution was the same: split it up.
>
> So I'm perfectly happy getting more than one pull request from everybody.
>
> That said, I also don't want to get _pointless_ pull requests. I don't
> want people to split their pulls "just because". It needs to make
> sense.
>
> And I *am* perfectly happy with you doing your merges too - I actually
> like seeing submaintainers using topic branches for the different
> things they do, and do their own merges. Because if I see splits, I
> want them to be along some "broader" more generic thing, not - for
> example - some "per driver pull request".
>
> But I do ask that when people do their own merges, they actually write
> a merge message for that local merge. I will still want the pull
> request to talk about the *whole* thing I'm pulling (so you having
> details in a local small merge doesn't mean that they shouldn't be
> mentioned when then asking me to pull the whole result), so there may
> be some duplication in commit messages as things get merged "up the
> stack", but that's fine.
>
> Also, the merge message really doesn't have to be extensive. But it
> should at least talk about what the topic branch you merged was doing.
> Just a sentence or two is fine, unless there's something subtle going
> on, in which case that subtle thing needs to be explained.
>
> Examples of "something subtle going on" is when the merge isn't just
> for normal development on your own, but is (for example) merging a
> common branch that you are also sharing with some other tree, or if
> the merge is a back-merge that has some important reason for it.
> Hopefully those back-merges don't even happen in the first place, but
> if they do, I _really_ want an explanation for _why_ they happened.
>
> For a regular topic branch merge, the "why" is not needed, because
> it's "obvious". You're merging development. Then just a short sentence
> of two of what the development is. Sometimes the topic branch name
> itself *might* be enough, but usually just a _bit_ more information.
>
> So for example, in that
>
>     Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next
>
> merge, even just then adding below that a few sentences saying something like
>
>  "Introduce new machine-check safe copy iterator to dax and make pmem
>   use it.
>
>   Add a test-case"
>
> would have been appreciated.
>
> It really doesn't need to be much. Just a "when people do 'git log',
> they see what each commit does without having to know the big
> picture".
>
> Because imagine doing just 'git log' on my tree. You won't see just
> the libnvdimm commits - you'll see tons of random other commits too,
> and the commits that get merged by that merge commit are *not* obvious
> (because they are probably way down deep in the history, and it's not
> so easy to find them in the output of "git log" among all the other
> thousands of  commits).
>
> So just making each commit - whether it's a regular commit or a merge
> - say what it does without the reader having to know the whole context
> really makes it much more understandable.
>
> So to re-iterate: you *can* send those branches individually to me,
> and I'll just do three merges instead of one. That works too. But
> libnvdimm isn't big enough of a subsystem for me to really necessarily
> care at that level, so you doing your own topic branch merges is
> *also* good.
>
> But when you do your topic branch merges, imagine that you are me, and
> that you're a maintainer that is merging something from a
> submaintainer, and write the commit message as if you were a bit of an
> outsider that had the submaintainer explain to you what you're
> merging.
>
> And simplifying and abstracting is good. Don't try to explain the
> commits that get merged at an individual commit level - that's what
> the commit messages _in_ those commits are for. The merge should have
> a more high-level view of what gets merged.

Thanks Linus. This is great, I'm going to collect this and some your
other advice into the Maintainer Guide so we have a central document.