The i915 stable patch marking is totally broken

The i915 stable patch marking is totally broken

[Greg KH]

Hi Daniel and Jani and other members of the i915-commit-cabal,

I've mentioned this a few times to Daniel in the past (like at the last
kernel summit), but the way you all are handling the tagging of patches
for inclusion in stable kernel releases is totally broken and causing me
no end of headaches.

It's due to your "you have a branch, you have a branch, you all have a
branch!" model of development.  You have patches that end up flowing in
to Linus's trees multiple times for different releases.  Now git is
smart enough to handle most of this, and you end up doing a lot of
hand-fixing for other merge issues, but when it comes to doing the
stable kernel work, none of that means squat.  All I get to see is when
a patch lands in Linus's tree, and if that patch has a "cc: stable@"
marking, then I look at it.

But, when a patch hits Linus through multiple branches, that means I see
it multiple times, usually months apart in timeframe.  This is
especially bad during the -rc1 merge window when all of the old work you
all did for the past 3 months hits Linus, which includes all of the old
bugfixes that were already in the previous kernel release.

I think there were over 25 different patches in -rc1 that hit this
issue.  Maybe more, maybe less, I don't know, I'm giving up at this
point, I wasted over 2 hours trying to figure out a way to do this in an
automated way, or by hand, or something else to deal with all of these
rejections or "fuzz merge" which really was a duplicate.

And the joy of your "cherry-picked from 12345..." messages, with git
commit ids that are no where to be found in Linus's tree at all, is
totally annoying as well, why even have this if it doesn't mean
anything?  I sure can't use it.

Not to mention all of the build-breakages, and normal "fails to apply"
that you all end up with, your driver subsystem has the largest instance
of those as well, and build-breakages are the worst, as they cause my
process to come to a screeching halt while I have to bisect to find the
broken patch.  Given the lack of patches that people actually send me
when I send in the "FAILED" emails, I'm guessing that you all don't care
that much about stable kernels either, which is fine, as again, I'm
giving up on your driver.

So, either you all only mark a patch _ONCE_.  Or come up with some way
for me to determine, in an automated way that a patch is already in
Linus's older release, or just don't mark anything and send me a
hand-curated list of git commit ids, or patches in mbox form (like DaveM
does), or something else you all come up with.  What is happening now is
not working at all, and as of now, I'm going to just drop all i915
patches with a cc: stable marking on the floor.

