Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1602025 > unrolled thread

Re: [Intel-gfx] The i915 stable patch marking is totally broken

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [Intel-gfx] The i915 stable patch marking is totally broken Daniel Vetter <daniel@ffwll.ch> - 2017-03-16 08:40 +0100
    Re: [Intel-gfx] The i915 stable patch marking is totally broken Greg KH <gregkh@linuxfoundation.org> - 2017-03-16 15:10 +0100
      Re: [Intel-gfx] The i915 stable patch marking is totally broken Jani Nikula <jani.nikula@linux.intel.com> - 2017-03-16 15:50 +0100
        Re: [Intel-gfx] The i915 stable patch marking is totally broken Greg KH <gregkh@linuxfoundation.org> - 2017-03-17 02:40 +0100

#1602025 — Re: [Intel-gfx] The i915 stable patch marking is totally broken

Hi Greg,

On Mon, Mar 13, 2017 at 07:40:50AM +0100, Daniel Vetter wrote:
> On Sun, Mar 12, 2017 at 11:01 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > So if a commit says "cherry-pick", I guess I can always assume it's safe
> > to add, right?  If not, _then_ I have to run the "search backwards"
> > logic, right?
> >
> > Ok, let me think about this a bit to see if that's possible to script...
> 
> Yes, but it shouldn't be hard to avoid the linear search:
> 
> 1. make sure you have the latest linux-next (to make sure all the sha1
> commit-ish resolve to something meaningful). You probably want to do
> that before you board a plane :-)
> 
> 2. When you parse an upstream commit that says "commit cherry-picked
> from $original_sha1", then add a git note for $original_sha1 that
> you've seen it already and can ignore it.
> 
> 3. Run that script over v4.9..v4.10 to backfill your git notes branch.
> 
> 4. Make sure you sync that git notes branch (and if you use git notes
> already, just use a different git notes branch name to avoid
> conflicts).
> 
> 5. When you spot a patch with cc: stable, check for a git note that
> says you've looked at it (or one of it's cherry-picks) already, if so,
> silently ignore it.
> 
> That should massively drop the ratio of failed patches, at least every
> time I look at your failed patche mail I think they're just
> double-applied ones. There's ofc a few patches that fail to apply, 3
> months of drm/i915 development even wreak the context of simple
> bugfixes sometimes, but most are not (which is btw why you don't get
> replies for most of these).

Are you implementing this? If you need inspiration, we also have a fairly
generic cherry-pick branch command, which filters out duplicated cherry
picks already with:

    git log drm-intel-fixes --format=format:%h --after=6months \
		    --grep="cherry picked .* $commit"

See https://cgit.freedesktop.org/drm-intel/tree/dim?h=maintainer-tools#n713

