On 6/9/21 5:28 PM, Darrick J. Wong wrote:
On Thu, Jun 10, 2021 at 08:03:39AM +1000, Dave Chinner wrote:
On Wed, Jun 09, 2021 at 12:44:47PM -0700, Allison Henderson wrote:
Hi Darrick,
I've created a branch and tag for the delay ready attribute series. I'ved
added the rvbs since the last review, but otherwise it is unchanged since
v20.
Please pull from the tag decsribed below.
Yay! At last! Good work, Allison. :)
Yes, indeed. Pulled!
Great! Very exciting, I think this chunk was probably the more complex
of the sub series for parent pointers.
Nothing to worry about here, but I thought I'd make an observation
on the construction branches for pull requests seeing as pull
request are becoming our way of saying "this code is ready to
merge".
Thanks!
Allison
The following changes since commit 0fe0bbe00a6fb77adf75085b7d06b71a830dd6f2:
xfs: bunmapi has unnecessary AG lock ordering issues (2021-05-27 08:11:24 -0700)
This looks like it has been built on top of a specific commit in the
linux-xfs tree - perhaps a for-next branch before all the most
recent development branches have been merged in.
Yes, it's the xfs-5.13-fixes-3 tag at the end of the 5.13 fixes branch.
Yes, i try to stay on top of the announcements when i see them
The problem with doing this is that the for-next tree can rebase,
which can leave your tree with orphaned commits that are no longer
in the main development branch or upstream. While these commits
are upstream now, this isn't an issue for this particular branch
and pull request.
i.e. if the recent rebase of the for-next branch rewrote the above
commit, pulling this branch would cause all sorts of problems.
So to avoid this sort of issue with pull requests, and to allow the
maintainer (darrick) to be able to easily reformulate the for-next
tree just by merging branches, pull requests should all come from a
known upstream base. In the case of pull requests for the xfs tree,
that known base is the master branch of the XFS tree.
This is a good point. Branches should be based off of something that's
stable, recent, and relatively close to the current development work.
Ideally that would be for-next, but I hadn't actually declared that
stable yet since I just started accepting pull requests and wanted a
couple of days to make sure nothing totally weird happened with Stephen
Rothwell's integration merge.
With for-next in flux, basing your branch off the end of the fixes
branch, or an upstream Linus release some time after that, are good
enough choices... since I hadn't updated xfs-linux.git#master in a
while.
For the past 4.5 years, the pattern has always been that the most recent
fixes branch (xfs-5.X-fixes) gets merged into upstream before I create
the xfs-5.(X+1)-merge branch. This could get murky if I ever have
enough bandwidth to be building a fixes branch and a merge branch at the
same time, but TBH if xfs is so unstable that we /need/ fixes past -rc4
then we really should concentrate on that at the expense of merging new
code.
I guess that means I should be updating xfs-linux.git#master to point to
the most recent -rc with any Xfs changes in it.
The only time that you wouldn't do this is when your work is
dependent on some other set of fixes. Those fixes then need to be
in a stable upstream branch somewhere, which you then merge into
your own dev branch based on xfs/master and the put your own commits
on top of. IOWs, you start your own branch with a merge commit...
If you do this, you should note in the pull request that there are
other branches merged into this pull request and where they came
from. THat way the maintainer can determine if the branch is
actually stable and will end up being merged upstream unchanged from
it's current state.
It is also nice to tell the maintainer that you've based the branch
on a stable XFS commit ahead of the current master branch. This
might be necessary because there's a dependency between multiple
development branches that are being merged one at a time in seperate
pull requests.
Agreed.
Got it. Will do, I do sort of run into that from time to time.
In terms of workflow, what this means is that development is done on
a xfs/master based branch. i.e. dev branches are built like this:
$ git checkout -b dev-branch-1 xfs/master
$ git merge remote/stable-dev-branch
$ git merge local-dependent-dev-branch
$ <apply all your changes>
$ <build kernel and test>
And for testing against the latest -rc (say 5.13-rc5) and for-next
kernels you do:
$ git checkout -b testing v5.13-rc5
$ git merge xfs/for-next
$ git merge dev-branch-1
<resolve conflicts>
$ git merge dev-branch-2
<resolve conflicts>
....
$ git merge dev-branch-N
<resolve conflicts>
$ <build kernel and test>
Whee, the modern era... :)
Sure, I will hang on to these instructions then
This means that each dev branch has all the correct dependencies
built into it, and they can be pulled by anyone without perturbing
their local tree for testing and review because they are all working
on the same xfs/master branch as your branches are.
This also means that the xfs/for-next tree can remain based on
xfs/master and be reformulated against xfs/master in a repeatable
manner. It just makes everything easier if all pull requests are
sent from the same stable base commit...
Anyway, this isn't an issue for this pull-req because it is based on
a stable XFS commit in a branch based on xfs/master, but I thought
it's worth pointing out the pitfalls of using random stable commits
as the base for pull requests so everyone knows what they should be
doing as it's not really documented anywhere. :)
Agreed, though this isn't entirely a "random stable commit", it's the
end of the most recent stable branch.
Right, i try to stay in top of the latest branch, but this way makes
sense too. I think it's good to establish a common procedure for people
to use so that everyone knows what to expect. Thanks for the examples!
Allison
--D
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