Congrats on being the first subsystem that I've had to blacklist from my
stable patch workflow :(

greg "35k feet above India and grumpy" k-h

Re: The i915 stable patch marking is totally broken

[Dave Airlie]

On 13 March 2017 at 05:44, Greg KH <gregkh@linuxfoundation.org> wrote:
> Hi Daniel and Jani and other members of the i915-commit-cabal,
>
> I've mentioned this a few times to Daniel in the past (like at the last
> kernel summit), but the way you all are handling the tagging of patches
> for inclusion in stable kernel releases is totally broken and causing me
> no end of headaches.
>
> It's due to your "you have a branch, you have a branch, you all have a
> branch!" model of development.  You have patches that end up flowing in
> to Linus's trees multiple times for different releases.  Now git is
> smart enough to handle most of this, and you end up doing a lot of
> hand-fixing for other merge issues, but when it comes to doing the
> stable kernel work, none of that means squat.  All I get to see is when
> a patch lands in Linus's tree, and if that patch has a "cc: stable@"
> marking, then I look at it.
>
> But, when a patch hits Linus through multiple branches, that means I see
> it multiple times, usually months apart in timeframe.  This is
> especially bad during the -rc1 merge window when all of the old work you
> all did for the past 3 months hits Linus, which includes all of the old
> bugfixes that were already in the previous kernel release.
>
> I think there were over 25 different patches in -rc1 that hit this
> issue.  Maybe more, maybe less, I don't know, I'm giving up at this
> point, I wasted over 2 hours trying to figure out a way to do this in an
> automated way, or by hand, or something else to deal with all of these
> rejections or "fuzz merge" which really was a duplicate.
>
> And the joy of your "cherry-picked from 12345..." messages, with git
> commit ids that are no where to be found in Linus's tree at all, is
> totally annoying as well, why even have this if it doesn't mean
> anything?  I sure can't use it.
>
> Not to mention all of the build-breakages, and normal "fails to apply"
> that you all end up with, your driver subsystem has the largest instance
> of those as well, and build-breakages are the worst, as they cause my
> process to come to a screeching halt while I have to bisect to find the
> broken patch.  Given the lack of patches that people actually send me
> when I send in the "FAILED" emails, I'm guessing that you all don't care
> that much about stable kernels either, which is fine, as again, I'm
> giving up on your driver.
>
> So, either you all only mark a patch _ONCE_.  Or come up with some way
> for me to determine, in an automated way that a patch is already in
> Linus's older release, or just don't mark anything and send me a
> hand-curated list of git commit ids, or patches in mbox form (like DaveM
> does), or something else you all come up with.  What is happening now is
> not working at all, and as of now, I'm going to just drop all i915
> patches with a cc: stable marking on the floor.
>
> Congrats on being the first subsystem that I've had to blacklist from my
> stable patch workflow :(
>
> greg "35k feet above India and grumpy" k-h

What happened to, I won't ask subsystem maintainers to do any extra work :-)

But I'm not sure why the model doesn't break for others, surely some subsystems
realise after committing patches to -next, they really are more urgent
so put them
into a -fixes pull earlier, but can't rebase -next. The cherry-pick tag should
have the info vs the -next tree that Linus is pulling in the next merge window,
it would be a bit difficult to do a cherry-pick to -fixes from a
branch Linus has
already pulled.

I don't think there is anything incorrect in the model, and it
certainly has nothing
to do with "the branch model" or whatever you are alluding to there.
The branches
are pretty simple, a -next and a -fixes with occasional -next-fixes
for merge window,
is it just the sheer number of patches that go to -next and get pulled
into -fixes that
overwhelms you?

Dave.

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

[Daniel Vetter]

On Sun, Mar 12, 2017 at 08:44:40PM +0100, Greg KH wrote:
> Hi Daniel and Jani and other members of the i915-commit-cabal,
> 
> I've mentioned this a few times to Daniel in the past (like at the last
> kernel summit), but the way you all are handling the tagging of patches
> for inclusion in stable kernel releases is totally broken and causing me
> no end of headaches.
> 
> It's due to your "you have a branch, you have a branch, you all have a
> branch!" model of development.  You have patches that end up flowing in
> to Linus's trees multiple times for different releases.  Now git is
> smart enough to handle most of this, and you end up doing a lot of
> hand-fixing for other merge issues, but when it comes to doing the
> stable kernel work, none of that means squat.  All I get to see is when
> a patch lands in Linus's tree, and if that patch has a "cc: stable@"
> marking, then I look at it.
> 
> But, when a patch hits Linus through multiple branches, that means I see
> it multiple times, usually months apart in timeframe.  This is
> especially bad during the -rc1 merge window when all of the old work you
> all did for the past 3 months hits Linus, which includes all of the old
> bugfixes that were already in the previous kernel release.
> 
> I think there were over 25 different patches in -rc1 that hit this
> issue.  Maybe more, maybe less, I don't know, I'm giving up at this
> point, I wasted over 2 hours trying to figure out a way to do this in an
> automated way, or by hand, or something else to deal with all of these
> rejections or "fuzz merge" which really was a duplicate.
> 
> And the joy of your "cherry-picked from 12345..." messages, with git
> commit ids that are no where to be found in Linus's tree at all, is
> totally annoying as well, why even have this if it doesn't mean
> anything?  I sure can't use it.
> 
> Not to mention all of the build-breakages, and normal "fails to apply"
> that you all end up with, your driver subsystem has the largest instance
> of those as well, and build-breakages are the worst, as they cause my
> process to come to a screeching halt while I have to bisect to find the
> broken patch.  Given the lack of patches that people actually send me
> when I send in the "FAILED" emails, I'm guessing that you all don't care
> that much about stable kernels either, which is fine, as again, I'm
> giving up on your driver.
> 
> So, either you all only mark a patch _ONCE_.  Or come up with some way
> for me to determine, in an automated way that a patch is already in
> Linus's older release, or just don't mark anything and send me a
> hand-curated list of git commit ids, or patches in mbox form (like DaveM
> does), or something else you all come up with.  What is happening now is
> not working at all, and as of now, I'm going to just drop all i915
> patches with a cc: stable marking on the floor.
> 
> Congrats on being the first subsystem that I've had to blacklist from my
> stable patch workflow :(
> 
> greg "35k feet above India and grumpy" k-h

So I blame this on flight level 350, but we discussed this at kernel
summit. Every patch we cherry-pick over comes with a "cherry-picked from
$sha1" line, as long as you ignore any such sha1 as duplicate you won't
see the same patch twice.

Iirc you said you'll implement this in your scripts, and as long as we
never break this rule, you'll be fine. Since you seemed to have agreed to
a solution that would solve all your headaches I didn't bother doing
any changes on our side here.

what happened? Has bashing drm become the cool thing to do somehow?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: The i915 stable patch marking is totally broken

[Greg KH]

On Mon, Mar 13, 2017 at 06:11:12AM +1000, Dave Airlie wrote:
> On 13 March 2017 at 05:44, Greg KH <gregkh@linuxfoundation.org> wrote:
> > Hi Daniel and Jani and other members of the i915-commit-cabal,
> >
> > I've mentioned this a few times to Daniel in the past (like at the last
> > kernel summit), but the way you all are handling the tagging of patches
> > for inclusion in stable kernel releases is totally broken and causing me
> > no end of headaches.
> >
> > It's due to your "you have a branch, you have a branch, you all have a
> > branch!" model of development.  You have patches that end up flowing in
> > to Linus's trees multiple times for different releases.  Now git is
> > smart enough to handle most of this, and you end up doing a lot of
> > hand-fixing for other merge issues, but when it comes to doing the
> > stable kernel work, none of that means squat.  All I get to see is when
> > a patch lands in Linus's tree, and if that patch has a "cc: stable@"
> > marking, then I look at it.
> >
> > But, when a patch hits Linus through multiple branches, that means I see
> > it multiple times, usually months apart in timeframe.  This is
> > especially bad during the -rc1 merge window when all of the old work you
> > all did for the past 3 months hits Linus, which includes all of the old
> > bugfixes that were already in the previous kernel release.
> >
> > I think there were over 25 different patches in -rc1 that hit this
> > issue.  Maybe more, maybe less, I don't know, I'm giving up at this
> > point, I wasted over 2 hours trying to figure out a way to do this in an
> > automated way, or by hand, or something else to deal with all of these
> > rejections or "fuzz merge" which really was a duplicate.
> >
> > And the joy of your "cherry-picked from 12345..." messages, with git
> > commit ids that are no where to be found in Linus's tree at all, is
> > totally annoying as well, why even have this if it doesn't mean
> > anything?  I sure can't use it.
> >
> > Not to mention all of the build-breakages, and normal "fails to apply"
> > that you all end up with, your driver subsystem has the largest instance
> > of those as well, and build-breakages are the worst, as they cause my
> > process to come to a screeching halt while I have to bisect to find the
> > broken patch.  Given the lack of patches that people actually send me
> > when I send in the "FAILED" emails, I'm guessing that you all don't care
> > that much about stable kernels either, which is fine, as again, I'm
> > giving up on your driver.
> >
> > So, either you all only mark a patch _ONCE_.  Or come up with some way
> > for me to determine, in an automated way that a patch is already in
> > Linus's older release, or just don't mark anything and send me a
> > hand-curated list of git commit ids, or patches in mbox form (like DaveM
> > does), or something else you all come up with.  What is happening now is
> > not working at all, and as of now, I'm going to just drop all i915
> > patches with a cc: stable marking on the floor.
> >
> > Congrats on being the first subsystem that I've had to blacklist from my
> > stable patch workflow :(
> >
> > greg "35k feet above India and grumpy" k-h
> 
> What happened to, I won't ask subsystem maintainers to do any extra work :-)

Well, when they cause me extra work, or problems, I'm allowed to complain :)

> But I'm not sure why the model doesn't break for others, surely some subsystems
> realise after committing patches to -next, they really are more urgent
> so put them into a -fixes pull earlier, but can't rebase -next. The
> cherry-pick tag should have the info vs the -next tree that Linus is
> pulling in the next merge window, it would be a bit difficult to do a
> cherry-pick to -fixes from a branch Linus has already pulled.

Nope, no other subsystem does it like the i915 driver does.  Only rarely
does a patch for stable come in multiple times.  The i915 driver does it
way too often.

Why don't the maintainers know which tree to put them in when they are
submitted?  As an example, if I get a patch that needs to go to Linus, I
put it in my usb-linus branch, and when it hits a -rc release, I then
merge that -rc back into my usb-next branch.  So I end up with about 2-3
merges to -rc every release, which isn't bad and doesn't cause any
duplication issues.

Seems that most other subsystems also do this as well.

> I don't think there is anything incorrect in the model, and it
> certainly has nothing to do with "the branch model" or whatever you
> are alluding to there.  The branches are pretty simple, a -next and a
> -fixes with occasional -next-fixes for merge window, is it just the
> sheer number of patches that go to -next and get pulled into -fixes
> that overwhelms you?

25-30 patches marked for stable ended up in 4.11-rc1 that were already
in 4.10.0.  And I think this was way less than what happened in
4.12-rc1.  It's hard to determine that a patch really is a duplicate, or
if it just doesn't apply for some other reason.  So much so that I dread
the drm stable patches, which should not happen.

thanks,

greg k-h

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

[Greg KH]

On Sun, Mar 12, 2017 at 09:46:21PM +0100, Daniel Vetter wrote:
> On Sun, Mar 12, 2017 at 08:44:40PM +0100, Greg KH wrote:
> > Hi Daniel and Jani and other members of the i915-commit-cabal,
> > 
> > I've mentioned this a few times to Daniel in the past (like at the last
> > kernel summit), but the way you all are handling the tagging of patches
> > for inclusion in stable kernel releases is totally broken and causing me
> > no end of headaches.
> > 
> > It's due to your "you have a branch, you have a branch, you all have a
> > branch!" model of development.  You have patches that end up flowing in
> > to Linus's trees multiple times for different releases.  Now git is
> > smart enough to handle most of this, and you end up doing a lot of
> > hand-fixing for other merge issues, but when it comes to doing the
> > stable kernel work, none of that means squat.  All I get to see is when
> > a patch lands in Linus's tree, and if that patch has a "cc: stable@"
> > marking, then I look at it.
> > 
> > But, when a patch hits Linus through multiple branches, that means I see
> > it multiple times, usually months apart in timeframe.  This is
> > especially bad during the -rc1 merge window when all of the old work you
> > all did for the past 3 months hits Linus, which includes all of the old
> > bugfixes that were already in the previous kernel release.
> > 
> > I think there were over 25 different patches in -rc1 that hit this
> > issue.  Maybe more, maybe less, I don't know, I'm giving up at this
> > point, I wasted over 2 hours trying to figure out a way to do this in an
> > automated way, or by hand, or something else to deal with all of these
> > rejections or "fuzz merge" which really was a duplicate.
> > 
> > And the joy of your "cherry-picked from 12345..." messages, with git
> > commit ids that are no where to be found in Linus's tree at all, is
> > totally annoying as well, why even have this if it doesn't mean
> > anything?  I sure can't use it.
> > 
> > Not to mention all of the build-breakages, and normal "fails to apply"
> > that you all end up with, your driver subsystem has the largest instance
> > of those as well, and build-breakages are the worst, as they cause my
> > process to come to a screeching halt while I have to bisect to find the
> > broken patch.  Given the lack of patches that people actually send me
> > when I send in the "FAILED" emails, I'm guessing that you all don't care
> > that much about stable kernels either, which is fine, as again, I'm
> > giving up on your driver.
> > 
> > So, either you all only mark a patch _ONCE_.  Or come up with some way
> > for me to determine, in an automated way that a patch is already in
> > Linus's older release, or just don't mark anything and send me a
> > hand-curated list of git commit ids, or patches in mbox form (like DaveM
> > does), or something else you all come up with.  What is happening now is
> > not working at all, and as of now, I'm going to just drop all i915
> > patches with a cc: stable marking on the floor.
> > 
> > Congrats on being the first subsystem that I've had to blacklist from my
> > stable patch workflow :(
> > 
> > greg "35k feet above India and grumpy" k-h
> 
> So I blame this on flight level 350, but we discussed this at kernel
> summit. Every patch we cherry-pick over comes with a "cherry-picked from
> $sha1" line, as long as you ignore any such sha1 as duplicate you won't
> see the same patch twice.

I tried that, but that cherry-pick number doesn't seem to match up with
anything in Linus's tree.  Where are those numbers coming from?

Or there aren't numbers at all.  Look at commit:
8726f2faa371514fba2f594d799db95203dfeee0.  It just showed up in Linus's
tree, and there's no "cherry-pick" number in there.  It ended up in
4.9.7.

Hm, ok, you want me to look at the commit id and then search to see if
it's already been merged "before".  Ah, that's crazy.  So I need to do
that for every i915 patch?  Search backwards?  Ugh, that's a mess, no
wonder I couldn't figure it out...

> Iirc you said you'll implement this in your scripts, and as long as we
> never break this rule, you'll be fine. Since you seemed to have agreed to
> a solution that would solve all your headaches I didn't bother doing
> any changes on our side here.

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...

> what happened? Has bashing drm become the cool thing to do somehow?

I don't understand, I haven't bashed drm in years :)

