2022-05-26 22:11:03

by Dave Chinner

[permalink] [raw]
Subject: [GIT PULL] xfs: new code for 5.19

Hi Linus,

Can you please pull the XFS updates for 5.19 from the tag listed
below? It merges cleanly with the TOT kernel I just pulled a couple
of minutes ago, though the diffstat I got on merge:

105 files changed, 4862 insertions(+), 2773 deletions(-)

is slightly different to the diffstat the pull request generated.

If I've made any mistakes or done stuff that is considered wrong or
out of date, let me know and I'll fix them up - it's been a while
since I built a tree for upstream merge.

This is a big update with lots of new code. The tag describes them
all, so I'll just touch on teh higlights. The two main new features
are Large Extent Counts and Logged Attribute Replay - these are two
new foundational features that we are building more complex future
features on top of.

For upcoming functionality, we need to be able to store hundreds of
millions of xattrs per inode. The Large Extent Count feature removes
the limits that prevent this scale of xattr storage, and while we
were modifying the on disk extent count format we also increased the
number of data extents we support per inode from 2^32 to 2^47.

We also need to be able to modify xattrs as part of larger atomic
transactions rather than as standalone transactions. The Logged
Attribute Replay feature introduces the infrastructure that allows
us to use intents to record the attribute modifications in the
journal before we start them, hence allowing other atomic
transactions to log attribute modification intents and then defer
the actual modification to later. If we then crash, log recovery
then guarantees that the attribute is replayed in the context of the
atomic transaction that logged the intent.

A significant chunk of the commits in this merge are for the base
attribute replay functionality along with fixes, improvements and
cleanups related to this new functioanlity. Allison deserves a big
round of thanks for her ongoing work to get this functionality into
XFS.

There are also many other smaller changes and improvements, so
overall this is one of the bigger XFS merge requests in some time.

I will be following up next week with another smaller pull request -
we already have another round of fixes and improvements to the
logged attribute replay functionality just about ready to go.
They'll soak and test over the next week, and I'll send a pull
request for them near the end of the merge window.

Thanks!

-Dave.


The following changes since commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1:

xfs: reorder iunlink remove operation in xfs_ifree (2022-04-21 08:45:16 +1000)

are available in the Git repository at:

git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git tags/xfs-5.19-for-linus

for you to fetch changes up to efd409a4329f6927795be5ae080cd3ec8c014f49:

Merge branch 'xfs-5.19-quota-warn-remove' into xfs-5.19-for-next (2022-05-12 15:23:07 +1000)

----------------------------------------------------------------
xfs: Changes for 5.19-rc1

This update includes:
- support for printk message indexing.
- large extent counts to provide support for up to 2^47 data extents and 2^32
attribute extents, allowing us to scale beyond 4 billion data extents
to billions of xattrs per inode.
- conversion of various flags fields to be consistently declared as
unsigned bit fields.
- improvements to realtime extent accounting and converts them to per-cpu
counters to match all the other block and inode accounting.
- reworks core log formatting code to reduce iterations, have a shorter, cleaner
fast path and generally be easier to understand and maintain.
- improvements to rmap btree searches that reduce overhead by up
to 30% resulting in xfs_scrub runtime reductions of 15%.
- improvements to reflink that remove the size limitations in remapping operations
and greatly reduce the size of transaction reservations.
- reworks the minimum log size calculations to allow us to change transaction
reservations without changing the minimum supported log size.
- removal of quota warning support as it has never been used on Linux.
- intent whiteouts to allow us to cancel intents that are completed entirely
in memory rather than having use CPU and disk bandwidth formatting and writing
them into the journal when it is not necessary. This makes rmap, reflink and
extent freeing slightly more efficient, but provides massive improvements
for....
- Logged Attribute Replay feature support. This is a fundamental change to the
way we modify attributes, laying the foundation for future integration of
attribute modifications as part of other atomic transactional operations the
filesystem performs.
- Lots of cleanups and fixes for the logged attribute replay functionality.