Please make sure you have something like this ready soon, otherwise we're
going to have this exact conversation again, like we did for the last few
merge windows ... :(

If you can't implement this, then I guess we have to try to avoid
double-tagging stuff with cc: stable. But that will work against 10+ years
of "pls cc: stable bugfixes" training from you. And we'd need to predict
when exactly the merge window cutoff is. Which is going to get it wrong by
1-2 weeks each release, so trying to fix this on our side will be at best
an 80% solution, after 1y of hard re-trainig work :(

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [next] | [standalone]

#1602307

On Thu, Mar 16, 2017 at 08:38:30AM +0100, Daniel Vetter wrote:
> Hi Greg,
> 
> On Mon, Mar 13, 2017 at 07:40:50AM +0100, Daniel Vetter wrote:
> > On Sun, Mar 12, 2017 at 11:01 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > So if a commit says "cherry-pick", I guess I can always assume it's safe
> > > to add, right?  If not, _then_ I have to run the "search backwards"
> > > logic, right?
> > >
> > > Ok, let me think about this a bit to see if that's possible to script...
> > 
> > Yes, but it shouldn't be hard to avoid the linear search:
> > 
> > 1. make sure you have the latest linux-next (to make sure all the sha1
> > commit-ish resolve to something meaningful). You probably want to do
> > that before you board a plane :-)
> > 
> > 2. When you parse an upstream commit that says "commit cherry-picked
> > from $original_sha1", then add a git note for $original_sha1 that
> > you've seen it already and can ignore it.
> > 
> > 3. Run that script over v4.9..v4.10 to backfill your git notes branch.
> > 
> > 4. Make sure you sync that git notes branch (and if you use git notes
> > already, just use a different git notes branch name to avoid
> > conflicts).
> > 
> > 5. When you spot a patch with cc: stable, check for a git note that
> > says you've looked at it (or one of it's cherry-picks) already, if so,
> > silently ignore it.
> > 
> > That should massively drop the ratio of failed patches, at least every
> > time I look at your failed patche mail I think they're just
> > double-applied ones. There's ofc a few patches that fail to apply, 3
> > months of drm/i915 development even wreak the context of simple
> > bugfixes sometimes, but most are not (which is btw why you don't get
> > replies for most of these).
> 
> Are you implementing this? If you need inspiration, we also have a fairly
> generic cherry-pick branch command, which filters out duplicated cherry
> picks already with:
> 
>     git log drm-intel-fixes --format=format:%h --after=6months \
> 		    --grep="cherry picked .* $commit"
> 
> See https://cgit.freedesktop.org/drm-intel/tree/dim?h=maintainer-tools#n713
> 
> Please make sure you have something like this ready soon, otherwise we're
> going to have this exact conversation again, like we did for the last few
> merge windows ... :(
> 
> If you can't implement this, then I guess we have to try to avoid
> double-tagging stuff with cc: stable. But that will work against 10+ years
> of "pls cc: stable bugfixes" training from you. And we'd need to predict
> when exactly the merge window cutoff is. Which is going to get it wrong by
> 1-2 weeks each release, so trying to fix this on our side will be at best
> an 80% solution, after 1y of hard re-trainig work :(

Sorry, I haven't had the chance to look at this again.

But, I still think this is wrong, you are getting commits into Linus's
tree that have git commit ids that hopefully show up 3 months later.
That feels bad from a "consistency" point of view.

Why not switch it around, and apply the patch to your "stable" branch
and then cherry-pick it to your "next" branch?  That way I can just
ignore any patch that has "cherry-pick" in it, not ever need to mess
with 'git notes' (not that I probably would anyway, they are horrid),
and the tree is always semi-sane.  And it would prevent me from having
to mess with linux-next, which I also don't want to have to do.
Especially for stable work, that just feels so wrong, as stable stuff
should not be depending on stuff that hasn't even hit Linus's tree yet,
and might never.

And again, you all are the only ones that have this issue.  You might
find a handfull of patches for stable that come in twice in the rest of
the kernel, but your "little" driver dwarfs that by an order of
magnitude.  I really think you are doing it wrong, no one else seems to
have this issue...

I'll be back home next week and look into writing some scripts for this,
but please consider just switching your "which branch does it go into
first" model, which would really save me a ton of time, and remove
confusion from anyone who ever runs across one of these cherry-pick
messages.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1602405

On Thu, 16 Mar 2017, Greg KH <gregkh@linuxfoundation.org> wrote:
> And again, you all are the only ones that have this issue.  You might
> find a handfull of patches for stable that come in twice in the rest of
> the kernel, but your "little" driver dwarfs that by an order of
> magnitude.  I really think you are doing it wrong, no one else seems to
> have this issue...

Just perhaps we have really active development with lots of diligence in
tagging fixes with Fixes: and Cc: stable, and not so many others do?

> I'll be back home next week and look into writing some scripts for this,
> but please consider just switching your "which branch does it go into
> first" model, which would really save me a ton of time, and remove
> confusion from anyone who ever runs across one of these cherry-pick
> messages.

Usually our development branches are months ahead of what's currently
happening in Linus' master. We already have tons of stuff ready for
v4.12, and at around v4.11-rc5 we start aiming at v4.13. This is what
everyone wants us to do, be ready earlier and earlier for the merge
windows.

It is *much* easier for us to grind the fixes through our CI and QA on
our development branches, make sure the fixes are good and compatible
with what's coming ahead, and that the issues stay fixed. When we merge
Linus' master and our -next, we can always trivially resolve the
conflict to what's in our -next, and the fixes are not lost. And if we
find issues with the commits, we can choose to not cherry-pick them
until they're fixed.

In the past, we did have lots of trouble with people fixing issues in
our development branches (because that's what you develop on), and the
fixes would not apply to Linus' master. We'd redo the patch, and end up
with nasty conflicts with what's in -next. We ended up stalling on fixes
in *both* branches. I think we did a much worse job getting things done
with the reverse order of applying fixes, because it was so much harder
for us.

In the end, the model is not unlike the stable workflow. It's just that
stable doesn't merge back with Linus' master.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1602978

On Thu, Mar 16, 2017 at 04:40:01PM +0200, Jani Nikula wrote:
> On Thu, 16 Mar 2017, Greg KH <gregkh@linuxfoundation.org> wrote:
> > And again, you all are the only ones that have this issue.  You might
> > find a handfull of patches for stable that come in twice in the rest of
> > the kernel, but your "little" driver dwarfs that by an order of
> > magnitude.  I really think you are doing it wrong, no one else seems to
> > have this issue...
> 
> Just perhaps we have really active development with lots of diligence in
> tagging fixes with Fixes: and Cc: stable, and not so many others do?

While you might think so, no, lots of other subsystems have lots of
stable patches, you aren't alone there :)

> > I'll be back home next week and look into writing some scripts for this,
> > but please consider just switching your "which branch does it go into
> > first" model, which would really save me a ton of time, and remove
> > confusion from anyone who ever runs across one of these cherry-pick
> > messages.
> 
> Usually our development branches are months ahead of what's currently
> happening in Linus' master. We already have tons of stuff ready for
> v4.12, and at around v4.11-rc5 we start aiming at v4.13. This is what
> everyone wants us to do, be ready earlier and earlier for the merge
> windows.

That's fine, and again, much like everyone else.

> It is *much* easier for us to grind the fixes through our CI and QA on
> our development branches, make sure the fixes are good and compatible
> with what's coming ahead, and that the issues stay fixed. When we merge
> Linus' master and our -next, we can always trivially resolve the
> conflict to what's in our -next, and the fixes are not lost. And if we
> find issues with the commits, we can choose to not cherry-pick them
> until they're fixed.
> 
> In the past, we did have lots of trouble with people fixing issues in
> our development branches (because that's what you develop on), and the
> fixes would not apply to Linus' master. We'd redo the patch, and end up
> with nasty conflicts with what's in -next. We ended up stalling on fixes
> in *both* branches. I think we did a much worse job getting things done
> with the reverse order of applying fixes, because it was so much harder
> for us.

Huh?  You are saying that today you fix things on the development branch
(-next), and then cherry-pick to your -linus branch, right?  That's why
the git hashes are "odd".  But you said that when you did this in the
past you had problems?  I don't understand what is different now.

> In the end, the model is not unlike the stable workflow. It's just that
> stable doesn't merge back with Linus' master.

No, it's very different.  I am "cherry-picking" patches from Linus's
master into the stable branches.  The commits in the whole tree always
refer to another patch that is in the same repo.  None of this "go look
over here in linux-next for something that we hope will land in Linus's
tree in 3 months" type crap.  The sha1 references in a repo should
_always_ be resolvable in that same repo at the same point in time.
Otherwise you are playing a game where you hope things get resolved
sometime in the future.

Again, that's my biggest objection to what you all are doing, I'm amazed
that Linus hasn't complained either.

thanks,

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web