thanks,

greg "jet lagged" k-h

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

[Daniel Vetter]

On Sun, Mar 12, 2017 at 11:01 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> So I blame this on flight level 350, but we discussed this at kernel
>> summit. Every patch we cherry-pick over comes with a "cherry-picked from
>> $sha1" line, as long as you ignore any such sha1 as duplicate you won't
>> see the same patch twice.
>
> I tried that, but that cherry-pick number doesn't seem to match up with
> anything in Linus's tree.  Where are those numbers coming from?
>
> Or there aren't numbers at all.  Look at commit:
> 8726f2faa371514fba2f594d799db95203dfeee0.  It just showed up in Linus's
> tree, and there's no "cherry-pick" number in there.  It ended up in
> 4.9.7.
>
> Hm, ok, you want me to look at the commit id and then search to see if
> it's already been merged "before".  Ah, that's crazy.  So I need to do
> that for every i915 patch?  Search backwards?  Ugh, that's a mess, no
> wonder I couldn't figure it out...

Our cherry-pick sha1 work exactly like yours: They don't make sense
when you only look at the tree a patch has been cherry-picked _to_,
since they're the sha1 from the tree they've been cherry-picked
_from_. When you clone a fresh copy of your stable tree then the
cherry-pick numbers also point nowhere. Only once you've pulled the
future tree they're from (Linus' git in your case) do they make sense.