----------------------------------------------------------------
Allison Henderson (14):
xfs: Fix double unlock in defer capture code
xfs: Return from xfs_attr_set_iter if there are no more rmtblks to process
xfs: Set up infrastructure for log attribute replay
xfs: Implement attr logging and replay
xfs: Skip flip flags for delayed attrs
xfs: Add xfs_attr_set_deferred and xfs_attr_remove_deferred
xfs: Remove unused xfs_attr_*_args
xfs: Add log attribute error tag
xfs: Add larp debug option
xfs: Merge xfs_delattr_context into xfs_attr_item
xfs: Add helper function xfs_attr_leaf_addname
xfs: Add helper function xfs_init_attr_trans
xfs: add leaf split error tag
xfs: add leaf to node error tag

Brian Foster (1):
xfs: fix soft lockup via spinning in filestream ag selection loop

Catherine Hoang (3):
xfs: remove quota warning limit from struct xfs_quota_limits
xfs: remove warning counters from struct xfs_dquot_res
xfs: don't set quota warning values

Chandan Babu R (19):
xfs: Move extent count limits to xfs_format.h
xfs: Define max extent length based on on-disk format definition
xfs: Introduce xfs_iext_max_nextents() helper
xfs: Use xfs_extnum_t instead of basic data types
xfs: Introduce xfs_dfork_nextents() helper
xfs: Use basic types to define xfs_log_dinode's di_nextents and di_anextents
xfs: Promote xfs_extnum_t and xfs_aextnum_t to 64 and 32-bits respectively
xfs: Introduce XFS_SB_FEAT_INCOMPAT_NREXT64 and associated per-fs feature bit
xfs: Introduce XFS_FSOP_GEOM_FLAGS_NREXT64
xfs: Introduce XFS_DIFLAG2_NREXT64 and associated helpers
xfs: Use uint64_t to count maximum blocks that can be used by BMBT
xfs: Introduce macros to represent new maximum extent counts for data/attr forks
xfs: Replace numbered inode recovery error messages with descriptive ones
xfs: Introduce per-inode 64-bit extent counters
xfs: Directory's data fork extent counter can never overflow
xfs: Conditionally upgrade existing inodes to use large extent counters
xfs: Decouple XFS_IBULK flags from XFS_IWALK flags
xfs: Enable bulkstat ioctl to support 64-bit per-inode extent counters
xfs: Add XFS_SB_FEAT_INCOMPAT_NREXT64 to the list of supported flags

Christoph Hellwig (2):
xfs: change the type of ic_datap
xfs: remove xlog_verify_dest_ptr

Darrick J. Wong (16):
xfs: pass explicit mount pointer to rtalloc query functions
xfs: recalculate free rt extents after log recovery
xfs: use a separate frextents counter for rt extent reservations
xfs: capture buffer ops in the xfs_buf tracepoints
xfs: simplify xfs_rmap_lookup_le call sites
xfs: speed up rmap lookups by using non-overlapped lookups when possible
xfs: speed up write operations by using non-overlapped lookups when possible
xfs: count EFIs when deciding to ask for a continuation of a refcount update
xfs: stop artificially limiting the length of bunmap calls
xfs: remove a __xfs_bunmapi call from reflink
xfs: create shadow transaction reservations for computing minimum log size
xfs: report "max_resp" used for min log size computation
xfs: reduce the absurdly large log operation count
xfs: reduce transaction reservations with reflink
xfs: rewrite xfs_reflink_end_cow to use intents
xfs: rename xfs_*alloc*_log_count to _block_count

