All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
To: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@ozlabs.org, Thomas Gleixner <tglx@linutronix.de>,
	Realtime Kernel <linux-rt-users@vger.kernel.org>
Subject: Re: [PATCH v2 3/4] Implement clockevents driver for powerpc
Date: Thu, 18 Oct 2007 19:11:57 +0400	[thread overview]
Message-ID: <471777BD.8090800@ru.mvista.com> (raw)
In-Reply-To: <18198.44590.721412.314409@cargo.ozlabs.ibm.com>

Hello.

Paul Mackerras wrote:

>>>Tony started from an earlier patch by John Stultz, not from your
>>>patches.

>>    Well, that I can believe, yet the clockevents patch has traces of my
>>former work, and looking at read_persisitent_time() it looks suspiciously 
>>close to my version too...

> There is basically only one reasonably way to do a lot of this stuff.

>>>The main reason your patches were rejected were that you completely
>>>broke the VDSO and the deterministic time accounting, and made no

>>    That's just not true!
>>    They didn't broke vDSO (to be precise it was John's patch that broke it), 
>>they just removed the vDSO code known to already be broken by -rt patch for 
>>several months by then.  And they didn't broke determinictic accounting -- 
>>they just made two things mutually exclusive.  I haven't yet seen how the 
>>patches that were preferred dealt with it at all.

> OK.  My requirement was that the clocksource/clockevent stuff and the
> VDSO were both functional.  Your patch didn't meet that requirement.

    Which of my patches? There were many, and only one of them dealing with 
vDSO. That's reasonable to drop that patch but it's not reasonable to drop the 
other ones, not directly connected to vDSO issue.  One flaw doesn't make the 
whole patchset bad.
    And now you have incomplete read_persistent_clock() implementation for 
example, god knows why it was preferred to mine -- well, it also implemented 
update_persistent_clock() bit those functions haven't appeared at the same 
time, so read_persistent_clock() was written by me in the .

>>    Really? IMO, the harware does keep a constant interrupt rate better than 
>>software.

> Well, if you have actual numbers to back that up, show them to us.
> I don't believe you would be able to measure any difference, and so I
> prefer the simplicity of only implementing the one-shot mode.

    Well, that's up to you.  I take it you wouldn't accept a patch 
implementing auto-reload mode?

>>>Because you broke important features

>>    That is *not true*.
>>    And nobody had interest to fix them for months (quite strange if they're 
>>so important) while I had neither time nor interest to deal with them anymore 
>>having written the code that *did work*, and not only for me.

> Well, this is the difference between having a hack that works for you,

    Agreed, -rt is a patchset full of hacks. :-)

> and having something that can go upstream into mainline.

    It *went* upstream. Mainline wasn't my aim at that time.

> Anyway, this discussion doesn't seem to be going anywhere.  If there
> are changes you want made, or any other specific concrete action you

    There are. I'll have to send patches (it's not that I have time for this) 
but this is surely the fastest way to get things fixed (if I don't get ignored 
that is).

> want anyone to do, say so.  Otherwise stop whinging.

    I just wanted the reasons clarified and got what I wanted -- as I thought, 
the decision behind preferring patches was somewhat biased, nobody really 
cared about code quality or just wasn't familiar with hrtimers enough to judge 
on the code quality...

> Paul.

WBR, Sergei

WARNING: multiple messages have this Message-ID (diff)
From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
To: Paul Mackerras <paulus@samba.org>
Cc: Tony Breeds <tony@bakeyournoodle.com>,
	linuxppc-dev@ozlabs.org, Thomas Gleixner <tglx@linutronix.de>,
	Realtime Kernel <linux-rt-users@vger.kernel.org>
Subject: Re: [PATCH v2 3/4] Implement clockevents driver for powerpc
Date: Thu, 18 Oct 2007 19:11:57 +0400	[thread overview]
Message-ID: <471777BD.8090800@ru.mvista.com> (raw)
In-Reply-To: <18198.44590.721412.314409@cargo.ozlabs.ibm.com>

Hello.

Paul Mackerras wrote:

>>>Tony started from an earlier patch by John Stultz, not from your
>>>patches.

>>    Well, that I can believe, yet the clockevents patch has traces of my
>>former work, and looking at read_persisitent_time() it looks suspiciously 
>>close to my version too...

> There is basically only one reasonably way to do a lot of this stuff.

>>>The main reason your patches were rejected were that you completely
>>>broke the VDSO and the deterministic time accounting, and made no

>>    That's just not true!
>>    They didn't broke vDSO (to be precise it was John's patch that broke it), 
>>they just removed the vDSO code known to already be broken by -rt patch for 
>>several months by then.  And they didn't broke determinictic accounting -- 
>>they just made two things mutually exclusive.  I haven't yet seen how the 
>>patches that were preferred dealt with it at all.

> OK.  My requirement was that the clocksource/clockevent stuff and the
> VDSO were both functional.  Your patch didn't meet that requirement.

    Which of my patches? There were many, and only one of them dealing with 
vDSO. That's reasonable to drop that patch but it's not reasonable to drop the 
other ones, not directly connected to vDSO issue.  One flaw doesn't make the 
whole patchset bad.
    And now you have incomplete read_persistent_clock() implementation for 
example, god knows why it was preferred to mine -- well, it also implemented 
update_persistent_clock() bit those functions haven't appeared at the same 
time, so read_persistent_clock() was written by me in the .

>>    Really? IMO, the harware does keep a constant interrupt rate better than 
>>software.

> Well, if you have actual numbers to back that up, show them to us.
> I don't believe you would be able to measure any difference, and so I
> prefer the simplicity of only implementing the one-shot mode.

    Well, that's up to you.  I take it you wouldn't accept a patch 
implementing auto-reload mode?

>>>Because you broke important features

>>    That is *not true*.
>>    And nobody had interest to fix them for months (quite strange if they're 
>>so important) while I had neither time nor interest to deal with them anymore 
>>having written the code that *did work*, and not only for me.

> Well, this is the difference between having a hack that works for you,

    Agreed, -rt is a patchset full of hacks. :-)

> and having something that can go upstream into mainline.

    It *went* upstream. Mainline wasn't my aim at that time.

> Anyway, this discussion doesn't seem to be going anywhere.  If there
> are changes you want made, or any other specific concrete action you

    There are. I'll have to send patches (it's not that I have time for this) 
but this is surely the fastest way to get things fixed (if I don't get ignored 
that is).

> want anyone to do, say so.  Otherwise stop whinging.

    I just wanted the reasons clarified and got what I wanted -- as I thought, 
the decision behind preferring patches was somewhat biased, nobody really 
cared about code quality or just wasn't familiar with hrtimers enough to judge 
on the code quality...

> Paul.