Same for our cherry-picks, except the future tree isn't Linus' git
(we'd have managed to make sha1 collisions cheaply otherwise ...) but
the future Linus' git tree. Which is maintained by Stephen Rothwell in
linux-next. As soon as you make sure you have the latest
linux-next.git they will all resolve to something meaningful.

Not crazy going on at all :-)

>> Iirc you said you'll implement this in your scripts, and as long as we
>> never break this rule, you'll be fine. Since you seemed to have agreed to
>> a solution that would solve all your headaches I didn't bother doing
>> any changes on our side here.
>
> 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).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

[Daniel Vetter]

On Sun, Mar 12, 2017 at 10:52 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> Why don't the maintainers know which tree to put them in when they are
> submitted?  As an example, if I get a patch that needs to go to Linus, I
> put it in my usb-linus branch, and when it hits a -rc release, I then
> merge that -rc back into my usb-next branch.  So I end up with about 2-3
> merges to -rc every release, which isn't bad and doesn't cause any
> duplication issues.
>
> Seems that most other subsystems also do this as well.

We do know (mostly) where a patch should go to, and we do push a
backmerge every 1-2 weeks or so, too.

The reason why we've started to require that every bugfix for drm/i915
land in -next first is fairly similar to why you insist every bugfix
must be in Linus' tree: Without that patches get lost. Well, they
don't get lost intentionally (they're all still in the git log for us
due to backmerges), but we did lose some in the horrible resulting
conflicts. Insisting that we have them in our -next branch means the
backmerges can be resolved with git merge -x ours.