Dave Chinner (73):
xfs: factor out the CIL transaction header building
xfs: only CIL pushes require a start record
xfs: embed the xlog_op_header in the unmount record
xfs: embed the xlog_op_header in the commit record
xfs: log tickets don't need log client id
xfs: move log iovec alignment to preparation function
xfs: reserve space and initialise xlog_op_header in item formatting
xfs: log ticket region debug is largely useless
xfs: pass lv chain length into xlog_write()
xfs: introduce xlog_write_full()
xfs: introduce xlog_write_partial()
xfs: xlog_write() no longer needs contwr state
xfs: xlog_write() doesn't need optype anymore
xfs: CIL context doesn't need to count iovecs
xfs: convert attr type flags to unsigned.
xfs: convert scrub type flags to unsigned.
xfs: convert bmap extent type flags to unsigned.
xfs: convert bmapi flags to unsigned.
xfs: convert AGF log flags to unsigned.
xfs: convert AGI log flags to unsigned.
xfs: convert btree buffer log flags to unsigned.
xfs: convert buffer log item flags to unsigned.
xfs: convert da btree operations flags to unsigned.
xfs: convert dquot flags to unsigned.
xfs: convert log item tracepoint flags to unsigned.
xfs: convert inode lock flags to unsigned.
xfs: convert ptag flags to unsigned.
xfs: convert quota options flags to unsigned.
xfs: convert shutdown reasons to unsigned.
xfs: convert log ticket and iclog flags to unsigned.
Merge branch 'guilt/5.19-miscellaneous' into xfs-5.19-for-next
Merge branch 'guilt/xfs-unsigned-flags-5.18' into xfs-5.19-for-next
Merge branch 'guilt/xlog-write-rework' into xfs-5.19-for-next
Merge tag 'large-extent-counters-v9' of https://github.com/chandanr/linux into xfs-5.19-for-next
xfs: zero inode fork buffer at allocation
xfs: fix potential log item leak
xfs: hide log iovec alignment constraints
xfs: don't commit the first deferred transaction without intents
xfs: add log item flags to indicate intents
xfs: tag transactions that contain intent done items
xfs: factor and move some code in xfs_log_cil.c
xfs: add log item method to return related intents
xfs: whiteouts release intents that are not in the AIL
xfs: intent item whiteouts
xfs: detect self referencing btree sibling pointers
xfs: validate inode fork size against fork format
xfs: set XFS_FEAT_NLINK correctly
xfs: validate v5 feature fields
Merge branch 'guilt/xfs-5.19-misc-2' into xfs-5.19-for-next
Merge branch 'guilt/xlog-intent-whiteouts' into xfs-5.19-for-next
Merge tag 'rmap-speedups-5.19_2022-04-28' of git://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-5.19-for-next
Merge tag 'reflink-speedups-5.19_2022-04-28' of git://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-5.19-for-next
Merge branch 'guilt/xfs-5.19-fuzz-fixes' into xfs-5.19-for-next
xfs: avoid empty xattr transaction when attrs are inline
xfs: initialise attrd item to zero
xfs: make xattri_leaf_bp more useful
xfs: rework deferred attribute operation setup
xfs: separate out initial attr_set states
xfs: kill XFS_DAC_LEAF_ADDNAME_INIT
xfs: consolidate leaf/node states in xfs_attr_set_iter
xfs: split remote attr setting out from replace path
xfs: XFS_DAS_LEAF_REPLACE state only needed if !LARP
xfs: remote xattr removal in xfs_attr_set_iter() is conditional
xfs: clean up final attr removal in xfs_attr_set_iter
xfs: xfs_attr_set_iter() does not need to return EAGAIN
xfs: introduce attr remove initial states into xfs_attr_set_iter
xfs: switch attr remove to xfs_attri_set_iter
xfs: remove xfs_attri_remove_iter
xfs: use XFS_DA_OP flags in deferred attr ops
xfs: ATTR_REPLACE algorithm with LARP enabled needs rework
xfs: detect empty attr leaf blocks in xfs_attr3_leaf_verify
xfs: can't use kmem_zalloc() for attribute buffers
Merge branch 'xfs-5.19-quota-warn-remove' into xfs-5.19-for-next

Eric Sandeen (1):
xfs: revert "xfs: actually bump warning counts when we send warnings"

Jonathan Lassoff (2):
xfs: Simplify XFS logging methods.
xfs: Add XFS messages to printk index

Kaixu Xia (1):
xfs: simplify local variable assignment in file write code

Matthew Wilcox (Oracle) (1):
xfs: Use generic_file_open()

Yang Xu (1):
xfs: improve __xfs_set_acl

