[RFC patch 00/18] Trace Clock v2 - Kernel

This is a discussion on [RFC patch 00/18] Trace Clock v2 - Kernel ; On Fri, Nov 07, 2008 at 11:47:58AM -0500, Mathieu Desnoyers wrote: > But any get_cycles() user of cnt32_to_63() should be shot down. The > bright side is : there is no way get_cycles() can be used with this > new ...

+ Reply to Thread
Page 4 of 6 FirstFirst ... 2 3 4 5 6 LastLast
Results 61 to 80 of 112

Thread: [RFC patch 00/18] Trace Clock v2

  1. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    On Fri, Nov 07, 2008 at 11:47:58AM -0500, Mathieu Desnoyers wrote:
    > But any get_cycles() user of cnt32_to_63() should be shot down. The
    > bright side is : there is no way get_cycles() can be used with this
    > new code.
    >
    > e.g. of incorrect users for arm (unless they are UP only, but that seems
    > like a weird design argument) :
    >
    > mach-sa1100/include/mach/SA-1100.h:#define OSCR __REG(0x90000010)
    > /* OS timer Counter Reg. */
    > mach-sa1100/generic.c: unsigned long long v = cnt32_to_63(OSCR);
    > mach-pxa/include/mach/pxa-regs.h:#define OSCR __REG(0x40A00010) /* OS
    > Timer Counter Register */
    > mach-pxa/time.c: unsigned long long v = cnt32_to_63(OSCR);


    It's strange for you to make that assertion when PXA was the exact
    platform that Nicolas created this code for - and that's a platform
    where preempt has been widely used.

    The two you mention are both ARMv5 or older architectures, and the
    first real SMP ARM architecture is ARMv6. So architecturally they
    are UP only.

    So, tell me why you say "unless they are UP only, but that seems like
    a weird design argument"? If the platforms can only ever be UP only,
    what's wrong with UP only code being used with them? (Not that I'm
    saying anything there about cnt32_to_63.)

    I'd like to see you modify the silicon of a PXA or SA11x0 SoC to add
    more than one processor to the chip - maybe you could use evostick to
    glue two dies together and a microscope to aid bonding wires between
    the two? (Of course, you'd need to design something to ensure cache
    coherence as well, and arbitrate the internal bus between the two
    dies.)

    Personally, I think that's highly unlikely.

    --
    Russell King
    Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
    maintainer of:
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    On Fri, 7 Nov 2008, Mathieu Desnoyers wrote:

    > * David Howells (dhowells@redhat.com) wrote:
    > > Nicolas Pitre wrote:
    > >
    > > > > I mean, the darned thing is called from sched_clock(), which can be
    > > > > concurrently called on separate CPUs and which can be called from
    > > > > interrupt context (with an arbitrary nesting level!) while it was running
    > > > > in process context.
    > > >
    > > > Yes! And this is so on *purpose*. Please take some time to read the
    > > > comment that goes along with it, and if you're still not convinced then
    > > > look for those explanation emails I've already posted.

    > >
    > > I agree with Nicolas on this. It's abominably clever, but I think he's right.
    > >
    > > The one place I remain unconvinced is over the issue of preemption of a process
    > > that is in the middle of cnt32_to_63(), where if the preempted process is
    > > asleep for long enough, I think it can wind time backwards when it resumes, but
    > > that's not a problem for the one place I want to use it (sched_clock()) because
    > > that is (almost) always called with preemption disabled in one way or another.
    > >
    > > The one place it isn't is a debugging case that I'm not too worried about.
    > >

    >
    > I am also concerned about the non-preemption off case.
    >
    > Then I think the function should document that it must be called with
    > preempt disabled.


    I explained several times already why I disagree. Preemption is not a
    problem unless you're preempted away for long enough, or IOW if your
    counter is too fast.

    And no, ^Z on a process doesn't create preemption. This is a signal that
    gets acted upon far away from the middle of cnt32_to_63().


    Nicolas
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    On Fri, 7 Nov 2008, Mathieu Desnoyers wrote:

    > I want to make sure
    >
    > __m_cnt_hi
    > is read before
    > mmio cnt_lo read
    >
    > for the detailed reasons explained in my previous discussion with
    > Nicolas here :
    > http://lkml.org/lkml/2008/10/21/1
    >
    > I use smp_rmb() to do this on SMP systems (hrm, actually, a rmb() could
    > be required so it works also on UP systems safely wrt interrupts).
    >
    > The write side is between the hardware counter, which is assumed to
    > increment monotonically between each read, and the value __m_cnt_hi
    > updated by the CPU. I don't see where we could put a wmb() there.
    >
    > Without barrier, the smp race looks as follow :
    >
    >
    > CPU A B
    > read hw cnt low (0xFFFFFFFA)
    > read __m_cnt_hi (0x80000000)
    > read hw cnt low (0x00000001)
    > (wrap detected :
    > (s32)(0x80000000 ^ 0x1) < 0)
    > write __m_cnt_hi = 0x00000001
    > read __m_cnt_hi (0x00000001)
    > return 0x0000000100000001
    > (wrap detected :
    > (s32)(0x00000001 ^ 0xFFFFFFFA) < 0)
    > write __m_cnt_hi = 0x80000001
    > return 0x80000001FFFFFFFA
    > (time jumps)


    Could you have hardware doing such things? You would get a non cached
    and more expensive read on CPU B which is not in program order with the
    read that should have happened before, and before that second out of
    order read could be performed, you'd have a full sequence in program
    order performed on CPU A.


    Nicolas
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    * Mathieu Desnoyers (mathieu.desnoyers@polymtl.ca) wrote:
    > * Peter Zijlstra (a.p.zijlstra@chello.nl) wrote:
    > > On Fri, 2008-11-07 at 14:18 -0500, Mathieu Desnoyers wrote:
    > > > * Steven Rostedt (rostedt@goodmis.org) wrote:
    > > > >
    > > > > On Fri, 7 Nov 2008, Mathieu Desnoyers wrote:
    > > > > >
    > > > > > __m_cnt_hi
    > > > > > is read before
    > > > > > mmio cnt_lo read
    > > > > >
    > > > > > for the detailed reasons explained in my previous discussion with
    > > > > > Nicolas here :
    > > > > > http://lkml.org/lkml/2008/10/21/1
    > > > > >
    > > > > > I use smp_rmb() to do this on SMP systems (hrm, actually, a rmb() could
    > > > > > be required so it works also on UP systems safely wrt interrupts).
    > > > >
    > > > > smp_rmb turns into a compiler barrier on UP and should prevent the below
    > > > > description.
    > > > >
    > > >
    > > > Ah, right, preserving program order on UP should be enough. smp_rmb()
    > > > then.

    > >
    > >
    > > I'm not quite sure I'm following here. Is this a global hardware clock
    > > you're reading from multiple cpus, if so, are you sure smp_rmb() will
    > > indeed be enough to sync the read?
    > >
    > > (In which case the smp_wmb() is provided by the hardware increasing the
    > > clock?)
    > >
    > > If these are per-cpu clocks then even in the smp case we'd be good with
    > > a plain barrier() because you'd only ever want to read your own cpu's
    > > clock (and have a separate __m_cnt_hi per cpu).
    > >
    > > Or am I totally missing out on something?
    > >

    >
    > This is the global hardware clock scenario.
    >
    > We have to order an uncached mmio read wrt a cached variable read/write.
    > The uncached mmio read vs smp_rmb() barrier (e.g. lfence instruction)
    > should be insured by program order because the read will skip the cache
    > and go directly to the bus. Luckily we only do a mmio read and no mmio
    > write, so mmiowb() is not required.
    >
    > You might be right in that it could require more barriers.
    >
    > Given adequate program order, we can assume the the mmio read will
    > happen "on the spot", but that the cached read may be delayed.
    >
    > What we want is :
    >
    > readl(io_addr)
    > read __m_cnt_hi
    > write __m_cnt_hi
    >
    > With the two reads in the correct order. If we consider two consecutive
    > executions on the same CPU :
    >
    > readl(io_addr)
    > read __m_cnt_hi
    > write __m_cnt_hi
    >
    > readl(io_addr)
    > read __m_cnt_hi
    > write __m_cnt_hi
    >
    > We might have to order the read/write pair wrt the following readl, such
    > as :
    >
    > smp_rmb(); /* Waits for every cached memory reads to complete */
    > readl(io_addr);
    > barrier(); /* Make sure the compiler leaves mmio read before cached read */
    > read __m_cnt_hi
    > write __m_cnt_hi
    >
    > smp_rmb(); /* Waits for every cached memory reads to complete */
    > readl(io_addr)
    > barrier(); /* Make sure the compiler leaves mmio read before cached read */
    > read __m_cnt_hi
    > write __m_cnt_hi
    >
    > Would that make more sense ?
    >


    Oh, actually, I got things reversed in this email : the readl(io_addr)
    must be done _after_ the __m_cnt_hi read.

    Therefore, two consecutive executions would look like :

    barrier(); /* Make sure the compiler does not reorder __m_cnt_hi and
    previous mmio read. */
    read __m_cnt_hi
    smp_rmb(); /* Waits for every cached memory reads to complete */
    readl(io_addr);
    write __m_cnt_hi


    barrier(); /* Make sure the compiler does not reorder __m_cnt_hi and
    previous mmio read. */
    read __m_cnt_hi
    smp_rmb(); /* Waits for every cached memory reads to complete */
    readl(io_addr);
    write __m_cnt_hi

    Mathieu

    > Mathieu
    >
    > --
    > Mathieu Desnoyers
    > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68


    --
    Mathieu Desnoyers
    OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    * Nicolas Pitre (nico@cam.org) wrote:
    > On Fri, 7 Nov 2008, Mathieu Desnoyers wrote:
    >
    > > I want to make sure
    > >
    > > __m_cnt_hi
    > > is read before
    > > mmio cnt_lo read
    > >
    > > for the detailed reasons explained in my previous discussion with
    > > Nicolas here :
    > > http://lkml.org/lkml/2008/10/21/1
    > >
    > > I use smp_rmb() to do this on SMP systems (hrm, actually, a rmb() could
    > > be required so it works also on UP systems safely wrt interrupts).
    > >
    > > The write side is between the hardware counter, which is assumed to
    > > increment monotonically between each read, and the value __m_cnt_hi
    > > updated by the CPU. I don't see where we could put a wmb() there.
    > >
    > > Without barrier, the smp race looks as follow :
    > >
    > >
    > > CPU A B
    > > read hw cnt low (0xFFFFFFFA)
    > > read __m_cnt_hi (0x80000000)
    > > read hw cnt low (0x00000001)
    > > (wrap detected :
    > > (s32)(0x80000000 ^ 0x1) < 0)
    > > write __m_cnt_hi = 0x00000001
    > > read __m_cnt_hi (0x00000001)
    > > return 0x0000000100000001
    > > (wrap detected :
    > > (s32)(0x00000001 ^ 0xFFFFFFFA) < 0)
    > > write __m_cnt_hi = 0x80000001
    > > return 0x80000001FFFFFFFA
    > > (time jumps)

    >
    > Could you have hardware doing such things? You would get a non cached
    > and more expensive read on CPU B which is not in program order with the
    > read that should have happened before, and before that second out of
    > order read could be performed, you'd have a full sequence in program
    > order performed on CPU A.
    >


    Hrm, yes ? Well, it's the whole point in barriers/cache coherency
    mechanisms, out-of-order reads... etc.

    First off, read hw cnt low _is_ an uncached memory read (this is the
    mmio read). __m_cnt_hi is a cached read, and therefore can be delayed if
    the cache-line is busy. And we have no control on how much time can pass
    between the two reads given the CPU may stall waiting for a cache-line.

    So the scenario above happens if CPU A have __m_cnt_hi in its cacheline,
    but for come reason CPU B have to defer the cacheline read of __m_cnt_hi
    due to heavy cacheline traffic and decides to proceed to mmio read
    before the cacheline has been brought to the CPU because "hey, there is
    no data dependency between those two reads !".

    See Documentation/memory-barriers.txt.

    Mathieu

    >
    > Nicolas


    --
    Mathieu Desnoyers
    OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    On Fri, Nov 07, 2008 at 03:45:46PM -0500, Mathieu Desnoyers wrote:
    > * Mathieu Desnoyers (mathieu.desnoyers@polymtl.ca) wrote:
    > > * Peter Zijlstra (a.p.zijlstra@chello.nl) wrote:
    > > > On Fri, 2008-11-07 at 14:18 -0500, Mathieu Desnoyers wrote:
    > > > > * Steven Rostedt (rostedt@goodmis.org) wrote:
    > > > > >
    > > > > > On Fri, 7 Nov 2008, Mathieu Desnoyers wrote:
    > > > > > >
    > > > > > > __m_cnt_hi
    > > > > > > is read before
    > > > > > > mmio cnt_lo read
    > > > > > >
    > > > > > > for the detailed reasons explained in my previous discussion with
    > > > > > > Nicolas here :
    > > > > > > http://lkml.org/lkml/2008/10/21/1
    > > > > > >
    > > > > > > I use smp_rmb() to do this on SMP systems (hrm, actually, a rmb() could
    > > > > > > be required so it works also on UP systems safely wrt interrupts).
    > > > > >
    > > > > > smp_rmb turns into a compiler barrier on UP and should prevent the below
    > > > > > description.
    > > > > >
    > > > >
    > > > > Ah, right, preserving program order on UP should be enough. smp_rmb()
    > > > > then.
    > > >
    > > >
    > > > I'm not quite sure I'm following here. Is this a global hardware clock
    > > > you're reading from multiple cpus, if so, are you sure smp_rmb() will
    > > > indeed be enough to sync the read?
    > > >
    > > > (In which case the smp_wmb() is provided by the hardware increasing the
    > > > clock?)
    > > >
    > > > If these are per-cpu clocks then even in the smp case we'd be good with
    > > > a plain barrier() because you'd only ever want to read your own cpu's
    > > > clock (and have a separate __m_cnt_hi per cpu).
    > > >
    > > > Or am I totally missing out on something?
    > > >

    > >
    > > This is the global hardware clock scenario.
    > >
    > > We have to order an uncached mmio read wrt a cached variable read/write.
    > > The uncached mmio read vs smp_rmb() barrier (e.g. lfence instruction)
    > > should be insured by program order because the read will skip the cache
    > > and go directly to the bus. Luckily we only do a mmio read and no mmio
    > > write, so mmiowb() is not required.
    > >
    > > You might be right in that it could require more barriers.
    > >
    > > Given adequate program order, we can assume the the mmio read will
    > > happen "on the spot", but that the cached read may be delayed.
    > >
    > > What we want is :
    > >
    > > readl(io_addr)
    > > read __m_cnt_hi
    > > write __m_cnt_hi
    > >
    > > With the two reads in the correct order. If we consider two consecutive
    > > executions on the same CPU :
    > >
    > > readl(io_addr)
    > > read __m_cnt_hi
    > > write __m_cnt_hi
    > >
    > > readl(io_addr)
    > > read __m_cnt_hi
    > > write __m_cnt_hi
    > >
    > > We might have to order the read/write pair wrt the following readl, such
    > > as :
    > >
    > > smp_rmb(); /* Waits for every cached memory reads to complete */
    > > readl(io_addr);
    > > barrier(); /* Make sure the compiler leaves mmio read before cached read */
    > > read __m_cnt_hi
    > > write __m_cnt_hi
    > >
    > > smp_rmb(); /* Waits for every cached memory reads to complete */
    > > readl(io_addr)
    > > barrier(); /* Make sure the compiler leaves mmio read before cached read */
    > > read __m_cnt_hi
    > > write __m_cnt_hi
    > >
    > > Would that make more sense ?
    > >

    >
    > Oh, actually, I got things reversed in this email : the readl(io_addr)
    > must be done _after_ the __m_cnt_hi read.
    >
    > Therefore, two consecutive executions would look like :
    >
    > barrier(); /* Make sure the compiler does not reorder __m_cnt_hi and
    > previous mmio read. */
    > read __m_cnt_hi
    > smp_rmb(); /* Waits for every cached memory reads to complete */


    If these are MMIO reads, then you need rmb() rather than smp_rmb(),
    at least on architectures that can reorder writes (Power, Itanium,
    and I believe also ARM, ...).

    Thanx, Paul

    > readl(io_addr);
    > write __m_cnt_hi
    >
    >
    > barrier(); /* Make sure the compiler does not reorder __m_cnt_hi and
    > previous mmio read. */
    > read __m_cnt_hi
    > smp_rmb(); /* Waits for every cached memory reads to complete */
    > readl(io_addr);
    > write __m_cnt_hi
    >
    > Mathieu
    >
    > > Mathieu
    > >
    > > --
    > > Mathieu Desnoyers
    > > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

    >
    > --
    > Mathieu Desnoyers
    > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    On Fri, Nov 07, 2008 at 03:08:12PM -0500, Steven Rostedt wrote:
    >
    > On Fri, 7 Nov 2008, Mathieu Desnoyers wrote:
    > >
    > > I want to make sure
    > >
    > > __m_cnt_hi
    > > is read before
    > > mmio cnt_lo read

    >
    > Hmm, let me make sure I understand why there is no wmb.
    >
    > Paul, can you verify this?
    >
    > Mathieu, you do the following:
    >
    > read a
    > smp_rmb
    > reab b
    > if (test b)
    > write a
    >
    > So the idea is that you must read b to test it. And since we must read a
    > before reading b we can see that we write a before either?
    >
    > The question remains, can the write happen before either of the reads?
    >
    > But since the read b is reading the hw clock, perhaps that just implies a
    > wmb on the hardware side?


    The hardware must do an equivalent of a wmb(), but this might well be
    done in logic or firmware on the device itself.

    Thanx, Paul
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()


    On Fri, 7 Nov 2008, Paul E. McKenney wrote:
    > > > Would that make more sense ?
    > > >

    > >
    > > Oh, actually, I got things reversed in this email : the readl(io_addr)
    > > must be done _after_ the __m_cnt_hi read.
    > >
    > > Therefore, two consecutive executions would look like :
    > >
    > > barrier(); /* Make sure the compiler does not reorder __m_cnt_hi and
    > > previous mmio read. */
    > > read __m_cnt_hi
    > > smp_rmb(); /* Waits for every cached memory reads to complete */

    >
    > If these are MMIO reads, then you need rmb() rather than smp_rmb(),
    > at least on architectures that can reorder writes (Power, Itanium,
    > and I believe also ARM, ...).


    The read is from a clock source. The only writes that are happening is
    by the clock itself.

    On a UP system, is a rmb still needed? That is, can you have two reads on
    the same CPU from the clock source that will produce a backwards clock?
    That to me sounds like the clock interface is broken.

    -- Steve
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
    > On Fri, Nov 07, 2008 at 03:45:46PM -0500, Mathieu Desnoyers wrote:
    > > * Mathieu Desnoyers (mathieu.desnoyers@polymtl.ca) wrote:
    > > > * Peter Zijlstra (a.p.zijlstra@chello.nl) wrote:
    > > > > On Fri, 2008-11-07 at 14:18 -0500, Mathieu Desnoyers wrote:
    > > > > > * Steven Rostedt (rostedt@goodmis.org) wrote:
    > > > > > >
    > > > > > > On Fri, 7 Nov 2008, Mathieu Desnoyers wrote:
    > > > > > > >
    > > > > > > > __m_cnt_hi
    > > > > > > > is read before
    > > > > > > > mmio cnt_lo read
    > > > > > > >
    > > > > > > > for the detailed reasons explained in my previous discussion with
    > > > > > > > Nicolas here :
    > > > > > > > http://lkml.org/lkml/2008/10/21/1
    > > > > > > >
    > > > > > > > I use smp_rmb() to do this on SMP systems (hrm, actually, a rmb() could
    > > > > > > > be required so it works also on UP systems safely wrt interrupts).
    > > > > > >
    > > > > > > smp_rmb turns into a compiler barrier on UP and should prevent the below
    > > > > > > description.
    > > > > > >
    > > > > >
    > > > > > Ah, right, preserving program order on UP should be enough. smp_rmb()
    > > > > > then.
    > > > >
    > > > >
    > > > > I'm not quite sure I'm following here. Is this a global hardware clock
    > > > > you're reading from multiple cpus, if so, are you sure smp_rmb() will
    > > > > indeed be enough to sync the read?
    > > > >
    > > > > (In which case the smp_wmb() is provided by the hardware increasing the
    > > > > clock?)
    > > > >
    > > > > If these are per-cpu clocks then even in the smp case we'd be good with
    > > > > a plain barrier() because you'd only ever want to read your own cpu's
    > > > > clock (and have a separate __m_cnt_hi per cpu).
    > > > >
    > > > > Or am I totally missing out on something?
    > > > >
    > > >
    > > > This is the global hardware clock scenario.
    > > >
    > > > We have to order an uncached mmio read wrt a cached variable read/write.
    > > > The uncached mmio read vs smp_rmb() barrier (e.g. lfence instruction)
    > > > should be insured by program order because the read will skip the cache
    > > > and go directly to the bus. Luckily we only do a mmio read and no mmio
    > > > write, so mmiowb() is not required.
    > > >
    > > > You might be right in that it could require more barriers.
    > > >
    > > > Given adequate program order, we can assume the the mmio read will
    > > > happen "on the spot", but that the cached read may be delayed.
    > > >
    > > > What we want is :
    > > >
    > > > readl(io_addr)
    > > > read __m_cnt_hi
    > > > write __m_cnt_hi
    > > >
    > > > With the two reads in the correct order. If we consider two consecutive
    > > > executions on the same CPU :
    > > >
    > > > readl(io_addr)
    > > > read __m_cnt_hi
    > > > write __m_cnt_hi
    > > >
    > > > readl(io_addr)
    > > > read __m_cnt_hi
    > > > write __m_cnt_hi
    > > >
    > > > We might have to order the read/write pair wrt the following readl, such
    > > > as :
    > > >
    > > > smp_rmb(); /* Waits for every cached memory reads to complete */
    > > > readl(io_addr);
    > > > barrier(); /* Make sure the compiler leaves mmio read before cached read */
    > > > read __m_cnt_hi
    > > > write __m_cnt_hi
    > > >
    > > > smp_rmb(); /* Waits for every cached memory reads to complete */
    > > > readl(io_addr)
    > > > barrier(); /* Make sure the compiler leaves mmio read before cached read */
    > > > read __m_cnt_hi
    > > > write __m_cnt_hi
    > > >
    > > > Would that make more sense ?
    > > >

    > >
    > > Oh, actually, I got things reversed in this email : the readl(io_addr)
    > > must be done _after_ the __m_cnt_hi read.
    > >
    > > Therefore, two consecutive executions would look like :
    > >
    > > barrier(); /* Make sure the compiler does not reorder __m_cnt_hi and
    > > previous mmio read. */
    > > read __m_cnt_hi
    > > smp_rmb(); /* Waits for every cached memory reads to complete */

    >
    > If these are MMIO reads, then you need rmb() rather than smp_rmb(),
    > at least on architectures that can reorder writes (Power, Itanium,
    > and I believe also ARM, ...).
    >
    > Thanx, Paul
    >



    I just dug into the barrier() question at the beginning of the code. I
    think it's not necessary after all, because the worse a compiler could
    do is probably the following :

    Read nr | code

    1 read a
    1 rmb()
    2 read a <------ ugh. Compiler could decide to prefetch the a value
    and only update it if the test is true
    1 read b
    1 if (test b) {
    1 write a
    2 read a
    }

    2 rmb()
    2 read b
    2 if (test b)
    2 write a

    But it would not mix the order of a/b reads. So I think just the rmb()
    would be enough.

    Mathieu


    > > readl(io_addr);
    > > write __m_cnt_hi
    > >
    > >
    > > barrier(); /* Make sure the compiler does not reorder __m_cnt_hi and
    > > previous mmio read. */
    > > read __m_cnt_hi
    > > smp_rmb(); /* Waits for every cached memory reads to complete */
    > > readl(io_addr);
    > > write __m_cnt_hi
    > >
    > > Mathieu
    > >
    > > > Mathieu
    > > >
    > > > --
    > > > Mathieu Desnoyers
    > > > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

    > >
    > > --
    > > Mathieu Desnoyers
    > > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

    >


    --
    Mathieu Desnoyers
    OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    On Fri, 7 Nov 2008, Mathieu Desnoyers wrote:

    > First off, read hw cnt low _is_ an uncached memory read (this is the
    > mmio read). __m_cnt_hi is a cached read, and therefore can be delayed if
    > the cache-line is busy. And we have no control on how much time can pass
    > between the two reads given the CPU may stall waiting for a cache-line.
    >
    > So the scenario above happens if CPU A have __m_cnt_hi in its cacheline,
    > but for come reason CPU B have to defer the cacheline read of __m_cnt_hi
    > due to heavy cacheline traffic and decides to proceed to mmio read
    > before the cacheline has been brought to the CPU because "hey, there is
    > no data dependency between those two reads !".


    OK that makes sense.


    Nicolas
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    * Steven Rostedt (rostedt@goodmis.org) wrote:
    >
    > On Fri, 7 Nov 2008, Mathieu Desnoyers wrote:
    > >
    > > I want to make sure
    > >
    > > __m_cnt_hi
    > > is read before
    > > mmio cnt_lo read

    >
    > Hmm, let me make sure I understand why there is no wmb.
    >
    > Paul, can you verify this?
    >
    > Mathieu, you do the following:
    >
    > read a
    > smp_rmb
    > reab b
    > if (test b)
    > write a
    >
    > So the idea is that you must read b to test it. And since we must read a
    > before reading b we can see that we write a before either?
    >
    > The question remains, can the write happen before either of the reads?
    >


    write a cannot happen before read a (same variable).
    write a must happen after read b because it depends on the b value. It
    makes sure the the side-effect of "write a" is seen by other CPUs
    *after* we have read the b value.

    > But since the read b is reading the hw clock, perhaps that just implies a
    > wmb on the hardware side?
    >


    It makes sense. The hardware clock has no cache coherency problem.. so
    it could be seen as doing wmb() after each data update.

    Mathieu

    > -- Steve


    --
    Mathieu Desnoyers
    OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    * Russell King (rmk+lkml@arm.linux.org.uk) wrote:
    > On Fri, Nov 07, 2008 at 11:47:58AM -0500, Mathieu Desnoyers wrote:
    > > But any get_cycles() user of cnt32_to_63() should be shot down. The
    > > bright side is : there is no way get_cycles() can be used with this
    > > new code.
    > >
    > > e.g. of incorrect users for arm (unless they are UP only, but that seems
    > > like a weird design argument) :
    > >
    > > mach-sa1100/include/mach/SA-1100.h:#define OSCR __REG(0x90000010)
    > > /* OS timer Counter Reg. */
    > > mach-sa1100/generic.c: unsigned long long v = cnt32_to_63(OSCR);
    > > mach-pxa/include/mach/pxa-regs.h:#define OSCR __REG(0x40A00010) /* OS
    > > Timer Counter Register */
    > > mach-pxa/time.c: unsigned long long v = cnt32_to_63(OSCR);

    >
    > It's strange for you to make that assertion when PXA was the exact
    > platform that Nicolas created this code for - and that's a platform
    > where preempt has been widely used.
    >
    > The two you mention are both ARMv5 or older architectures, and the
    > first real SMP ARM architecture is ARMv6. So architecturally they
    > are UP only.
    >


    Ok. And hopefully they do not execute instructions speculatively ?
    Because then a instruction sync would be required between the __m_hi_cnt
    read and get_cycles.

    If you design such stuff with portability in mind, you'd use per-cpu
    variables, which ends up being a single variable in the single-cpu
    special-case.

    > So, tell me why you say "unless they are UP only, but that seems like
    > a weird design argument"? If the platforms can only ever be UP only,
    > what's wrong with UP only code being used with them? (Not that I'm
    > saying anything there about cnt32_to_63.)


    That's fine, as long as the code does not end up in include/linux and
    stays in arch/arm/up-only-subarch/.

    When one try to create architecture agnostic code (which is what is
    likely to be palatable to arch agnostic headers), designing with UP in
    mind does not make much sense.

    >
    > I'd like to see you modify the silicon of a PXA or SA11x0 SoC to add
    > more than one processor to the chip - maybe you could use evostick to
    > glue two dies together and a microscope to aid bonding wires between
    > the two? (Of course, you'd need to design something to ensure cache
    > coherence as well, and arbitrate the internal bus between the two
    > dies.)
    >
    > Personally, I think that's highly unlikely.
    >


    Very unlikely indeed.

    Mathieu

    > --
    > Russell King
    > Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
    > maintainer of:


    --
    Mathieu Desnoyers
    OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    On Fri, Nov 07, 2008 at 04:36:10PM -0500, Mathieu Desnoyers wrote:
    > * Russell King (rmk+lkml@arm.linux.org.uk) wrote:
    > > On Fri, Nov 07, 2008 at 11:47:58AM -0500, Mathieu Desnoyers wrote:
    > > > But any get_cycles() user of cnt32_to_63() should be shot down. The
    > > > bright side is : there is no way get_cycles() can be used with this
    > > > new code.
    > > >
    > > > e.g. of incorrect users for arm (unless they are UP only, but that seems
    > > > like a weird design argument) :
    > > >
    > > > mach-sa1100/include/mach/SA-1100.h:#define OSCR __REG(0x90000010)
    > > > /* OS timer Counter Reg. */
    > > > mach-sa1100/generic.c: unsigned long long v = cnt32_to_63(OSCR);
    > > > mach-pxa/include/mach/pxa-regs.h:#define OSCR __REG(0x40A00010) /* OS
    > > > Timer Counter Register */
    > > > mach-pxa/time.c: unsigned long long v = cnt32_to_63(OSCR);

    > >
    > > It's strange for you to make that assertion when PXA was the exact
    > > platform that Nicolas created this code for - and that's a platform
    > > where preempt has been widely used.
    > >
    > > The two you mention are both ARMv5 or older architectures, and the
    > > first real SMP ARM architecture is ARMv6. So architecturally they
    > > are UP only.

    >
    > Ok. And hopefully they do not execute instructions speculatively ?


    Again, that's ARMv6 and later.

    > Because then a instruction sync would be required between the __m_hi_cnt
    > read and get_cycles.


    What get_cycles? This is the ARM implementation of get_cycles():

    static inline cycles_t get_cycles (void)
    {
    return 0;
    }

    Maybe you're using a name for one thing which means something else to
    other people? Please don't use confusing vocabulary.

    > If you design such stuff with portability in mind, you'd use per-cpu
    > variables, which ends up being a single variable in the single-cpu
    > special-case.


    Explain how and why sched_clock(), which is a global time source, should
    use per-cpu variables.

    > > So, tell me why you say "unless they are UP only, but that seems like
    > > a weird design argument"? If the platforms can only ever be UP only,
    > > what's wrong with UP only code being used with them? (Not that I'm
    > > saying anything there about cnt32_to_63.)

    >
    > That's fine, as long as the code does not end up in include/linux and
    > stays in arch/arm/up-only-subarch/.


    Well, that's where it was - private to ARM. Then David Howells came
    along and unilaterally - and without reference to anyone as far as I
    can see - moved it to include/linux.

    Neither Nicolas, nor me had any idea that it was going to move into
    include/linux - the first we knew of it was when pulling the change
    from Linus' tree.

    Look, if people in the kernel community can't or won't communicate
    with others (either through malice, purpose or accident), you can
    expect this kind of crap to happen.

    > When one try to create architecture agnostic code (which is what is
    > likely to be palatable to arch agnostic headers), designing with UP in
    > mind does not make much sense.


    It wasn't architecture agnostic code. It was ARM specific. We have
    a "version control system" which stores "comments" for changes to the
    kernel tree. Please use it to find out the true story. I'll save
    you the trouble, here's the commits with full comments:

    $ git log include/linux/cnt32_to_63.h
    commit b4f151ff899362fec952c45d166252c9912c041f
    Author: David Howells
    Date: Wed Sep 24 17:48:26 2008 +0100

    MN10300: Move asm-arm/cnt32_to_63.h to include/linux/

    Move asm-arm/cnt32_to_63.h to include/linux/ so that MN10300 can make
    use of it too.

    Signed-off-by: David Howells
    Signed-off-by: Linus Torvalds

    $ git log -- arch/arm/include/asm/cnt32_to_63.h include/asm-arm/cnt32_to_63.h
    commit bc173c5789e1fc6065fd378edc815914b40ee86b
    Author: David Howells
    Date: Fri Sep 26 16:22:58 2008 +0100

    ARM: Delete ARM's own cnt32_to_63.h

    Delete ARM's own cnt32_to_63.h as the copy in include/linux/ should now be
    used instead.

    Signed-off-by: David Howells
    Signed-off-by: Linus Torvalds

    commit 4baa9922430662431231ac637adedddbb0cfb2d7
    Author: Russell King
    Date: Sat Aug 2 10:55:55 2008 +0100

    [ARM] move include/asm-arm to arch/arm/include/asm

    Move platform independent header files to arch/arm/include/asm, leaving
    those in asm/arch* and asm/plat* alone.

    Signed-off-by: Russell King

    commit 838ccbc35eae5b44d47724e5f694dbec4a26d269
    Author: Nicolas Pitre
    Date: Mon Dec 4 20:19:31 2006 +0100

    [ARM] 3978/1: macro to provide a 63-bit value from a 32-bit hardware counter

    This is done in a completely lockless fashion. Bits 0 to 31 of the count
    are provided by the hardware while bits 32 to 62 are stored in memory.
    The top bit in memory is used to synchronize with the hardware count
    half-period. When the top bit of both counters (hardware and in memory)
    differ then the memory is updated with a new value, incrementing it when
    the hardware counter wraps around. Because a word store in memory is
    atomic then the incremented value will always be in synch with the top
    bit indicating to any potential concurrent reader if the value in memory
    is up to date or not wrt the needed increment. And any race in updating
    the value in memory is harmless as the same value would be stored more
    than once.

    Signed-off-by: Nicolas Pitre
    Signed-off-by: Russell King

    So, stop slinging mud onto Nicolas and me over this. The resulting
    mess is clearly not our creation.

    --
    Russell King
    Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
    maintainer of:
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    * Russell King (rmk+lkml@arm.linux.org.uk) wrote:
    > On Fri, Nov 07, 2008 at 04:36:10PM -0500, Mathieu Desnoyers wrote:
    > > * Russell King (rmk+lkml@arm.linux.org.uk) wrote:
    > > > On Fri, Nov 07, 2008 at 11:47:58AM -0500, Mathieu Desnoyers wrote:
    > > > > But any get_cycles() user of cnt32_to_63() should be shot down. The
    > > > > bright side is : there is no way get_cycles() can be used with this
    > > > > new code.
    > > > >
    > > > > e.g. of incorrect users for arm (unless they are UP only, but that seems
    > > > > like a weird design argument) :
    > > > >
    > > > > mach-sa1100/include/mach/SA-1100.h:#define OSCR __REG(0x90000010)
    > > > > /* OS timer Counter Reg. */
    > > > > mach-sa1100/generic.c: unsigned long long v = cnt32_to_63(OSCR);
    > > > > mach-pxa/include/mach/pxa-regs.h:#define OSCR __REG(0x40A00010) /* OS
    > > > > Timer Counter Register */
    > > > > mach-pxa/time.c: unsigned long long v = cnt32_to_63(OSCR);
    > > >
    > > > It's strange for you to make that assertion when PXA was the exact
    > > > platform that Nicolas created this code for - and that's a platform
    > > > where preempt has been widely used.
    > > >
    > > > The two you mention are both ARMv5 or older architectures, and the
    > > > first real SMP ARM architecture is ARMv6. So architecturally they
    > > > are UP only.

    > >
    > > Ok. And hopefully they do not execute instructions speculatively ?

    >
    > Again, that's ARMv6 and later.
    >
    > > Because then a instruction sync would be required between the __m_hi_cnt
    > > read and get_cycles.

    >
    > What get_cycles? This is the ARM implementation of get_cycles():
    >
    > static inline cycles_t get_cycles (void)
    > {
    > return 0;
    > }
    >
    > Maybe you're using a name for one thing which means something else to
    > other people? Please don't use confusing vocabulary.
    >


    get_cycles() is expected to be a cpu register read which reads a cycle
    counter. As far as I can tell,

    #define OSCR __REG(0x90000010) /* OS timer Counter Reg. */

    seems to fit this definition pretty closely. Or maybe not ?

    I am personnally not trying to find someone to blame. Just trying to
    figure out how to fix the existing code or, at the very least, to make
    sure nobody will ask me to use a piece of code not suitable for
    tracing clock source purposes.

    Mathieu

    > > If you design such stuff with portability in mind, you'd use per-cpu
    > > variables, which ends up being a single variable in the single-cpu
    > > special-case.

    >
    > Explain how and why sched_clock(), which is a global time source, should
    > use per-cpu variables.
    >
    > > > So, tell me why you say "unless they are UP only, but that seems like
    > > > a weird design argument"? If the platforms can only ever be UP only,
    > > > what's wrong with UP only code being used with them? (Not that I'm
    > > > saying anything there about cnt32_to_63.)

    > >
    > > That's fine, as long as the code does not end up in include/linux and
    > > stays in arch/arm/up-only-subarch/.

    >
    > Well, that's where it was - private to ARM. Then David Howells came
    > along and unilaterally - and without reference to anyone as far as I
    > can see - moved it to include/linux.
    >
    > Neither Nicolas, nor me had any idea that it was going to move into
    > include/linux - the first we knew of it was when pulling the change
    > from Linus' tree.
    >
    > Look, if people in the kernel community can't or won't communicate
    > with others (either through malice, purpose or accident), you can
    > expect this kind of crap to happen.
    >
    > > When one try to create architecture agnostic code (which is what is
    > > likely to be palatable to arch agnostic headers), designing with UP in
    > > mind does not make much sense.

    >
    > It wasn't architecture agnostic code. It was ARM specific. We have
    > a "version control system" which stores "comments" for changes to the
    > kernel tree. Please use it to find out the true story. I'll save
    > you the trouble, here's the commits with full comments:
    >
    > $ git log include/linux/cnt32_to_63.h
    > commit b4f151ff899362fec952c45d166252c9912c041f
    > Author: David Howells
    > Date: Wed Sep 24 17:48:26 2008 +0100
    >
    > MN10300: Move asm-arm/cnt32_to_63.h to include/linux/
    >
    > Move asm-arm/cnt32_to_63.h to include/linux/ so that MN10300 can make
    > use of it too.
    >
    > Signed-off-by: David Howells
    > Signed-off-by: Linus Torvalds
    >
    > $ git log -- arch/arm/include/asm/cnt32_to_63.h include/asm-arm/cnt32_to_63.h
    > commit bc173c5789e1fc6065fd378edc815914b40ee86b
    > Author: David Howells
    > Date: Fri Sep 26 16:22:58 2008 +0100
    >
    > ARM: Delete ARM's own cnt32_to_63.h
    >
    > Delete ARM's own cnt32_to_63.h as the copy in include/linux/ should now be
    > used instead.
    >
    > Signed-off-by: David Howells
    > Signed-off-by: Linus Torvalds
    >
    > commit 4baa9922430662431231ac637adedddbb0cfb2d7
    > Author: Russell King
    > Date: Sat Aug 2 10:55:55 2008 +0100
    >
    > [ARM] move include/asm-arm to arch/arm/include/asm
    >
    > Move platform independent header files to arch/arm/include/asm, leaving
    > those in asm/arch* and asm/plat* alone.
    >
    > Signed-off-by: Russell King
    >
    > commit 838ccbc35eae5b44d47724e5f694dbec4a26d269
    > Author: Nicolas Pitre
    > Date: Mon Dec 4 20:19:31 2006 +0100
    >
    > [ARM] 3978/1: macro to provide a 63-bit value from a 32-bit hardware counter
    >
    > This is done in a completely lockless fashion. Bits 0 to 31 of the count
    > are provided by the hardware while bits 32 to 62 are stored in memory.
    > The top bit in memory is used to synchronize with the hardware count
    > half-period. When the top bit of both counters (hardware and in memory)
    > differ then the memory is updated with a new value, incrementing it when
    > the hardware counter wraps around. Because a word store in memory is
    > atomic then the incremented value will always be in synch with the top
    > bit indicating to any potential concurrent reader if the value in memory
    > is up to date or not wrt the needed increment. And any race in updating
    > the value in memory is harmless as the same value would be stored more
    > than once.
    >
    > Signed-off-by: Nicolas Pitre
    > Signed-off-by: Russell King
    >
    > So, stop slinging mud onto Nicolas and me over this. The resulting
    > mess is clearly not our creation.
    >
    > --
    > Russell King
    > Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
    > maintainer of:


    --
    Mathieu Desnoyers
    OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    Mathieu Desnoyers wrote:

    > Yes. Do you think the synchronization of the cycles counters is
    > _perfect_ across CPUs so that there is no possible way whatsoever that
    > two cycle counter values appear to go backward between CPUs ? (also
    > taking in account delays in __m_cnt_hi write-back...)


    Given there's currently only one CPU allowed, yes, I think it's perfect:-)

    It's something to re-evaluate should Panasonic decide to do SMP.

    > If we expect the only correct use-case to be with readl(), I don't see
    > the problem with added synchronization.


    It might be expensive if you don't actually want to call readl(). But that's
    on a par with using funky instructions to read the TSC, I guess.

    David
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    Russell King wrote:

    > Well, that's where it was - private to ARM. Then David Howells came
    > along and unilaterally - and without reference to anyone as far as I
    > can see - moved it to include/linux.
    >
    > Neither Nicolas, nor me had any idea that it was going to move into
    > include/linux - the first we knew of it was when pulling the change
    > from Linus' tree.
    >
    > Look, if people in the kernel community can't or won't communicate
    > with others (either through malice, purpose or accident), you can
    > expect this kind of crap to happen.


    Excuse me, Russell, but I sent Nicolas an email prior to doing so asking him
    if he had any objections:

    To: Nicolas Pitre
    cc: dhowells@redhat.com
    Subject: Moving asm-arm/cnt32_to_63.h to include/linux/
    Date: Thu, 31 Jul 2008 16:04:04 +0100

    Hi Nicolas,

    Mind if I move include/asm-arm/cnt32_to_63.h to include/linux/?

    I need to use it for MN10300.

    David

    He didn't respond. Not only that, but I copied Nicolas on the patch to make
    the move and the patch to make MN10300 use it when I submitted it to Linus on
    the 24th September, so it's not like he didn't have plenty of time. He
    certainly saw that because he joined in the discussion of the second patch.
    Furthermore, he could've asked Linus to refuse the patch, or to revert it if
    it had already gone in.

    I suppose I should've cc'd the ARM list too... but why should it adversely
    affect ARM?

    David
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    Nicolas Pitre wrote:

    > I explained several times already why I disagree. Preemption is not a
    > problem unless you're preempted away for long enough, or IOW if your
    > counter is too fast.


    That's my point. Say a nice -19 process gets preempted... what guarantees
    that it will resume within, the time it takes the counter to wrap? Even if
    the preempting process goes back to sleep, in that time a bunch of other
    processes could have woken up and could starve it for a long period of time.

    David
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  18. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    Steven Rostedt wrote:

    > > I use smp_rmb() to do this on SMP systems (hrm, actually, a rmb() could
    > > be required so it works also on UP systems safely wrt interrupts).

    >
    > smp_rmb turns into a compiler barrier on UP and should prevent the below
    > description.


    Note that that does not guarantee that the two reads will be done in the order
    you want. The compiler barrier _only_ affects the compiler. It does not stop
    the CPU from doing the reads in any order it wants. You need something
    stronger than smp_rmb() if you need the reads to be so ordered.

    David
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  19. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    On Fri, Nov 07, 2008 at 11:41:55PM +0000, David Howells wrote:
    > Russell King wrote:
    >
    > > Well, that's where it was - private to ARM. Then David Howells came
    > > along and unilaterally - and without reference to anyone as far as I
    > > can see - moved it to include/linux.
    > >
    > > Neither Nicolas, nor me had any idea that it was going to move into
    > > include/linux - the first we knew of it was when pulling the change
    > > from Linus' tree.
    > >
    > > Look, if people in the kernel community can't or won't communicate
    > > with others (either through malice, purpose or accident), you can
    > > expect this kind of crap to happen.

    >
    > Excuse me, Russell, but I sent Nicolas an email prior to doing so asking him
    > if he had any objections:
    >
    > To: Nicolas Pitre
    > cc: dhowells@redhat.com
    > Subject: Moving asm-arm/cnt32_to_63.h to include/linux/
    > Date: Thu, 31 Jul 2008 16:04:04 +0100
    >
    > Hi Nicolas,
    >
    > Mind if I move include/asm-arm/cnt32_to_63.h to include/linux/?
    >
    > I need to use it for MN10300.
    >
    > David
    >
    > He didn't respond. Not only that, but I copied Nicolas on the patch to make
    > the move and the patch to make MN10300 use it when I submitted it to Linus on
    > the 24th September, so it's not like he didn't have plenty of time. He
    > certainly saw that because he joined in the discussion of the second patch.
    > Furthermore, he could've asked Linus to refuse the patch, or to revert it if
    > it had already gone in.
    >
    > I suppose I should've cc'd the ARM list too... but why should it adversely
    > affect ARM?


    I take back the "Neither Nicolas" bit but the rest of my comment stands
    and remains valid.

    In light of akpm's demands to know how this got into the kernel, I decided
    I'd put the story forward, especially as people in this thread are confused
    about what it was designed for, and making random unfounded claiming that
    its existing ARM uses are buggy when they aren't.

    It sounds to me as if the right answer is for it to move back to being an
    ARM private thing with a MN10300 private copy, rather than it pretending
    to be something everyone can use.

    --
    Russell King
    Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
    maintainer of:
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  20. Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()

    On Fri, Nov 07, 2008 at 04:04:48PM -0500, Steven Rostedt wrote:
    >
    > On Fri, 7 Nov 2008, Paul E. McKenney wrote:
    > > > > Would that make more sense ?
    > > > >
    > > >
    > > > Oh, actually, I got things reversed in this email : the readl(io_addr)
    > > > must be done _after_ the __m_cnt_hi read.
    > > >
    > > > Therefore, two consecutive executions would look like :
    > > >
    > > > barrier(); /* Make sure the compiler does not reorder __m_cnt_hi and
    > > > previous mmio read. */
    > > > read __m_cnt_hi
    > > > smp_rmb(); /* Waits for every cached memory reads to complete */

    > >
    > > If these are MMIO reads, then you need rmb() rather than smp_rmb(),
    > > at least on architectures that can reorder writes (Power, Itanium,
    > > and I believe also ARM, ...).

    >
    > The read is from a clock source. The only writes that are happening is
    > by the clock itself.
    >
    > On a UP system, is a rmb still needed? That is, can you have two reads on
    > the same CPU from the clock source that will produce a backwards clock?
    > That to me sounds like the clock interface is broken.


    I do not believe that all CPUs are guaranteed to execute a sequence
    of MMIO reads in order.

    Thanx, Paul
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 4 of 6 FirstFirst ... 2 3 4 5 6 LastLast