And in the end this is how it's done byalmost everyone: You push to
master and cherry-pick over to stable/release branches. Most projects
don't apply bugfixes to the stable branch and then backmerge to their
master branch, because it would result in pure chaos. You don't do
that either for stable kernel. It's just that for most subsystems the
resulting conflict gallore of using backmerges is fairly manageable
(it's getting into the no-fun territory with drm core too, but still
ok), whereas drm/i915 is just too much, moving too fast, to make that
a working model.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

[Jani Nikula]

On Mon, 13 Mar 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> Our cherry-pick sha1 work exactly like yours: They don't make sense
> when you only look at the tree a patch has been cherry-picked _to_,
> since they're the sha1 from the tree they've been cherry-picked
> _from_. When you clone a fresh copy of your stable tree then the
> cherry-pick numbers also point nowhere. Only once you've pulled the
> future tree they're from (Linus' git in your case) do they make sense.
>
> Same for our cherry-picks, except the future tree isn't Linus' git
> (we'd have managed to make sha1 collisions cheaply otherwise ...) but
> the future Linus' git tree. Which is maintained by Stephen Rothwell in
> linux-next. As soon as you make sure you have the latest
> linux-next.git they will all resolve to something meaningful.

Indeed, if there's a cherry-pick reference to a commit that's *not* in
linux-next, we deserve to be yelled at. The branches that feed to
linux-next that we cherry-pick from are non-rebasing, so the commit ids
should not change when they eventually hit Linus' tree.

>> 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...

Most of our cherry-picking is scripted, so if there's further annotation
that you'd like, just let us know. (Too bad it's virtually impossible to
modify the commit being cherry-picked. Unless someone(tm) comes up with
a way to share git-notes in a sensible, distributed way.)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

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