fs/xfs/Makefile | 1 +
fs/xfs/libxfs/xfs_alloc.c | 12 +-
fs/xfs/libxfs/xfs_alloc.h | 2 +-
fs/xfs/libxfs/xfs_attr.c | 1644 +++++++++++++++++++--------------------
fs/xfs/libxfs/xfs_attr.h | 198 ++++-
fs/xfs/libxfs/xfs_attr_leaf.c | 64 +-
fs/xfs/libxfs/xfs_attr_remote.c | 37 +-
fs/xfs/libxfs/xfs_attr_remote.h | 6 +-
fs/xfs/libxfs/xfs_bmap.c | 167 ++--
fs/xfs/libxfs/xfs_bmap.h | 58 +-
fs/xfs/libxfs/xfs_bmap_btree.c | 9 +-
fs/xfs/libxfs/xfs_btree.c | 150 +++-
fs/xfs/libxfs/xfs_btree.h | 26 +-
fs/xfs/libxfs/xfs_da_btree.c | 4 +
fs/xfs/libxfs/xfs_da_btree.h | 25 +-
fs/xfs/libxfs/xfs_da_format.h | 9 +-
fs/xfs/libxfs/xfs_defer.c | 54 +-
fs/xfs/libxfs/xfs_defer.h | 3 +
fs/xfs/libxfs/xfs_dir2.c | 8 +
fs/xfs/libxfs/xfs_errortag.h | 8 +-
fs/xfs/libxfs/xfs_format.h | 189 +++--
fs/xfs/libxfs/xfs_fs.h | 41 +-
fs/xfs/libxfs/xfs_ialloc.c | 8 +-
fs/xfs/libxfs/xfs_ialloc.h | 2 +-
fs/xfs/libxfs/xfs_inode_buf.c | 118 ++-
fs/xfs/libxfs/xfs_inode_fork.c | 51 +-
fs/xfs/libxfs/xfs_inode_fork.h | 76 +-
fs/xfs/libxfs/xfs_log_format.h | 79 +-
fs/xfs/libxfs/xfs_log_recover.h | 2 +
fs/xfs/libxfs/xfs_log_rlimit.c | 75 +-
fs/xfs/libxfs/xfs_quota_defs.h | 50 +-
fs/xfs/libxfs/xfs_refcount.c | 14 +-
fs/xfs/libxfs/xfs_refcount.h | 13 +-
fs/xfs/libxfs/xfs_rmap.c | 161 ++--
fs/xfs/libxfs/xfs_rmap.h | 7 +-
fs/xfs/libxfs/xfs_rtbitmap.c | 9 +-
fs/xfs/libxfs/xfs_sb.c | 80 +-
fs/xfs/libxfs/xfs_shared.h | 24 +-
fs/xfs/libxfs/xfs_trans_resv.c | 225 ++++--
fs/xfs/libxfs/xfs_trans_resv.h | 16 +-
fs/xfs/libxfs/xfs_types.h | 11 +-
fs/xfs/scrub/bmap.c | 26 +-
fs/xfs/scrub/common.c | 2 +
fs/xfs/scrub/inode.c | 20 +-
fs/xfs/scrub/rtbitmap.c | 9 +-
fs/xfs/xfs_acl.c | 4 +-
fs/xfs/xfs_acl.h | 8 +-
fs/xfs/xfs_attr_item.c | 824 ++++++++++++++++++++
fs/xfs/xfs_attr_item.h | 46 ++
fs/xfs/xfs_attr_list.c | 1 +
fs/xfs/xfs_bmap_item.c | 27 +-
fs/xfs/xfs_bmap_util.c | 27 +-
fs/xfs/xfs_buf_item.h | 24 +-
fs/xfs/xfs_dquot.c | 18 +-
fs/xfs/xfs_dquot.h | 8 -
fs/xfs/xfs_error.c | 9 +
fs/xfs/xfs_error.h | 20 +-
fs/xfs/xfs_extfree_item.c | 23 +-
fs/xfs/xfs_file.c | 24 +-
fs/xfs/xfs_filestream.c | 7 +-
fs/xfs/xfs_fsmap.c | 6 +-
fs/xfs/xfs_fsops.c | 7 +-
fs/xfs/xfs_globals.c | 1 +
fs/xfs/xfs_icache.c | 9 +-
fs/xfs/xfs_icreate_item.c | 1 +
fs/xfs/xfs_inode.c | 80 +-
fs/xfs/xfs_inode.h | 29 +-
fs/xfs/xfs_inode_item.c | 48 +-
fs/xfs/xfs_inode_item_recover.c | 145 +++-
fs/xfs/xfs_ioctl.c | 7 +-
fs/xfs/xfs_ioctl32.c | 2 +
fs/xfs/xfs_iomap.c | 33 +-
fs/xfs/xfs_iops.c | 4 +-
fs/xfs/xfs_itable.c | 15 +-
fs/xfs/xfs_itable.h | 5 +-
fs/xfs/xfs_iwalk.h | 2 +-
fs/xfs/xfs_log.c | 807 +++++++++----------
fs/xfs/xfs_log.h | 90 ++-
fs/xfs/xfs_log_cil.c | 391 ++++++----
fs/xfs/xfs_log_priv.h | 89 +--
fs/xfs/xfs_log_recover.c | 2 +
fs/xfs/xfs_message.c | 58 +-
fs/xfs/xfs_message.h | 55 +-
fs/xfs/xfs_mount.c | 91 ++-
fs/xfs/xfs_mount.h | 32 +-
fs/xfs/xfs_ondisk.h | 2 +
fs/xfs/xfs_qm.c | 9 -
fs/xfs/xfs_qm.h | 5 -
fs/xfs/xfs_qm_syscalls.c | 26 +-
fs/xfs/xfs_quotaops.c | 8 +-
fs/xfs/xfs_refcount_item.c | 25 +-
fs/xfs/xfs_reflink.c | 100 ++-
fs/xfs/xfs_rmap_item.c | 25 +-
fs/xfs/xfs_rtalloc.c | 41 +
fs/xfs/xfs_rtalloc.h | 9 +-
fs/xfs/xfs_super.c | 18 +-
fs/xfs/xfs_symlink.c | 5 -
fs/xfs/xfs_sysctl.h | 1 +
fs/xfs/xfs_sysfs.c | 24 +
fs/xfs/xfs_trace.h | 100 ++-
fs/xfs/xfs_trans.c | 52 +-
fs/xfs/xfs_trans.h | 38 +-
fs/xfs/xfs_trans_dquot.c | 4 +-
fs/xfs/xfs_xattr.c | 2 +-
104 files changed, 4690 insertions(+), 2676 deletions(-)
create mode 100644 fs/xfs/xfs_attr_item.c
create mode 100644 fs/xfs/xfs_attr_item.h
--
Dave Chinner
[email protected]