WBR, Sergei

  reply	other threads:[~2007-10-18 15:11 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-21  3:26 [PATCH v2 1/4] Implement {read,update}_persistent_clock Tony Breeds
2007-09-21  3:26 ` [PATCH v2 2/4] Implement generic time of day clocksource for powerpc machines Tony Breeds
2007-09-21  4:05   ` Daniel Walker
2007-09-21  4:05     ` Daniel Walker
2007-09-21  4:59     ` Paul Mackerras
2007-09-21  4:59       ` Paul Mackerras
2007-09-21  6:43       ` David Gibson
2007-09-21  6:43         ` David Gibson
2007-09-21  4:52   ` Stephen Rothwell
2007-09-21  4:52     ` Stephen Rothwell
2007-09-21 21:35     ` Tony Breeds
2007-09-21 21:35       ` Tony Breeds
2007-09-21 21:35     ` [PATCH v3 " Tony Breeds
2007-09-21 21:35       ` Tony Breeds
2007-10-03  0:48       ` Paul Mackerras
2007-10-03  0:48         ` Paul Mackerras
2007-10-03  4:00         ` Thomas Gleixner
2007-10-03  4:00           ` Thomas Gleixner
2007-09-21  3:26 ` [PATCH v2 4/4] Enable tickless idle and high res timers for powerpc Tony Breeds
2007-09-21  3:26 ` [PATCH v2 3/4] Implement clockevents driver " Tony Breeds
2007-10-15 17:40   ` Sergei Shtylyov
2007-10-15 17:40     ` Sergei Shtylyov
2007-10-15 18:33     ` Sergei Shtylyov
2007-10-15 18:33       ` Sergei Shtylyov
2007-10-15 23:44     ` Paul Mackerras
2007-10-15 23:44       ` Paul Mackerras
2007-10-17 14:29       ` Sergei Shtylyov
2007-10-17 14:29         ` Sergei Shtylyov
2007-10-18  0:51         ` Paul Mackerras
2007-10-18  0:51           ` Paul Mackerras
2007-10-18 15:11           ` Sergei Shtylyov [this message]
2007-10-18 15:11             ` Sergei Shtylyov
2007-10-19  1:53             ` Paul Mackerras
2007-10-19  1:53               ` Paul Mackerras
2007-10-19 12:11               ` Sergei Shtylyov
2007-10-19 12:11                 ` Sergei Shtylyov
2007-10-19 12:36                 ` Paul Mackerras
2007-10-19 12:36                   ` Paul Mackerras
2007-10-19 13:35                   ` Sergei Shtylyov
2007-10-19 13:35                     ` Sergei Shtylyov
2007-10-24 12:07                     ` Sergei Shtylyov
2007-10-24 12:07                       ` Sergei Shtylyov
2007-10-24 23:55                       ` Paul Mackerras
2007-10-17 14:34       ` Sergei Shtylyov
2007-10-17 14:34         ` Sergei Shtylyov
2007-10-18  0:36         ` Paul Mackerras
2007-10-18  0:36           ` Paul Mackerras
2007-10-18 14:48           ` Sergei Shtylyov
2007-10-18 14:48             ` Sergei Shtylyov
2007-10-19  0:14             ` Paul Mackerras
2007-10-19  0:14               ` Paul Mackerras
2007-10-19  9:22               ` Gabriel Paubert
2007-10-19  9:22                 ` Gabriel Paubert
2007-10-19 11:22                 ` Paul Mackerras
2007-10-19 11:49               ` Sergei Shtylyov
2007-10-19 11:49                 ` Sergei Shtylyov
2007-10-19 12:24                 ` Paul Mackerras
2007-10-19 12:24                   ` Paul Mackerras
2007-09-26 19:04 ` [PATCH v2 1/4] Implement {read,update}_persistent_clock Steven Rostedt
2007-09-26 19:04   ` Steven Rostedt
2007-09-26 19:39   ` Sergei Shtylyov
2007-09-26 19:39     ` Sergei Shtylyov
2007-09-26 19:44     ` Steven Rostedt
2007-09-26 19:44       ` Steven Rostedt
2007-09-26 19:58       ` Thomas Gleixner
2007-09-26 19:58         ` Thomas Gleixner
2007-10-15 18:05         ` Sergei Shtylyov
2007-10-15 18:05           ` Sergei Shtylyov
2007-10-15 23:46           ` Paul Mackerras
2007-10-15 23:46             ` Paul Mackerras
2007-10-16  1:19           ` Benjamin Herrenschmidt
2007-10-16  1:19             ` Benjamin Herrenschmidt
2007-10-17 12:45             ` Sergei Shtylyov
2007-10-17 12:45               ` Sergei Shtylyov
2007-09-27  1:59     ` Benjamin Herrenschmidt
2007-09-27  1:59       ` Benjamin Herrenschmidt
2007-10-15 18:07       ` Sergei Shtylyov
2007-10-15 18:07         ` Sergei Shtylyov
2007-10-15 23:02         ` Benjamin Herrenschmidt
2007-10-17 15:34 ` Sergei Shtylyov
2007-10-17 15:34   ` Sergei Shtylyov
2007-10-18 14:18   ` Sergei Shtylyov
2007-10-18 14:18     ` Sergei Shtylyov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=471777BD.8090800@ru.mvista.com \
    --to=sshtylyov@ru.mvista.com \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=paulus@samba.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.