[Daniel Vetter]

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

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

[Greg KH]

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

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

[Jani Nikula]

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

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

[Greg KH]

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

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

[Greg KH]

On Mon, Mar 13, 2017 at 07:49:59AM +0100, Daniel Vetter wrote:
> On Sun, Mar 12, 2017 at 10:52 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > Why don't the maintainers know which tree to put them in when they are
> > submitted?  As an example, if I get a patch that needs to go to Linus, I
> > put it in my usb-linus branch, and when it hits a -rc release, I then
> > merge that -rc back into my usb-next branch.  So I end up with about 2-3
> > merges to -rc every release, which isn't bad and doesn't cause any
> > duplication issues.
> >
> > Seems that most other subsystems also do this as well.
> 
> We do know (mostly) where a patch should go to, and we do push a
> backmerge every 1-2 weeks or so, too.
> 
> The reason why we've started to require that every bugfix for drm/i915
> land in -next first is fairly similar to why you insist every bugfix
> must be in Linus' tree: Without that patches get lost. Well, they
> don't get lost intentionally (they're all still in the git log for us
> due to backmerges), but we did lose some in the horrible resulting
> conflicts. Insisting that we have them in our -next branch means the
> backmerges can be resolved with git merge -x ours.
> 
> And in the end this is how it's done byalmost everyone: You push to
> master and cherry-pick over to stable/release branches. Most projects
> don't apply bugfixes to the stable branch and then backmerge to their
> master branch, because it would result in pure chaos. You don't do
> that either for stable kernel. It's just that for most subsystems the
> resulting conflict gallore of using backmerges is fairly manageable
> (it's getting into the no-fun territory with drm core too, but still
> ok), whereas drm/i915 is just too much, moving too fast, to make that
> a working model.