2022-05-27 00:52:55

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] xfs: new code for 5.19

On Wed, May 25, 2022 at 7:21 PM Dave Chinner <[email protected]> wrote:
>
> Can you please pull the XFS updates for 5.19 from the tag listed
> below? It merges cleanly with the TOT kernel I just pulled a couple
> of minutes ago, though the diffstat I got on merge:
>
> 105 files changed, 4862 insertions(+), 2773 deletions(-)
>
> is slightly different to the diffstat the pull request generated.

Funky. I get the same diffstat that you list below in the pull
request, not the above.

Different diff algorithms do get different results, so the actual line
numbers vary a bit with the default myers vs minimal vs patience vs
histogram algorithms. But while that changes line numbers, it
shouldn't then change the actual number of files...

I wonder what the difference is, but I'm not going to delve into it
further since what I see matches the pull request and it all looks
fine.

I do notice that if I use

git diff -C10 ..

to make git more eagerly find file copies, I get

[...]
104 files changed, 4444 insertions(+), 3125 deletions(-)
copy fs/xfs/{xfs_bmap_item.c => xfs_attr_item.c} (13%)
create mode 100644 fs/xfs/xfs_attr_item.h

and adding "-B/10%" to make git also look for rewrites (ie files that
might be better shown as "remove file and then re-add") gives:

[..]
104 files changed, 5449 insertions(+), 4130 deletions(-)
rewrite fs/xfs/libxfs/xfs_attr.c (43%)
copy fs/xfs/{xfs_bmap_item.c => xfs_attr_item.c} (13%)
create mode 100644 fs/xfs/xfs_attr_item.h
rewrite fs/xfs/xfs_log.h (31%)
rewrite fs/xfs/xfs_message.h (30%)

