[urk, html email.. forgive the mess]
On Mon, Jul 15, 2019 at 04:59:39PM +1000, Dave Airlie wrote:
> VMware had some mm helpers go in via my tree (looking back I'm
> not sure Thomas really secured enough acks on these, but I'm
I saw those patches, honestly I couldn't entirely understand what
problem they were trying to address..
> going with it for now until I get push back). They conflicted
> with one of the mm cleanups in the hmm tree, I've pushed a
> patch to the top of my next to fix most of the fallout in my
> tree, and the resulting fixup is to pick the closure->ptefn
> hunk and apply something like in mm/memory.c
Did I mess a notification from StephenR in linux-next? I was unwaware
of this conflict?
The 'hmm' tree is something I ran to try and help workflow issues like
this, as it could be merged to DRM as a topic branch - maybe consider
this flow in future?
Linus, do you have any advice on how best to handle sharing mm
patches? The hmm.git was mildly painful as it sits between quilt on
the -mm side and what seems like 'a world of interesting git things'
on the DRM side (but maybe I just don't know enough about DRM).
> @@ -2201,7 +2162,7 @@ static int apply_to_page_range_wrapper(pte_t
> *pte,
> struct page_range_apply *pra =
> container_of(pter, typeof(*pra), pter);
> - return pra->fn(pte, NULL, addr, pra->data);
> + return pra->fn(pte, addr, pra->data);
> }
I looked through this and it looks OK to me, thanks
Jason
Hi Jason,
On Mon, 15 Jul 2019 12:29:28 +0000 Jason Gunthorpe <[email protected]> wrote:
>
> On Mon, Jul 15, 2019 at 04:59:39PM +1000, Dave Airlie wrote:
>
> > going with it for now until I get push back). They conflicted
> > with one of the mm cleanups in the hmm tree, I've pushed a
> > patch to the top of my next to fix most of the fallout in my
> > tree, and the resulting fixup is to pick the closure->ptefn
> > hunk and apply something like in mm/memory.c
>
> Did I mess a notification from StephenR in linux-next? I was unwaware
> of this conflict?
That conflict was with Andrew's akpm-current tree, not the hmm tree ...
(on June 24)
"Today's linux-next merge of the akpm-current tree got a conflict in:
mm/memory.c
between commit:
29875a52915e ("mm: Add an apply_to_pfn_range interface")
from the drm tree and commit:
e972cea08fb3 ("mm/pgtable: drop pgtable_t variable from pte_fn_t functions")
from the akpm-current tree."
--
Cheers,
Stephen Rothwell
On Mon, Jul 15, 2019 at 2:29 PM Jason Gunthorpe <[email protected]> wrote:
>
> [urk, html email.. forgive the mess]
>
> On Mon, Jul 15, 2019 at 04:59:39PM +1000, Dave Airlie wrote:
>
> > VMware had some mm helpers go in via my tree (looking back I'm
> > not sure Thomas really secured enough acks on these, but I'm
>
> I saw those patches, honestly I couldn't entirely understand what
> problem they were trying to address..
>
> > going with it for now until I get push back). They conflicted
> > with one of the mm cleanups in the hmm tree, I've pushed a
> > patch to the top of my next to fix most of the fallout in my
> > tree, and the resulting fixup is to pick the closure->ptefn
> > hunk and apply something like in mm/memory.c
>
> Did I mess a notification from StephenR in linux-next? I was unwaware
> of this conflict?
>
> The 'hmm' tree is something I ran to try and help workflow issues like
> this, as it could be merged to DRM as a topic branch - maybe consider
> this flow in future?
>
> Linus, do you have any advice on how best to handle sharing mm
> patches? The hmm.git was mildly painful as it sits between quilt on
> the -mm side and what seems like 'a world of interesting git things'
> on the DRM side (but maybe I just don't know enough about DRM).
I think the approach in this merge window worked fairly well:
- refactor/rework core mm stuff in (h)mm.git
- handle all the gpu stuff in drm.git
- make the clashes workable through some clever prep patches like
we've done this time around
I think Linus wants to be able to look through core mm stuff quite
closely, so not a good idea if we deeply intertwin it with one of the
biggest subsystems there is. And I don't think there will be a real
conflict like this every merge window, this should be the exception.
Worst case we have to stage some work 1 release cycle apart, i.e.
merge mm stuff first, then drm 3 months later. Usually that's not
going to slow things down noticeable given average merge latency for
core mm features :-)
-Daniel
> > @@ -2201,7 +2162,7 @@ static int apply_to_page_range_wrapper(pte_t
> > *pte,
> > struct page_range_apply *pra =
> > container_of(pter, typeof(*pra), pter);
> > - return pra->fn(pte, NULL, addr, pra->data);
> > + return pra->fn(pte, addr, pra->data);
> > }
>
> I looked through this and it looks OK to me, thanks
>
> Jason
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Mon, Jul 15, 2019 at 04:19:26PM +0200, Daniel Vetter wrote:
> > Linus, do you have any advice on how best to handle sharing mm
> > patches? The hmm.git was mildly painful as it sits between quilt on
> > the -mm side and what seems like 'a world of interesting git things'
> > on the DRM side (but maybe I just don't know enough about DRM).
>
> I think the approach in this merge window worked fairly well:
> - refactor/rework core mm stuff in (h)mm.git
> - handle all the gpu stuff in drm.git
> - make the clashes workable through some clever prep patches like
> we've done this time around
Okay, as long as we agree.
I think part of the challenge with hmm.git was setting some
expectation for managing conflicts - there is some happy middle ground
between none & scary we need to navigate to make this work.
> I think Linus wants to be able to look through core mm stuff quite
> closely, so not a good idea if we deeply intertwin it with one of the
> biggest subsystems there is.
There is definately a bunch of stuff that is under the mm/ directory
but is not quite 'core mm' - it would be good if we could rely on
mailing list review of that part and use a topic workflow to manage
things. This was/is more or less my plan with hmm.git
Otherwise yes, as much as reasonable we should avoid topic merges
between the trees.
However, I again see conflict risk coming this cycle ..
> And I don't think there will be a real conflict like this every
> merge window, this should be the exception. Worst case we have to
> stage some work 1 release cycle apart, i.e. merge mm stuff first,
> then drm 3 months later.
I really dislike this idea. Not being able to provide users for core
APIs is a huge process/review problem. Worse it tends to create a
'ladder' where in-flight changes to drivers (to implement the new
core) now block any changes to the core APIs on alternating cycles. So
'the big pitcture' starts to fall a part if we can't co-ordinate cross
tree changes in some way.
.. and honestly, splitting a review process across a 9 week gap is one
of the best ways I've seen to get a poor quality review :(
I'm pretty familiar with this problem, we had it in spades between RDMA
and netdev for a long time, and it is finally fully resolved now
through a careful use of topic branches and merges.
For example, next week I'll queue CH's patches that shift the last
batch of hmm 'conflict management' shims into nouveau, and tries to
fix them:
https://patchwork.kernel.org/project/linux-mm/list/?series=141835
This is something uncontroversial that really should go into the DRM
world as a topic so it doesn't interfere with ongoing nouveau dev. But
I want to keep the hmm.c/.h bits in the hmm.git to avoid conflicts.
So, I'll put it on a topic and send you two a note next week to decide
if you want to merge it or not. I'm really unclear how nouveau's and
AMD's patchflow works..
Thanks.
Jason
On Mon, Jul 15, 2019 at 5:04 PM Jason Gunthorpe <[email protected]> wrote:
>
> On Mon, Jul 15, 2019 at 04:19:26PM +0200, Daniel Vetter wrote:
>
> > > Linus, do you have any advice on how best to handle sharing mm
> > > patches? The hmm.git was mildly painful as it sits between quilt on
> > > the -mm side and what seems like 'a world of interesting git things'
> > > on the DRM side (but maybe I just don't know enough about DRM).
> >
> > I think the approach in this merge window worked fairly well:
> > - refactor/rework core mm stuff in (h)mm.git
> > - handle all the gpu stuff in drm.git
> > - make the clashes workable through some clever prep patches like
> > we've done this time around
>
> Okay, as long as we agree.
>
> I think part of the challenge with hmm.git was setting some
> expectation for managing conflicts - there is some happy middle ground
> between none & scary we need to navigate to make this work.
>
> > I think Linus wants to be able to look through core mm stuff quite
> > closely, so not a good idea if we deeply intertwin it with one of the
> > biggest subsystems there is.
>
> There is definately a bunch of stuff that is under the mm/ directory
> but is not quite 'core mm' - it would be good if we could rely on
> mailing list review of that part and use a topic workflow to manage
> things. This was/is more or less my plan with hmm.git
>
> Otherwise yes, as much as reasonable we should avoid topic merges
> between the trees.
>
> However, I again see conflict risk coming this cycle ..
>
> > And I don't think there will be a real conflict like this every
> > merge window, this should be the exception. Worst case we have to
> > stage some work 1 release cycle apart, i.e. merge mm stuff first,
> > then drm 3 months later.
>
> I really dislike this idea. Not being able to provide users for core
> APIs is a huge process/review problem. Worse it tends to create a
> 'ladder' where in-flight changes to drivers (to implement the new
> core) now block any changes to the core APIs on alternating cycles. So
> 'the big pitcture' starts to fall a part if we can't co-ordinate cross
> tree changes in some way.
>
> .. and honestly, splitting a review process across a 9 week gap is one
> of the best ways I've seen to get a poor quality review :(
>
> I'm pretty familiar with this problem, we had it in spades between RDMA
> and netdev for a long time, and it is finally fully resolved now
> through a careful use of topic branches and merges.
>
> For example, next week I'll queue CH's patches that shift the last
> batch of hmm 'conflict management' shims into nouveau, and tries to
> fix them:
>
> https://patchwork.kernel.org/project/linux-mm/list/?series=141835
>
> This is something uncontroversial that really should go into the DRM
> world as a topic so it doesn't interfere with ongoing nouveau dev. But
> I want to keep the hmm.c/.h bits in the hmm.git to avoid conflicts.
>
> So, I'll put it on a topic and send you two a note next week to decide
> if you want to merge it or not. I'm really unclear how nouveau's and
> AMD's patchflow works..
DRM is 2-level for pretty much everything. First it lands in a driver
tree (or a collectiv of drivers, like in drm-misc). Then those send
pull requests to drm.git for integration. Busy trees do that every 1-2
weeks (e.g. amdgpu), slower trees once per merge window (e.g. nouveau)
for drm-next, similar for drm-fixes.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Mon, Jul 15, 2019 at 07:53:06PM +0200, Daniel Vetter wrote:
> > So, I'll put it on a topic and send you two a note next week to decide
> > if you want to merge it or not. I'm really unclear how nouveau's and
> > AMD's patchflow works..
>
> DRM is 2-level for pretty much everything. First it lands in a driver
> tree (or a collectiv of drivers, like in drm-misc). Then those send
> pull requests to drm.git for integration. Busy trees do that every 1-2
> weeks (e.g. amdgpu), slower trees once per merge window (e.g. nouveau)
> for drm-next, similar for drm-fixes.
The DRM part seems logical - it is how the AMD GPU and nouveau git
trees trees work that I don't know. I heard that neither could take in
a stable topic branch?
Jason
On Mon, Jul 15, 2019 at 7:57 PM Jason Gunthorpe <[email protected]> wrote:
>
> On Mon, Jul 15, 2019 at 07:53:06PM +0200, Daniel Vetter wrote:
>
> > > So, I'll put it on a topic and send you two a note next week to decide
> > > if you want to merge it or not. I'm really unclear how nouveau's and
> > > AMD's patchflow works..
> >
> > DRM is 2-level for pretty much everything. First it lands in a driver
> > tree (or a collectiv of drivers, like in drm-misc). Then those send
> > pull requests to drm.git for integration. Busy trees do that every 1-2
> > weeks (e.g. amdgpu), slower trees once per merge window (e.g. nouveau)
> > for drm-next, similar for drm-fixes.
>
> The DRM part seems logical - it is how the AMD GPU and nouveau git
> trees trees work that I don't know. I heard that neither could take in
> a stable topic branch?
Hm yeah they're a bit special. Nouveau is mostly developed in
userspace, at least by it's main developer Ben Skeggs. But generally
he's taking care of shuffling patches back&forth using scripts.
Usually easier to do any merges and stuff in drm.git though.
AMD is a different case, they've disappeared quite a lot behind the
amd firewalls with the new codebase. So there the rebasing-before-pull
is for corporate politics reasons it seems, and really not needed for
any technical reasons (unlike nouveau, where reverse-engineering hw in
userspace is a lot nicer than crashing kernels). Dave&me have been
nagging them for a few years now to fix this.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[ Ugh, I have three different threads about the drm pull because of
the subject / html confusion. So now I'm replying in separate threads
and I'm hoping the people involved have better threading than gmail
does ;/ ]
On Mon, Jul 15, 2019 at 5:29 AM Jason Gunthorpe <[email protected]> wrote:
>
> The 'hmm' tree is something I ran to try and help workflow issues like
> this, as it could be merged to DRM as a topic branch - maybe consider
> this flow in future?
>
> Linus, do you have any advice on how best to handle sharing mm
> patches?
I don't have a lot of advice except for "very very carefully".
I think the hmm tree worked really well this merge window, at least
from my standpoint.
But it is of course possible that my happiness about the hmm tree is a
complete fluke and came about because pretty much all the patches were
removing oddities and cleaning things up, and they weren't adding new
odd things (or if they were, you hid it better ;^).
Basically, people should know that there are some subsystems that I
end up being *much* more worried about than others. If I see a pull
request that modifies core VM of VFS code, I tend to go into "careful
mode", in ways that I don't do when some maintainer sends me a pull
request that obviously only changes some subtree that the maintainer
owns.
When driver maintainers send me patches that touch their drivers, I
look at the diffstat.
But when driver maintainers send me patches that change mm/, I look at
individual commit messages and the actual diff itself, and if I see
contentious stuff, I simply won't pull.
It's why I left the hmm pull request (which came in early - thank you)
until yesterday, simply because just from the diffstat I could tell
that "ok, this merge isn't just me merging and doing sanity checks,
this is me having to set aside time to do reviews". So just from the
diffstat, I avoided even looking at it while I still had a mailbox
full of other pull requests.
So I do like seeing core mm changes come in through a separate branch.
That's partly because that way it's easier to review without having
the parts I care about be mixed up with other parts (it's one reason I
asked the security layer pulls to be split up, to pick another area
entirely as an example). But it's also partly because if you have a
separate branch for the stuff that you know is contentious, that
doesn't then hold up the parts that _aren't_.
For example, right now I'm not pulling _any_ of the drm updates,
simply because there's a part of it that I can't stomach. It would
have been so much nicer if the contentious things that need a lot of
care separate, and I could have at least pulled the other stuff.
Linus
On Mon, Jul 15, 2019 at 11:16 AM Linus Torvalds
<[email protected]> wrote:
>
> On Mon, Jul 15, 2019 at 5:29 AM Jason Gunthorpe <[email protected]> wrote:
> >
> > The 'hmm' tree is something I ran to try and help workflow issues like
> > this, as it could be merged to DRM as a topic branch - maybe consider
> > this flow in future?
> >
> > Linus, do you have any advice on how best to handle sharing mm
> > patches?
>
> I don't have a lot of advice except for "very very carefully".
>
> I think the hmm tree worked really well this merge window, at least
> from my standpoint.
Side note: I suspect that having a separate branch maintained by a
separate person actually does help the "very carefully" part.
I think the hmm branch ended up getting more "incidental review"
simply because of how it was done. So even if the original reason for
the separate branch was to resolve some quilt/git integration issues,
I would not be at all surprised if just the extra indirection through
another person ended up making both the sending and receiving side
think more about each patch and think more about the abstraction.
The hmm branch didn't actually seem to have any of the core VM people
reviewing it either, but it did have reviewers across companies for
all the patches, and I do think that that makes a difference.
It's _soo_ easy for a patch series to be developed inside one company
by a couple of people who are probably in the same group, and have the
exact same objectives, to be a lot more biased (and likely biased not
towards the mm goals, but the goals of the code _outside_ the mm).
This is just a long-winded way to say "I do think that the separate
and external branch with multiple interested parties" can have some
inherent advantages, when you actually have multiple people looking at
it with care and intent.
(And here the fact that you have multiple subsystems looking at the
code is very much part of what makes it a good model - if it was just
an external branch for one single user - the vmware gfx driver - you
wouldn't get the same kind of advantages. So it's not the "external
branch" part itself, it's the "multiple users who care" part that
likely causes people to think more about the end result)
Again - maybe I'm rationalizing, and the hmm branch just randomly
happened to work well this time. I do like having multiple people from
different groups look at things, though.
Linus
On Mon, Jul 15, 2019 at 11:16:11AM -0700, Linus Torvalds wrote:
> [ Ugh, I have three different threads about the drm pull because of
> the subject / html confusion. So now I'm replying in separate threads
> and I'm hoping the people involved have better threading than gmail
> does ;/ ]
>
> On Mon, Jul 15, 2019 at 5:29 AM Jason Gunthorpe <[email protected]> wrote:
> >
> > The 'hmm' tree is something I ran to try and help workflow issues like
> > this, as it could be merged to DRM as a topic branch - maybe consider
> > this flow in future?
> >
> > Linus, do you have any advice on how best to handle sharing mm
> > patches?
>
> I don't have a lot of advice except for "very very carefully".
>
> I think the hmm tree worked really well this merge window, at least
> from my standpoint.
>
> But it is of course possible that my happiness about the hmm tree is a
> complete fluke and came about because pretty much all the patches were
> removing oddities and cleaning things up, and they weren't adding new
> odd things (or if they were, you hid it better ;^).
lol
Actually I think it was a lot of effort from many people to monitor
and stay on top of conflicts, and there was certainly a deliberate
effort to bring many people together.
About the only thing I could concretely suggest for working with -mm
is if there was some way the -mm quilt patches could participate in
'git merge' resolution at your level.
I only say this because the lowest point was when merging CH's series
to hmm.git caused Andrew to have to do a lot of work rebasing DanW's
series during rc7. Arguably that should have been my work preparing a
conflict resolution instruction, not his doing rebases.
.. and if we needed to revise hmm.git Dan's series would have been at
more risk. It kind of still is as I haven't seen him Ack Andrew's
rebase yet?
Thanks,
Jason
On Mon, Jul 15, 2019 at 12:17 PM Jason Gunthorpe <[email protected]> wrote:
>
> About the only thing I could concretely suggest for working with -mm
> is if there was some way the -mm quilt patches could participate in
> 'git merge' resolution at your level.
Andrew did make noises about having multiple git branches. We'll see.
Linus