Ok, I agree that your code is moving too fast for the "normal" stable
model here.  I just tried to apply a potential 17 patches and only 8
applied.  That's not a good percentage.

So, including all of the mess where you are sending patches to Linus in
multiple branches tagged for stable, confusing the heck out of things,
combined with the extreemly low percentage of patches that actually
apply, can I just ask for you all to send me a list of commit ids, or
patches in the format that DaveM does for networking, for i915 stable
patches?  As it is, it's not working for me, nor you, so something has
to change.

And yes, I have said that I don't want to impose extra work on
maintainers for stable stuff, but right now, you are wasting stable
developers time with this low percentage rate, and ending up with a
miss-match of patches that I bet you all don't even know if it works or
not, making things even worse overall for our users.

thanks,

greg k-h

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

[Greg KH]

On Wed, Apr 12, 2017 at 02:48:55PM +0200, Greg KH wrote:
> On Mon, Mar 13, 2017 at 07:49:59AM +0100, Daniel Vetter wrote:
> > On Sun, Mar 12, 2017 at 10:52 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > Why don't the maintainers know which tree to put them in when they are
> > > submitted?  As an example, if I get a patch that needs to go to Linus, I
> > > put it in my usb-linus branch, and when it hits a -rc release, I then
> > > merge that -rc back into my usb-next branch.  So I end up with about 2-3
> > > merges to -rc every release, which isn't bad and doesn't cause any
> > > duplication issues.
> > >
> > > Seems that most other subsystems also do this as well.
> > 
> > We do know (mostly) where a patch should go to, and we do push a
> > backmerge every 1-2 weeks or so, too.
> > 
> > The reason why we've started to require that every bugfix for drm/i915
> > land in -next first is fairly similar to why you insist every bugfix
> > must be in Linus' tree: Without that patches get lost. Well, they
> > don't get lost intentionally (they're all still in the git log for us
> > due to backmerges), but we did lose some in the horrible resulting
> > conflicts. Insisting that we have them in our -next branch means the
> > backmerges can be resolved with git merge -x ours.
> > 
> > And in the end this is how it's done byalmost everyone: You push to
> > master and cherry-pick over to stable/release branches. Most projects
> > don't apply bugfixes to the stable branch and then backmerge to their
> > master branch, because it would result in pure chaos. You don't do
> > that either for stable kernel. It's just that for most subsystems the
> > resulting conflict gallore of using backmerges is fairly manageable
> > (it's getting into the no-fun territory with drm core too, but still
> > ok), whereas drm/i915 is just too much, moving too fast, to make that
> > a working model.
> 
> Ok, I agree that your code is moving too fast for the "normal" stable
> model here.  I just tried to apply a potential 17 patches and only 8
> applied.  That's not a good percentage.

Ok, the last remaining ones (all 6) in my queue, did apply cleanly, so
your percentage went up a bit more, but it's still the worst of any part
of the kernel and I don't think this is working as-is.

greg k-h