so it might be something along those lines where our git configs
differ. I couldn't get it to give "105 files", though.

Not important. I just got curious.

And it might also be as simple as you just having had something else
in your tree at the same time, that you didn't want to send, but
forgot about when you did the test-merge. That would be the simplest
explanation..

> If I've made any mistakes or done stuff that is considered wrong or
> out of date, let me know and I'll fix them up - it's been a while
> since I built a tree for upstream merge.

It all looks fine.

I might wish that your merge commit messages were a bit more
consistent about the merge details ("why and what"), but you are most
definitely not the only one with that, and a number of them are quite
nice (ie the merge of the large extent counters has a nice informative
commit message, as does the rmap speedup one).

And then some of them are the uninformative one-lines that just say
"Merge branch X"

Linus

2022-05-27 08:24:57

by Dave Chinner

[permalink] [raw]
Subject: Re: [GIT PULL] xfs: new code for 5.19

On Wed, May 25, 2022 at 07:56:31PM -0700, Linus Torvalds wrote:
> On Wed, May 25, 2022 at 7:21 PM Dave Chinner <[email protected]> wrote:
> >
> > Can you please pull the XFS updates for 5.19 from the tag listed
> > below? It merges cleanly with the TOT kernel I just pulled a couple
> > of minutes ago, though the diffstat I got on merge:
> >
> > 105 files changed, 4862 insertions(+), 2773 deletions(-)
> >
> > is slightly different to the diffstat the pull request generated.
>
> Funky. I get the same diffstat that you list below in the pull
> request, not the above.
>
> Different diff algorithms do get different results, so the actual line
> numbers vary a bit with the default myers vs minimal vs patience vs
> histogram algorithms. But while that changes line numbers, it
> shouldn't then change the actual number of files...
>
> I wonder what the difference is, but I'm not going to delve into it
> further since what I see matches the pull request and it all looks
> fine.
>
> I do notice that if I use
>
> git diff -C10 ..
>
> to make git more eagerly find file copies, I get
>
> [...]
> 104 files changed, 4444 insertions(+), 3125 deletions(-)
> copy fs/xfs/{xfs_bmap_item.c => xfs_attr_item.c} (13%)
> create mode 100644 fs/xfs/xfs_attr_item.h
>
> and adding "-B/10%" to make git also look for rewrites (ie files that
> might be better shown as "remove file and then re-add") gives:
>
> [..]
> 104 files changed, 5449 insertions(+), 4130 deletions(-)
> rewrite fs/xfs/libxfs/xfs_attr.c (43%)
> copy fs/xfs/{xfs_bmap_item.c => xfs_attr_item.c} (13%)
> create mode 100644 fs/xfs/xfs_attr_item.h
> rewrite fs/xfs/xfs_log.h (31%)
> rewrite fs/xfs/xfs_message.h (30%)
>
> so it might be something along those lines where our git configs
> differ. I couldn't get it to give "105 files", though.
>
> Not important. I just got curious.

Weird.

> And it might also be as simple as you just having had something else
> in your tree at the same time, that you didn't want to send, but
> forgot about when you did the test-merge. That would be the simplest
> explanation..

$ git reset --hard origin/master
Updating files: 100% (6994/6994), done.
HEAD is now at d7227785e384 Merge tag 'sound-5.19-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
$ git merge xfs-5.19-for-next
Auto-merging fs/xfs/xfs_file.c
Auto-merging fs/xfs/xfs_log_cil.c
Auto-merging fs/xfs/xfs_super.c
Merge made by the 'ort' strategy.
fs/xfs/Makefile | 1 +
....
fs/xfs/xfs_xattr.c | 2 +-
105 files changed, 4862 insertions(+), 2773 deletions(-)
create mode 100644 fs/xfs/xfs_attr_item.c
create mode 100644 fs/xfs/xfs_attr_item.h
$

> > If I've made any mistakes or done stuff that is considered wrong or
> > out of date, let me know and I'll fix them up - it's been a while
> > since I built a tree for upstream merge.
>
> It all looks fine.
>
> I might wish that your merge commit messages were a bit more
> consistent about the merge details ("why and what"), but you are most
> definitely not the only one with that, and a number of them are quite
> nice (ie the merge of the large extent counters has a nice informative
> commit message, as does the rmap speedup one).

Those one came from pull requests with informative signed
tags. We're trying to move more of our development processes to
using signed pull reqs when eveything is done, so this hopefully
will happen more often.

> And then some of them are the uninformative one-lines that just say
> "Merge branch X"

Yeah, those are merges from local topic branches where I pulled in
individual patches or entire series from the mailing list via 'b4 am
-o - <msg_id> | git am -s'. AFAICT there is no way to have this
retain the patch series cover letter, which generally contains what
I would want to be putting into the merge commit message.

I'll keep that in mind for future composes, though I do wish there
was an easy way to just have b4/git manage cover letters as part of
the topic branch so they can feed into local merge commits just as
easily remote pulls do....

Cheers,

Dave.
--
Dave Chinner
[email protected]

2022-05-29 07:50:19

by Amir Goldstein

[permalink] [raw]
Subject: Re: [GIT PULL] xfs: new code for 5.19

> > I might wish that your merge commit messages were a bit more
> > consistent about the merge details ("why and what"), but you are most
> > definitely not the only one with that, and a number of them are quite
> > nice (ie the merge of the large extent counters has a nice informative
> > commit message, as does the rmap speedup one).
>
> Those one came from pull requests with informative signed
> tags. We're trying to move more of our development processes to
> using signed pull reqs when eveything is done, so this hopefully
> will happen more often.
>
> > And then some of them are the uninformative one-lines that just say
> > "Merge branch X"
>
> Yeah, those are merges from local topic branches where I pulled in
> individual patches or entire series from the mailing list via 'b4 am
> -o - <msg_id> | git am -s'. AFAICT there is no way to have this
> retain the patch series cover letter, which generally contains what
> I would want to be putting into the merge commit message.
>
> I'll keep that in mind for future composes, though I do wish there
> was an easy way to just have b4/git manage cover letters as part of
> the topic branch so they can feed into local merge commits just as
> easily remote pulls do....
>

There is.
I have been hacking on b4 and found many hidden features :)

b4 am [email protected] -n
xfs-5.19-quota-warn-remove.mbx
git am -s xfs-5.19-quota-warn-remove.mbx
git tag -F xfs-5.19-quota-warn-remove.cover xfs-5.19-quota-warn-remove

Konstantine has added the "b4 shazam" combo recently for
'b4 am -o - <msg_id> | git am -s'

The shazam command is not well documented, so most info can be found in
the git log, but that seems like it might be a good place to add an auto tagging
feature. It will also help to include a link to lore in the "topic
tag" to make it
easier for people to get to the developer discussions on the topic.

My dream is that all linux pull requests will have links to lore patch series.

Below is an example output of a gadget I created [1] to help maintainers
and git archaeologists to generate those links automatically from PRs
(pre or post merge).

The gadget is far from perfect, it still has some rough edges, but it fits my
needs so far.
If folks are interested, you are welcome to try it out and provide me
feedback so I can get it in shape for upstream b4.

But the tool won't be needed for maintainers that work with topic
branches if each internal topic merge contains a link to the lore thread
the topic was applied from.

Thanks,
Amir.

[1] https://github.com/amir73il/b4/commits/release-notes

This example analyses a range of commits that did not originate
from a single patch series to demonstrate how an analysis of
PR topics looks like:

$ git show 1499b8a3a37b
commit 1499b8a3a37baf5a78ee8044e9a8fa0471268d74
Merge: 9a5280b312e2 2d9ac4319b99
Author: Dave Chinner <[email protected]>
Date: Thu Apr 21 11:40:17 2022 +1000

Merge branch 'guilt/5.19-miscellaneous' into xfs-5.19-for-next

$ git format-patch 1499b8a3a37b^..1499b8a3a37b^2 --stdout | b4 rn -m - 2>rn.log

---

- [PATH ?/?] xfs: Simplify XFS logging methods.

- [PATCHSET v2 0/3] xfs: fix corruption of free rt extent count
[https://lore.kernel.org/r/164961485474.70555.18228016043917319266.stgit@magnolia]
Tests: xfs/141

- [PATH ?/?] xfs: Add XFS messages to printk index

- [PATCH] xfs: Use generic_file_open()
[https://lore.kernel.org/r/[email protected]]

- [PATH ?/?] xfs: simplify local variable assignment in file write code

---

2022-05-30 08:38:10

by Dave Chinner

[permalink] [raw]
Subject: Re: [GIT PULL] xfs: new code for 5.19

On Sun, May 29, 2022 at 10:32:58AM +0300, Amir Goldstein wrote:
> > > I might wish that your merge commit messages were a bit more
> > > consistent about the merge details ("why and what"), but you are most
> > > definitely not the only one with that, and a number of them are quite
> > > nice (ie the merge of the large extent counters has a nice informative
> > > commit message, as does the rmap speedup one).
> >
> > Those one came from pull requests with informative signed
> > tags. We're trying to move more of our development processes to
> > using signed pull reqs when eveything is done, so this hopefully
> > will happen more often.
> >
> > > And then some of them are the uninformative one-lines that just say
> > > "Merge branch X"
> >
> > Yeah, those are merges from local topic branches where I pulled in
> > individual patches or entire series from the mailing list via 'b4 am
> > -o - <msg_id> | git am -s'. AFAICT there is no way to have this
> > retain the patch series cover letter, which generally contains what
> > I would want to be putting into the merge commit message.
> >
> > I'll keep that in mind for future composes, though I do wish there
> > was an easy way to just have b4/git manage cover letters as part of
> > the topic branch so they can feed into local merge commits just as
> > easily remote pulls do....
> >
>
> There is.
> I have been hacking on b4 and found many hidden features :)
>
> b4 am [email protected] -n
> xfs-5.19-quota-warn-remove.mbx
> git am -s xfs-5.19-quota-warn-remove.mbx
> git tag -F xfs-5.19-quota-warn-remove.cover xfs-5.19-quota-warn-remove

That's a tag on a commit, not a persistent object associated with a
branch. I've considered this, but if I append a new commit, rebase
the branch, or do anything I normally do with topic branches, then
that tag ends up pointing at the wrong commit or even a non-existent
commit. It just adds another thing to forget/get wrong when managing
topic branches for merge. i.e. it doesn't make things simpler.

As it is, I use guilt for managing the contents of all my git
branches in all my git trees. I already have a local hack in guilt
to use the first commit of a series as the series description/cover
letter. I pass a special flag to 'guilt patchbomb' and it turns the
first commit into the cover letter for editing and sending. With
this, I have an object associated with the topic branch that follows
all operations on the branch (including rebases) and so is always
there in the same place.

However, I can't use such topic branches for merges - the series
description commit needs to be purged and the series rebased before
I merge it. You can see this in this old branch here:

https://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs.git/log/?h=xfs-iunlink-item

In that branch there are actually two description commits:

https://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs.git/commit/?h=xfs-iunlink-item&id=35d3b6ac52b5870484182d476cb253021e44acc5
https://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs.git/commit/?h=xfs-iunlink-item&id=a561cb0e09fa7886d034be0ae94f5f77d327014d

because the second line of development (unlinked inode mods) was
dependent on another set of patches (async inode reclaim).

That's the text and changelog for the cover letter for that specific
line of development. As a "here's a topic branch with all the
changes in it" push, I didn't sanitise them.

I think what I'm going to end up doing is add a 'guilt am' command
that runs b4, extracts the cover letter as an internal guilt file
(in .git/patches/<branch>/series-description) and add a `guilt
series -d [-e]` command to print or edit it directly. Then that file
exists, guilt patchbomb will just pick it up. If I add a `guilt
merge` wrapper then it will get picked up as the merge description
automatically, too...

This way the cover letter follows the topic branch no matter what I
do with the branch once I've downloaded it from the mailing list,
and it doesn't show up in the commit history and hence I can merge
the branches easily.

-Dave.
--
Dave Chinner
[email protected]