LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v6 08/46] CPU hotplug: Provide APIs to prevent CPU offline from atomic context
From: Michel Lespinasse @ 2013-02-18 16:23 UTC (permalink / raw)
  To: Srivatsa S. Bhat
  Cc: linux-doc, peterz, fweisbec, linux-kernel, mingo, linux-arch,
	linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
	rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev, oleg,
	vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <20130218123920.26245.56709.stgit@srivatsabhat.in.ibm.com>

On Mon, Feb 18, 2013 at 8:39 PM, Srivatsa S. Bhat
<srivatsa.bhat@linux.vnet.ibm.com> wrote:
> Some important design requirements and considerations:
> -----------------------------------------------------
>
> 1. Scalable synchronization at the reader-side, especially in the fast-path
>
>    Any synchronization at the atomic hotplug readers side must be highly
>    scalable - avoid global single-holder locks/counters etc. Because, these
>    paths currently use the extremely fast preempt_disable(); our replacement
>    to preempt_disable() should not become ridiculously costly and also should
>    not serialize the readers among themselves needlessly.
>
>    At a minimum, the new APIs must be extremely fast at the reader side
>    atleast in the fast-path, when no CPU offline writers are active.
>
> 2. preempt_disable() was recursive. The replacement should also be recursive.
>
> 3. No (new) lock-ordering restrictions
>
>    preempt_disable() was super-flexible. It didn't impose any ordering
>    restrictions or rules for nesting. Our replacement should also be equally
>    flexible and usable.
>
> 4. No deadlock possibilities
>
>    Regular per-cpu locking is not the way to go if we want to have relaxed
>    rules for lock-ordering. Because, we can end up in circular-locking
>    dependencies as explained in https://lkml.org/lkml/2012/12/6/290
>
>    So, avoid the usual per-cpu locking schemes (per-cpu locks/per-cpu atomic
>    counters with spin-on-contention etc) as much as possible, to avoid
>    numerous deadlock possibilities from creeping in.
>
>
> Implementation of the design:
> ----------------------------
>
> We use per-CPU reader-writer locks for synchronization because:
>
>   a. They are quite fast and scalable in the fast-path (when no writers are
>      active), since they use fast per-cpu counters in those paths.
>
>   b. They are recursive at the reader side.
>
>   c. They provide a good amount of safety against deadlocks; they don't
>      spring new deadlock possibilities on us from out of nowhere. As a
>      result, they have relaxed locking rules and are quite flexible, and
>      thus are best suited for replacing usages of preempt_disable() or
>      local_irq_disable() at the reader side.
>
> Together, these satisfy all the requirements mentioned above.

Thanks for this detailed design explanation.

> +/*
> + * Invoked by atomic hotplug reader (a task which wants to prevent
> + * CPU offline, but which can't afford to sleep), to prevent CPUs from
> + * going offline. So, you can call this function from atomic contexts
> + * (including interrupt handlers).
> + *
> + * Note: This does NOT prevent CPUs from coming online! It only prevents
> + * CPUs from going offline.
> + *
> + * You can call this function recursively.
> + *
> + * Returns with preemption disabled (but interrupts remain as they are;
> + * they are not disabled).
> + */
> +void get_online_cpus_atomic(void)
> +{
> +       percpu_read_lock_irqsafe(&hotplug_pcpu_rwlock);
> +}
> +EXPORT_SYMBOL_GPL(get_online_cpus_atomic);
> +
> +void put_online_cpus_atomic(void)
> +{
> +       percpu_read_unlock_irqsafe(&hotplug_pcpu_rwlock);
> +}
> +EXPORT_SYMBOL_GPL(put_online_cpus_atomic);

So, you made it clear why you want a recursive read side here.

I am wondering though, if you could take care of recursive uses in
get/put_online_cpus_atomic() instead of doing it as a property of your
rwlock:

get_online_cpus_atomic()
{
    unsigned long flags;
    local_irq_save(flags);
    if (this_cpu_inc_return(hotplug_recusion_count) == 1)
        percpu_read_lock_irqsafe(&hotplug_pcpu_rwlock);
    local_irq_restore(flags);
}

Once again, the idea there is to avoid baking the reader side
recursive properties into your rwlock itself, so that it won't be
impossible to implement reader/writer fairness into your rwlock in the
future (which may be not be very important for the hotplug use, but
could be when other uses get introduced).

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

^ permalink raw reply

* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Steven Rostedt @ 2013-02-18 16:31 UTC (permalink / raw)
  To: Srivatsa S. Bhat
  Cc: linux-doc, peterz, fweisbec, linux-kernel, Michel Lespinasse,
	mingo, linux-arch, linux, xiaoguangrong, wangyun, paulmck, nikunj,
	linux-pm, rusty, rjw, namhyung, tglx, linux-arm-kernel, netdev,
	oleg, vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <5122551E.1080703@linux.vnet.ibm.com>

On Mon, 2013-02-18 at 21:51 +0530, Srivatsa S. Bhat wrote:
> Hi Michel,

> Yes.. I don't think we can avoid that. Moreover, since we _want_ unfair
> reader/writer semantics to allow flexible locking rules and guarantee
> deadlock-safety, having a recursive reader side is not even an issue, IMHO.

Recursive unfair reader lock may guarantee deadlock-safety, but
remember, it adds a higher probability of live-locking the write_lock.
Which is another argument to keep this separate to cpu hotplug only.

-- Steve

^ permalink raw reply

* Re: [PATCH v6 07/46] percpu_rwlock: Allow writers to be readers, and add lockdep annotations
From: Srivatsa S. Bhat @ 2013-02-18 16:31 UTC (permalink / raw)
  To: Michel Lespinasse
  Cc: linux-doc, peterz, fweisbec, linux-kernel, namhyung, mingo,
	linux-arch, linux, xiaoguangrong, wangyun, paulmck, nikunj,
	linux-pm, rusty, rostedt, rjw, vincent.guittot, tglx,
	linux-arm-kernel, netdev, oleg, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <CANN689Ec4g7jMapv8ChAEom+gEDQ8RDVLa07CKwJ==YO+MUUrA@mail.gmail.com>

On 02/18/2013 09:21 PM, Michel Lespinasse wrote:
> On Mon, Feb 18, 2013 at 8:39 PM, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>> @@ -200,6 +217,16 @@ void percpu_write_lock_irqsave(struct percpu_rwlock *pcpu_rwlock,
>>
>>         smp_mb(); /* Complete the wait-for-readers, before taking the lock */
>>         write_lock_irqsave(&pcpu_rwlock->global_rwlock, *flags);
>> +
>> +       /*
>> +        * It is desirable to allow the writer to acquire the percpu-rwlock
>> +        * for read (if necessary), without deadlocking or getting complaints
>> +        * from lockdep. To achieve that, just increment the reader_refcnt of
>> +        * this CPU - that way, any attempt by the writer to acquire the
>> +        * percpu-rwlock for read, will get treated as a case of nested percpu
>> +        * reader, which is safe, from a locking perspective.
>> +        */
>> +       this_cpu_inc(pcpu_rwlock->rw_state->reader_refcnt);
> 
> I find this quite disgusting, but once again this may be because I
> don't like unfair recursive rwlocks.
> 

:-)

> In my opinion, the alternative of explicitly not taking the read lock
> when one already has the write lock sounds *much* nicer.

I don't seem to recall any strong reasons to do it this way, so I don't have
any strong opinions on doing it this way. But one of the things to note is that,
in the CPU Hotplug case, the readers are *way* more hotter than the writer.
So avoiding extra checks/'if' conditions/memory barriers in the reader-side
is very welcome. (If we slow down the read-side, we get a performance hit
even when *not* doing hotplug!). Considering this, the logic used in this
patchset seems better, IMHO.

Regards,
Srivatsa S. Bhat

^ permalink raw reply

* Re: [PATCH v6 08/46] CPU hotplug: Provide APIs to prevent CPU offline from atomic context
From: Srivatsa S. Bhat @ 2013-02-18 16:43 UTC (permalink / raw)
  To: Michel Lespinasse
  Cc: linux-doc, peterz, fweisbec, linux-kernel, mingo, linux-arch,
	linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
	rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev, oleg,
	vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <CANN689HhVxDv+3Bn9UAxOMzj0egTvVp_c7oAmQ2nnQ8Xjn_aMA@mail.gmail.com>

On 02/18/2013 09:53 PM, Michel Lespinasse wrote:
> On Mon, Feb 18, 2013 at 8:39 PM, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>> Some important design requirements and considerations:
>> -----------------------------------------------------
[...]
>> +/*
>> + * Invoked by atomic hotplug reader (a task which wants to prevent
>> + * CPU offline, but which can't afford to sleep), to prevent CPUs from
>> + * going offline. So, you can call this function from atomic contexts
>> + * (including interrupt handlers).
>> + *
>> + * Note: This does NOT prevent CPUs from coming online! It only prevents
>> + * CPUs from going offline.
>> + *
>> + * You can call this function recursively.
>> + *
>> + * Returns with preemption disabled (but interrupts remain as they are;
>> + * they are not disabled).
>> + */
>> +void get_online_cpus_atomic(void)
>> +{
>> +       percpu_read_lock_irqsafe(&hotplug_pcpu_rwlock);
>> +}
>> +EXPORT_SYMBOL_GPL(get_online_cpus_atomic);
>> +
>> +void put_online_cpus_atomic(void)
>> +{
>> +       percpu_read_unlock_irqsafe(&hotplug_pcpu_rwlock);
>> +}
>> +EXPORT_SYMBOL_GPL(put_online_cpus_atomic);
> 
> So, you made it clear why you want a recursive read side here.
> 
> I am wondering though, if you could take care of recursive uses in
> get/put_online_cpus_atomic() instead of doing it as a property of your
> rwlock:
> 
> get_online_cpus_atomic()
> {
>     unsigned long flags;
>     local_irq_save(flags);
>     if (this_cpu_inc_return(hotplug_recusion_count) == 1)
>         percpu_read_lock_irqsafe(&hotplug_pcpu_rwlock);
>     local_irq_restore(flags);
> }
> 
> Once again, the idea there is to avoid baking the reader side
> recursive properties into your rwlock itself, so that it won't be
> impossible to implement reader/writer fairness into your rwlock in the
> future (which may be not be very important for the hotplug use, but
> could be when other uses get introduced).
> 

Hmm, your proposal above looks good to me, at first glance.
(Sorry, I had mistaken your earlier mails to mean that you were against
recursive reader-side, while you actually meant that you didn't like
implementing the recursive reader-side logic using the recursive property
of rwlocks).

While your idea above looks good, it might introduce more complexity
in the unlock path, since this would allow nesting of heterogeneous readers
(ie., if hotplug_recursion_count == 1, you don't know whether you need to
simply decrement the counter or unlock the rwlock as well).

But I'll give this some more thought to see if we can implement this
without making it too complex. Thank you!

Regards,
Srivatsa S. Bhat

^ permalink raw reply

* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Srivatsa S. Bhat @ 2013-02-18 16:46 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-doc, peterz, fweisbec, linux-kernel, Michel Lespinasse,
	mingo, linux-arch, linux, xiaoguangrong, wangyun, paulmck, nikunj,
	linux-pm, rusty, rjw, namhyung, tglx, linux-arm-kernel, netdev,
	oleg, vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <1361205087.23152.159.camel@gandalf.local.home>

On 02/18/2013 10:01 PM, Steven Rostedt wrote:
> On Mon, 2013-02-18 at 21:51 +0530, Srivatsa S. Bhat wrote:
>> Hi Michel,
> 
>> Yes.. I don't think we can avoid that. Moreover, since we _want_ unfair
>> reader/writer semantics to allow flexible locking rules and guarantee
>> deadlock-safety, having a recursive reader side is not even an issue, IMHO.
> 
> Recursive unfair reader lock may guarantee deadlock-safety, but
> remember, it adds a higher probability of live-locking the write_lock.
> Which is another argument to keep this separate to cpu hotplug only.
> 

True.
 
Regards,
Srivatsa S. Bhat

^ permalink raw reply

* Re: [PATCH v5 00/45] CPU hotplug: stop_machine()-free CPU hotplug
From: Vincent Guittot @ 2013-02-18 16:50 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-doc, peterz, fweisbec, linux-kernel, walken, mingo,
	linux-arch, Russell King - ARM Linux, xiaoguangrong, wangyun,
	paulmck, nikunj, linux-pm, Rusty Russell, rjw, namhyung, tglx,
	linux-arm-kernel, netdev, oleg, sbw, Srivatsa S. Bhat, tj, akpm,
	linuxppc-dev
In-Reply-To: <1361201422.23152.154.camel@gandalf.local.home>

On 18 February 2013 16:30, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 2013-02-18 at 11:58 +0100, Vincent Guittot wrote:
>
>> My tests have been done without cpuidle because i have some issues
>> with function tracer and cpuidle
>>
>> But the cpu hotplug and cpuidle work well when I run the tests without
>> enabling the function tracer
>>
>
> I know suspend and resume has issues with function tracing (because it
> makes things like calling smp_processor_id() crash the system), but I'm
> unaware of issues with hotplug itself. Could be some of the same issues.
>
> Can you give me more details, I'll try to investigate it.

yes for sure.
The problem is more linked to cpuidle and function tracer.

cpu hotplug and function tracer work when cpuidle is disable.
cpu hotplug and cpuidle works if i don't enable function tracer.
my platform is dead as soon as I enable function tracer if cpuidle is
enabled. It looks like some notrace are missing in my platform driver
but we haven't completely fix the issue yet

Vincent

>
> Thanks,
>
> -- Steve
>
>

^ permalink raw reply

* Accumulating PPC_FEATURE_ARCH bits
From: Richard Henderson @ 2013-02-18 17:06 UTC (permalink / raw)
  To: linuxppc-dev

If one wants to test the PPC_FEATURE_ARCH_ISA_2_0[56] bits, one has to 
be careful that if 2_06 is set, 2_05 won't be.  This seems illogical to 
me: given that 2.06 includes all of the 2.05 instructions, it would make 
most sense if both bits were set in the hwcap.

Now, obviously I have to code around existing practice, but going 
forward this isn't tennable.  When a Power 2.07 ISA is released, I'd 
rather not have existing programs degrade because suddenly the 2_06 bit 
is no longer set.

Thanks,


r~

^ permalink raw reply

* Re: [PATCH v6 08/46] CPU hotplug: Provide APIs to prevent CPU offline from atomic context
From: Michel Lespinasse @ 2013-02-18 17:21 UTC (permalink / raw)
  To: Srivatsa S. Bhat
  Cc: linux-doc, peterz, fweisbec, linux-kernel, mingo, linux-arch,
	linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
	rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev, oleg,
	vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <51225A36.40600@linux.vnet.ibm.com>

On Tue, Feb 19, 2013 at 12:43 AM, Srivatsa S. Bhat
<srivatsa.bhat@linux.vnet.ibm.com> wrote:
> On 02/18/2013 09:53 PM, Michel Lespinasse wrote:
>> I am wondering though, if you could take care of recursive uses in
>> get/put_online_cpus_atomic() instead of doing it as a property of your
>> rwlock:
>>
>> get_online_cpus_atomic()
>> {
>>     unsigned long flags;
>>     local_irq_save(flags);
>>     if (this_cpu_inc_return(hotplug_recusion_count) == 1)
>>         percpu_read_lock_irqsafe(&hotplug_pcpu_rwlock);
>>     local_irq_restore(flags);
>> }
>>
>> Once again, the idea there is to avoid baking the reader side
>> recursive properties into your rwlock itself, so that it won't be
>> impossible to implement reader/writer fairness into your rwlock in the
>> future (which may be not be very important for the hotplug use, but
>> could be when other uses get introduced).
>
> Hmm, your proposal above looks good to me, at first glance.
> (Sorry, I had mistaken your earlier mails to mean that you were against
> recursive reader-side, while you actually meant that you didn't like
> implementing the recursive reader-side logic using the recursive property
> of rwlocks).

To be honest, I was replying as I went through the series, so I hadn't
digested your hotplug use case yet :)

But yes - I don't like having the rwlock itself be recursive, but I do
understand that you have a legitimate requirement for
get_online_cpus_atomic() to be recursive. This IMO points to the
direction I suggested, of explicitly handling the recusion in
get_online_cpus_atomic() so that the underlying rwlock doesn't have to
support recursive reader side itself.

(And this would work for the idea of making writers own the reader
side as well - you can do it with the hotplug_recursion_count instead
of with the underlying rwlock).

> While your idea above looks good, it might introduce more complexity
> in the unlock path, since this would allow nesting of heterogeneous readers
> (ie., if hotplug_recursion_count == 1, you don't know whether you need to
> simply decrement the counter or unlock the rwlock as well).

Well, I think the idea doesn't make the underlying rwlock more
complex, since you could in principle keep your existing
percpu_read_lock_irqsafe implementation as is and just remove the
recursive behavior from its documentation.

Now ideally if we're adding a bit of complexity in
get_online_cpus_atomic() it'd be nice if we could remove some from
percpu_read_lock_irqsafe, but I haven't thought about that deeply
either. I think you'd still want to have the equivalent of a percpu
reader_refcnt, except it could now be a bool instead of an int, and
percpu_read_lock_irqsafe would still set it to back to 0/false after
acquiring the global read side if a writer is signaled. Basically your
existing percpu_read_lock_irqsafe code should still work, and we could
remove just the parts that were only there to deal with the recursive
property.

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

^ permalink raw reply

* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Srivatsa S. Bhat @ 2013-02-18 17:56 UTC (permalink / raw)
  To: Michel Lespinasse
  Cc: linux-doc, peterz, fweisbec, linux-kernel, namhyung, mingo,
	linux-arch, linux, xiaoguangrong, wangyun, paulmck, nikunj,
	linux-pm, rusty, rostedt, rjw, vincent.guittot, tglx,
	linux-arm-kernel, netdev, oleg, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <5122551E.1080703@linux.vnet.ibm.com>

On 02/18/2013 09:51 PM, Srivatsa S. Bhat wrote:
> Hi Michel,
> 
> On 02/18/2013 09:15 PM, Michel Lespinasse wrote:
>> Hi Srivasta,
>>
>> I admit not having followed in detail the threads about the previous
>> iteration, so some of my comments may have been discussed already
>> before - apologies if that is the case.
>>
>> On Mon, Feb 18, 2013 at 8:38 PM, Srivatsa S. Bhat
>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>> Reader-writer locks and per-cpu counters are recursive, so they can be
>>> used in a nested fashion in the reader-path, which makes per-CPU rwlocks also
>>> recursive. Also, this design of switching the synchronization scheme ensures
>>> that you can safely nest and use these locks in a very flexible manner.
[...]
>>>  void percpu_write_lock(struct percpu_rwlock *pcpu_rwlock)
>>>  {
>>> +       unsigned int cpu;
>>> +
>>> +       /*
>>> +        * Tell all readers that a writer is becoming active, so that they
>>> +        * start switching over to the global rwlock.
>>> +        */
>>> +       for_each_possible_cpu(cpu)
>>> +               per_cpu_ptr(pcpu_rwlock->rw_state, cpu)->writer_signal = true;
>>
>> I don't see anything preventing a race with the corresponding code in
>> percpu_write_unlock() that sets writer_signal back to false. Did I
>> miss something here ? It seems to me we don't have any guarantee that
>> all writer signals will be set to true at the end of the loop...
>>
> 
> Ah, thanks for pointing that out! IIRC Oleg had pointed this issue in the last
> version, but back then, I hadn't fully understood what he meant. Your
> explanation made it clear. I'll work on fixing this.
> 

We can fix this by using the simple patch (untested) shown below.
The alternative would be to acquire the rwlock for write, update the
->writer_signal values, release the lock, wait for readers to switch,
again acquire the rwlock for write with interrupts disabled etc... which
makes it kinda messy, IMHO. So I prefer the simple version shown below.


diff --git a/lib/percpu-rwlock.c b/lib/percpu-rwlock.c
index bf95e40..64ccd3f 100644
--- a/lib/percpu-rwlock.c
+++ b/lib/percpu-rwlock.c
@@ -50,6 +50,12 @@
 	(__this_cpu_read((pcpu_rwlock)->rw_state->writer_signal))
 
 
+/*
+ * Spinlock to synchronize access to the writer's data-structures
+ * (->writer_signal) from multiple writers.
+ */
+static DEFINE_SPINLOCK(writer_side_lock);
+
 int __percpu_init_rwlock(struct percpu_rwlock *pcpu_rwlock,
 			 const char *name, struct lock_class_key *rwlock_key)
 {
@@ -191,6 +197,8 @@ void percpu_write_lock_irqsave(struct percpu_rwlock *pcpu_rwlock,
 {
 	unsigned int cpu;
 
+	spin_lock(&writer_side_lock);
+
 	/*
 	 * Tell all readers that a writer is becoming active, so that they
 	 * start switching over to the global rwlock.
@@ -252,5 +260,6 @@ void percpu_write_unlock_irqrestore(struct percpu_rwlock *pcpu_rwlock,
 		per_cpu_ptr(pcpu_rwlock->rw_state, cpu)->writer_signal = false;
 
 	write_unlock_irqrestore(&pcpu_rwlock->global_rwlock, *flags);
+	spin_unlock(&writer_side_lock);
 }

^ permalink raw reply related

* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Michel Lespinasse @ 2013-02-18 18:07 UTC (permalink / raw)
  To: Srivatsa S. Bhat
  Cc: linux-doc, peterz, fweisbec, linux-kernel, namhyung, mingo,
	linux-arch, linux, xiaoguangrong, wangyun, paulmck, nikunj,
	linux-pm, rusty, rostedt, rjw, vincent.guittot, tglx,
	linux-arm-kernel, netdev, oleg, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <51226B46.9080707@linux.vnet.ibm.com>

On Tue, Feb 19, 2013 at 1:56 AM, Srivatsa S. Bhat
<srivatsa.bhat@linux.vnet.ibm.com> wrote:
> On 02/18/2013 09:51 PM, Srivatsa S. Bhat wrote:
>> On 02/18/2013 09:15 PM, Michel Lespinasse wrote:
>>> I don't see anything preventing a race with the corresponding code in
>>> percpu_write_unlock() that sets writer_signal back to false. Did I
>>> miss something here ? It seems to me we don't have any guarantee that
>>> all writer signals will be set to true at the end of the loop...
>>
>> Ah, thanks for pointing that out! IIRC Oleg had pointed this issue in the last
>> version, but back then, I hadn't fully understood what he meant. Your
>> explanation made it clear. I'll work on fixing this.
>
> We can fix this by using the simple patch (untested) shown below.
> The alternative would be to acquire the rwlock for write, update the
> ->writer_signal values, release the lock, wait for readers to switch,
> again acquire the rwlock for write with interrupts disabled etc... which
> makes it kinda messy, IMHO. So I prefer the simple version shown below.

Looks good.

Another alternative would be to make writer_signal an atomic integer
instead of a bool. That way writers can increment it before locking
and decrement it while unlocking.

To reduce the number of atomic ops during writer lock/unlock, the
writer_signal could also be a global read_mostly variable (I don't see
any downsides to that compared to having it percpu - or is it because
you wanted all the fastpath state to be in one single cacheline ?)

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

^ permalink raw reply

* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Srivatsa S. Bhat @ 2013-02-18 18:14 UTC (permalink / raw)
  To: Michel Lespinasse
  Cc: linux-doc, peterz, fweisbec, linux-kernel, mingo, linux-arch,
	linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
	rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev, oleg,
	vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <CANN689FSxOz+0Cu7EG_yKD8ZE1OpT4kyT+ybLfXSqaifodJRpw@mail.gmail.com>

On 02/18/2013 11:37 PM, Michel Lespinasse wrote:
> On Tue, Feb 19, 2013 at 1:56 AM, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>> On 02/18/2013 09:51 PM, Srivatsa S. Bhat wrote:
>>> On 02/18/2013 09:15 PM, Michel Lespinasse wrote:
>>>> I don't see anything preventing a race with the corresponding code in
>>>> percpu_write_unlock() that sets writer_signal back to false. Did I
>>>> miss something here ? It seems to me we don't have any guarantee that
>>>> all writer signals will be set to true at the end of the loop...
>>>
>>> Ah, thanks for pointing that out! IIRC Oleg had pointed this issue in the last
>>> version, but back then, I hadn't fully understood what he meant. Your
>>> explanation made it clear. I'll work on fixing this.
>>
>> We can fix this by using the simple patch (untested) shown below.
>> The alternative would be to acquire the rwlock for write, update the
>> ->writer_signal values, release the lock, wait for readers to switch,
>> again acquire the rwlock for write with interrupts disabled etc... which
>> makes it kinda messy, IMHO. So I prefer the simple version shown below.
> 
> Looks good.
> 
> Another alternative would be to make writer_signal an atomic integer
> instead of a bool. That way writers can increment it before locking
> and decrement it while unlocking.
> 

Yep, that would also do. But the spinlock version looks simpler - no need
to check if the atomic counter is non-zero, no need to explicitly spin in
a tight-loop etc.

> To reduce the number of atomic ops during writer lock/unlock, the
> writer_signal could also be a global read_mostly variable (I don't see
> any downsides to that compared to having it percpu - or is it because
> you wanted all the fastpath state to be in one single cacheline ?)
> 

Yes, we (Oleg and I) debated for a while about global vs percpu, and then
finally decided to go with percpu to have cache benefits.

Regards,
Srivatsa S. Bhat

^ permalink raw reply

* Re: [PATCH v6 08/46] CPU hotplug: Provide APIs to prevent CPU offline from atomic context
From: Srivatsa S. Bhat @ 2013-02-18 18:50 UTC (permalink / raw)
  To: Michel Lespinasse
  Cc: linux-doc, peterz, fweisbec, linux-kernel, mingo, linux-arch,
	linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
	rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev, oleg,
	vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <CANN689FJPLfz06MFRJ9T+BuToejAzjYP_yrw_H4iGi1Kb3p2-g@mail.gmail.com>

On 02/18/2013 10:51 PM, Michel Lespinasse wrote:
> On Tue, Feb 19, 2013 at 12:43 AM, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>> On 02/18/2013 09:53 PM, Michel Lespinasse wrote:
>>> I am wondering though, if you could take care of recursive uses in
>>> get/put_online_cpus_atomic() instead of doing it as a property of your
>>> rwlock:
>>>
>>> get_online_cpus_atomic()
>>> {
>>>     unsigned long flags;
>>>     local_irq_save(flags);
>>>     if (this_cpu_inc_return(hotplug_recusion_count) == 1)
>>>         percpu_read_lock_irqsafe(&hotplug_pcpu_rwlock);
>>>     local_irq_restore(flags);
>>> }
>>>
>>> Once again, the idea there is to avoid baking the reader side
>>> recursive properties into your rwlock itself, so that it won't be
>>> impossible to implement reader/writer fairness into your rwlock in the
>>> future (which may be not be very important for the hotplug use, but
>>> could be when other uses get introduced).
>>
>> Hmm, your proposal above looks good to me, at first glance.
>> (Sorry, I had mistaken your earlier mails to mean that you were against
>> recursive reader-side, while you actually meant that you didn't like
>> implementing the recursive reader-side logic using the recursive property
>> of rwlocks).
> 
> To be honest, I was replying as I went through the series, so I hadn't
> digested your hotplug use case yet :)
> 
> But yes - I don't like having the rwlock itself be recursive, but I do
> understand that you have a legitimate requirement for
> get_online_cpus_atomic() to be recursive. This IMO points to the
> direction I suggested, of explicitly handling the recusion in
> get_online_cpus_atomic() so that the underlying rwlock doesn't have to
> support recursive reader side itself.
> 
> (And this would work for the idea of making writers own the reader
> side as well - you can do it with the hotplug_recursion_count instead
> of with the underlying rwlock).
> 
>> While your idea above looks good, it might introduce more complexity
>> in the unlock path, since this would allow nesting of heterogeneous readers
>> (ie., if hotplug_recursion_count == 1, you don't know whether you need to
>> simply decrement the counter or unlock the rwlock as well).
> 
> Well, I think the idea doesn't make the underlying rwlock more
> complex, since you could in principle keep your existing
> percpu_read_lock_irqsafe implementation as is and just remove the
> recursive behavior from its documentation.
> 
> Now ideally if we're adding a bit of complexity in
> get_online_cpus_atomic() it'd be nice if we could remove some from
> percpu_read_lock_irqsafe, but I haven't thought about that deeply
> either. I think you'd still want to have the equivalent of a percpu
> reader_refcnt, except it could now be a bool instead of an int, and
> percpu_read_lock_irqsafe would still set it to back to 0/false after
> acquiring the global read side if a writer is signaled. Basically your
> existing percpu_read_lock_irqsafe code should still work, and we could
> remove just the parts that were only there to deal with the recursive
> property.
> 

But, the whole intention behind removing the parts depending on the
recursive property of rwlocks would be to make it easier to make rwlocks
fair (going forward) right? Then, that won't work for CPU hotplug, because,
just like we have a legitimate reason to have recursive
get_online_cpus_atomic(), we also have a legitimate reason to have
unfairness in locking (i.e., for deadlock-safety). So we simply can't
afford to make the locking fair - we'll end up in too many deadlock
possibilities, as hinted in the changelog of patch 1.

(Remember, we are replacing preempt_disable(), which had absolutely no
special nesting rules or locking implications. That is why we are forced
to provide maximum locking flexibility and safety against new/previously
non-existent deadlocks, in the new synchronization scheme).

So the only long-term solution I can think of is to decouple
percpu-rwlocks and rwlock_t (like what Tejun suggested) by implementing
our own unfair locking scheme inside. What do you think?

Regards,
Srivatsa S. Bhat

^ permalink raw reply

* Re: [PATCH][UPSTEAM] powerpc/mpic: add irq_set_wake support
From: Kumar Gala @ 2013-02-18 19:43 UTC (permalink / raw)
  To: Wang Dongsheng; +Cc: linuxppc-dev
In-Reply-To: <1359601823-9861-1-git-send-email-dongsheng.wang@freescale.com>


On Jan 30, 2013, at 9:10 PM, Wang Dongsheng wrote:

> Add irq_set_wake support. Just add IRQF_NO_SUSPEND to =
desc->action->flag.
> So the wake up interrupt will not be disable in suspend_device_irqs.
>=20
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> ---
> arch/powerpc/sysdev/mpic.c |   15 +++++++++++++++
> 1 files changed, 15 insertions(+), 0 deletions(-)

Why are we doing this globally for all interrupts?  Don't we only have =
some specific interrupts that wake us up?

Also, I'm guessing the wake behavior for interrupts is FSL specific so =
should not apply to ALL users of MPIC.

- k

>=20
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 9c6e535..2ed0220 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -920,6 +920,18 @@ int mpic_set_irq_type(struct irq_data *d, =
unsigned int flow_type)
> 	return IRQ_SET_MASK_OK_NOCOPY;
> }
>=20
> +static int mpic_irq_set_wake(struct irq_data *d, unsigned int on)
> +{
> +	struct irq_desc *desc =3D container_of(d, struct irq_desc, =
irq_data);
> +
> +	if (on)
> +		desc->action->flags |=3D IRQF_NO_SUSPEND;
> +	else
> +		desc->action->flags &=3D ~IRQF_NO_SUSPEND;
> +
> +	return 0;
> +}
> +
> void mpic_set_vector(unsigned int virq, unsigned int vector)
> {
> 	struct mpic *mpic =3D mpic_from_irq(virq);
> @@ -957,6 +969,7 @@ static struct irq_chip mpic_irq_chip =3D {
> 	.irq_unmask	=3D mpic_unmask_irq,
> 	.irq_eoi	=3D mpic_end_irq,
> 	.irq_set_type	=3D mpic_set_irq_type,
> +	.irq_set_wake	=3D mpic_irq_set_wake,
> };
>=20
> #ifdef CONFIG_SMP
> @@ -971,6 +984,7 @@ static struct irq_chip mpic_tm_chip =3D {
> 	.irq_mask	=3D mpic_mask_tm,
> 	.irq_unmask	=3D mpic_unmask_tm,
> 	.irq_eoi	=3D mpic_end_irq,
> +	.irq_set_wake	=3D mpic_irq_set_wake,
> };
>=20
> #ifdef CONFIG_MPIC_U3_HT_IRQS
> @@ -981,6 +995,7 @@ static struct irq_chip mpic_irq_ht_chip =3D {
> 	.irq_unmask	=3D mpic_unmask_ht_irq,
> 	.irq_eoi	=3D mpic_end_ht_irq,
> 	.irq_set_type	=3D mpic_set_irq_type,
> +	.irq_set_wake	=3D mpic_irq_set_wake,
> };
> #endif /* CONFIG_MPIC_U3_HT_IRQS */
>=20
> --
> 1.7.5.1
>=20
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH v5 00/45] CPU hotplug: stop_machine()-free CPU hotplug
From: Steven Rostedt @ 2013-02-18 19:53 UTC (permalink / raw)
  To: Vincent Guittot
  Cc: linux-doc, peterz, fweisbec, linux-kernel, walken, mingo,
	linux-arch, Russell King - ARM Linux, xiaoguangrong, wangyun,
	paulmck, nikunj, linux-pm, Rusty Russell, rjw, namhyung, tglx,
	linux-arm-kernel, netdev, oleg, sbw, Srivatsa S. Bhat, tj, akpm,
	linuxppc-dev
In-Reply-To: <CAKfTPtC4hbXXWO=_zsGYryM9hpWaGFLbiY+jiLf=koLa4xGJOQ@mail.gmail.com>

On Mon, 2013-02-18 at 17:50 +0100, Vincent Guittot wrote:

> yes for sure.
> The problem is more linked to cpuidle and function tracer.
> 
> cpu hotplug and function tracer work when cpuidle is disable.
> cpu hotplug and cpuidle works if i don't enable function tracer.
> my platform is dead as soon as I enable function tracer if cpuidle is
> enabled. It looks like some notrace are missing in my platform driver
> but we haven't completely fix the issue yet
> 

You can bisect to find out exactly what function is the problem:

 cat /debug/tracing/available_filter_functions > t
 
f(t) {
 num=`wc -l t`
 sed -ne "1,${num}p" t > t1
 let num=num+1
 sed -ne "${num},$p" t > t2

 cat t1 > /debug/tracing/set_ftrace_filter
 # note this may take a long time to finish

 echo function > /debug/tracing/current_tracer

 <failed? bisect f(t1), if not bisect f(t2)>
}

^ permalink raw reply

* Re: [PATCH v5 00/45] CPU hotplug: stop_machine()-free CPU hotplug
From: Steven Rostedt @ 2013-02-18 19:53 UTC (permalink / raw)
  To: Vincent Guittot
  Cc: linux-doc, peterz, fweisbec, linux-kernel, walken, mingo,
	linux-arch, Russell King - ARM Linux, xiaoguangrong, wangyun,
	paulmck, nikunj, linux-pm, Rusty Russell, rjw, namhyung, tglx,
	linux-arm-kernel, netdev, oleg, sbw, Srivatsa S. Bhat, tj, akpm,
	linuxppc-dev
In-Reply-To: <CAKfTPtC4hbXXWO=_zsGYryM9hpWaGFLbiY+jiLf=koLa4xGJOQ@mail.gmail.com>

On Mon, 2013-02-18 at 17:50 +0100, Vincent Guittot wrote:

> yes for sure.
> The problem is more linked to cpuidle and function tracer.
> 
> cpu hotplug and function tracer work when cpuidle is disable.
> cpu hotplug and cpuidle works if i don't enable function tracer.
> my platform is dead as soon as I enable function tracer if cpuidle is
> enabled. It looks like some notrace are missing in my platform driver
> but we haven't completely fix the issue yet
> 

You can bisect to find out exactly what function is the problem:

 cat /debug/tracing/available_filter_functions > t
 
f(t) {
 num=`wc -l t`
 sed -ne "1,${num}p" t > t1
 let num=num+1
 sed -ne "${num},$p" t > t2

 cat t1 > /debug/tracing/set_ftrace_filter
 # note this may take a long time to finish

 echo function > /debug/tracing/current_tracer

 <failed? bisect f(t1), if not bisect f(t2)>
}

-- Steve

^ permalink raw reply

* [PATCH] powerpc/85xx: dts - add ranges property for SEC
From: Po Liu @ 2013-02-19  0:29 UTC (permalink / raw)
  To: linuxppc-dev, Kumar Gala; +Cc: Po Liu

This facilitates getting the physical address of the SEC node.

Signed-off-by: Liu po <po.liu@freescale.com>
---
 arch/powerpc/boot/dts/fsl/pq3-sec4.4-0.dtsi |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/fsl/pq3-sec4.4-0.dtsi b/arch/powerpc/boot/dts/fsl/pq3-sec4.4-0.dtsi
index d4c9d5d..ffadcb5 100644
--- a/arch/powerpc/boot/dts/fsl/pq3-sec4.4-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/pq3-sec4.4-0.dtsi
@@ -36,6 +36,7 @@ crypto@30000 {
 	compatible = "fsl,sec-v4.4", "fsl,sec-v4.0";
 	#address-cells = <1>;
 	#size-cells = <1>;
+	ranges		 = <0x0 0x30000 0x10000>;
 	reg		 = <0x30000 0x10000>;
 	interrupts	 = <58 2 0 0>;
 
-- 
1.6.4

^ permalink raw reply related

* [PATCH] bsc9131/dts: Correct typo in SDHC device node
From: Harninder Rai @ 2013-02-19  9:13 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Harninder Rai, kumar.gala

BSC9131RDB doesn't have SDHC enabled. As a result of this typo,
the node was not getting disabled from the device tree which was
leading to linux hang during bootup

Signed-off-by: Harninder Rai <harninder.rai@freescale.com>
---
 arch/powerpc/boot/dts/bsc9131rdb.dtsi |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/bsc9131rdb.dtsi b/arch/powerpc/boot/dts/bsc9131rdb.dtsi
index 638adda..9e6c013 100644
--- a/arch/powerpc/boot/dts/bsc9131rdb.dtsi
+++ b/arch/powerpc/boot/dts/bsc9131rdb.dtsi
@@ -126,7 +126,7 @@
 		};
 	};
 
-	sdhci@2e000 {
+	sdhc@2e000 {
 		status = "disabled";
 	};
 
-- 
1.7.6.GIT

^ permalink raw reply related

* [PATCH] bsc9131:l2sram: Add compatible string for BSC9131 platform
From: Harninder Rai @ 2013-02-19  9:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Harninder Rai, kumar.gala

Signed-off-by: Harninder Rai <harninder.rai@freescale.com>
---
 arch/powerpc/sysdev/fsl_85xx_l2ctlr.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c b/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c
index 8cf93f0..afc2dbf 100644
--- a/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c
+++ b/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c
@@ -203,6 +203,7 @@ static struct of_device_id mpc85xx_l2ctlr_of_match[] = {
 	{	.compatible = "fsl,p1024-l2-cache-controller",},
 	{	.compatible = "fsl,p1015-l2-cache-controller",},
 	{	.compatible = "fsl,p1010-l2-cache-controller",},
+	{	.compatible = "fsl,bsc9131-l2-cache-controller",},
 	{},
 };
 
-- 
1.7.6.GIT

^ permalink raw reply related

* [PATCH] bsc913x:defconfig: Add new defconfig file for BSC913x platforms
From: Harninder Rai @ 2013-02-19  9:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Harninder Rai, kumar.gala

BSC913x are heterogeneous platforms having DSP and PowerPC.
* Lot of new IPs like AIC (Antenna Interface Controller), RF (radio) etc
* Such IPs are not present in any other 85xx platform
* Lot of optimizations related to ethernet/ASF (Application Specific Fastpath)
  are enabled in this config
* IPC for inter-domain communication (DSP and PA) is present

Signed-off-by: Harninder Rai <harninder.rai@freescale.com>
---
 arch/powerpc/configs/qoriq_sdk_asf_term_defconfig |  209 +++++++++++++++++++++
 1 files changed, 209 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/configs/qoriq_sdk_asf_term_defconfig

diff --git a/arch/powerpc/configs/qoriq_sdk_asf_term_defconfig b/arch/powerpc/configs/qoriq_sdk_asf_term_defconfig
new file mode 100644
index 0000000..6ded6af
--- /dev/null
+++ b/arch/powerpc/configs/qoriq_sdk_asf_term_defconfig
@@ -0,0 +1,209 @@
+CONFIG_PPC_85xx=y
+CONFIG_EXPERIMENTAL=y
+# CONFIG_LOCALVERSION_AUTO is not set
+CONFIG_SYSVIPC=y
+CONFIG_POSIX_MQUEUE=y
+CONFIG_AUDIT=y
+CONFIG_NO_HZ=y
+CONFIG_HIGH_RES_TIMERS=y
+CONFIG_BSD_PROCESS_ACCT=y
+CONFIG_IKCONFIG=y
+CONFIG_IKCONFIG_PROC=y
+CONFIG_LOG_BUF_SHIFT=14
+CONFIG_SYSFS_DEPRECATED=y
+CONFIG_SYSFS_DEPRECATED_V2=y
+CONFIG_BLK_DEV_INITRD=y
+CONFIG_SYSCTL_SYSCALL=y
+CONFIG_EMBEDDED=y
+# CONFIG_PERF_EVENTS is not set
+CONFIG_SLAB=y
+CONFIG_PROFILING=y
+CONFIG_OPROFILE=y
+CONFIG_MODULES=y
+CONFIG_MODULE_UNLOAD=y
+CONFIG_MODULE_FORCE_UNLOAD=y
+# CONFIG_BLK_DEV_BSG is not set
+CONFIG_PARTITION_ADVANCED=y
+CONFIG_MAC_PARTITION=y
+# CONFIG_EFI_PARTITION is not set
+CONFIG_BSC9131_RDB=y
+CONFIG_HIGHMEM=y
+CONFIG_PREEMPT=y
+# CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set
+CONFIG_MATH_EMULATION=y
+CONFIG_SWIOTLB=y
+CONFIG_PCI=y
+CONFIG_PCIEPORTBUS=y
+# CONFIG_PCIEAER is not set
+# CONFIG_PCIEASPM is not set
+CONFIG_PCI_MSI=y
+CONFIG_ADVANCED_OPTIONS=y
+CONFIG_LOWMEM_CAM_NUM_BOOL=y
+CONFIG_LOWMEM_CAM_NUM=6
+CONFIG_NET=y
+CONFIG_PACKET=y
+CONFIG_UNIX=y
+CONFIG_XFRM_USER=y
+CONFIG_NET_KEY=y
+CONFIG_INET=y
+CONFIG_IP_MULTICAST=y
+CONFIG_IP_ADVANCED_ROUTER=y
+CONFIG_IP_MULTIPLE_TABLES=y
+CONFIG_IP_ROUTE_MULTIPATH=y
+CONFIG_IP_ROUTE_VERBOSE=y
+CONFIG_IP_PNP=y
+CONFIG_IP_PNP_DHCP=y
+CONFIG_IP_PNP_BOOTP=y
+CONFIG_IP_PNP_RARP=y
+CONFIG_NET_IPIP=y
+CONFIG_IP_MROUTE=y
+CONFIG_IP_PIMSM_V1=y
+CONFIG_IP_PIMSM_V2=y
+CONFIG_ARPD=y
+CONFIG_INET_AH=y
+CONFIG_INET_ESP=y
+CONFIG_INET_IPCOMP=y
+# CONFIG_INET_LRO is not set
+CONFIG_IPV6=y
+CONFIG_INET6_AH=y
+CONFIG_NETFILTER=y
+CONFIG_NF_CONNTRACK=y
+CONFIG_NF_CONNTRACK_EVENTS=y
+CONFIG_NF_CONNTRACK_FTP=y
+CONFIG_NF_CONNTRACK_TFTP=y
+CONFIG_NETFILTER_XT_MATCH_HL=y
+CONFIG_NF_CONNTRACK_IPV4=y
+CONFIG_IP_NF_IPTABLES=y
+CONFIG_IP_NF_FILTER=y
+CONFIG_IP_NF_TARGET_REJECT=y
+CONFIG_IP_NF_MANGLE=y
+CONFIG_BRIDGE=y
+CONFIG_VLAN_8021Q=y
+CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
+CONFIG_MTD=y
+CONFIG_MTD_CMDLINE_PARTS=y
+CONFIG_MTD_CHAR=y
+CONFIG_MTD_BLOCK=y
+CONFIG_FTL=y
+CONFIG_MTD_CFI=y
+CONFIG_MTD_CFI_INTELEXT=y
+CONFIG_MTD_CFI_AMDSTD=y
+CONFIG_MTD_PHYSMAP_OF=y
+CONFIG_MTD_M25P80=y
+CONFIG_MTD_NAND=y
+CONFIG_MTD_NAND_PLATFORM=y
+CONFIG_MTD_NAND_FSL_ELBC=y
+CONFIG_MTD_NAND_FSL_IFC=y
+CONFIG_MTD_NAND_FSL_UPM=y
+CONFIG_MTD_UBI=y
+CONFIG_PROC_DEVICETREE=y
+CONFIG_BLK_DEV_LOOP=y
+CONFIG_BLK_DEV_RAM=y
+CONFIG_BLK_DEV_RAM_SIZE=131072
+CONFIG_BLK_DEV_SD=y
+CONFIG_CHR_DEV_ST=y
+CONFIG_BLK_DEV_SR=y
+CONFIG_CHR_DEV_SG=y
+CONFIG_SCSI_MULTI_LUN=y
+CONFIG_SCSI_LOGGING=y
+CONFIG_ATA=y
+CONFIG_SATA_AHCI=y
+CONFIG_SATA_FSL=y
+CONFIG_SATA_SIL24=y
+CONFIG_MD=y
+CONFIG_BLK_DEV_MD=y
+# CONFIG_MD_AUTODETECT is not set
+CONFIG_MD_RAID456=y
+CONFIG_NETDEVICES=y
+CONFIG_DUMMY=y
+CONFIG_MII=y
+# CONFIG_NET_VENDOR_3COM is not set
+CONFIG_GIANFAR=y
+CONFIG_E1000E=y
+CONFIG_VITESSE_PHY=y
+CONFIG_FIXED_PHY=y
+CONFIG_PPP=y
+CONFIG_PPPOE=y
+CONFIG_PPP_ASYNC=y
+CONFIG_INPUT_FF_MEMLESS=m
+# CONFIG_INPUT_MOUSEDEV is not set
+# CONFIG_INPUT_KEYBOARD is not set
+# CONFIG_INPUT_MOUSE is not set
+CONFIG_SERIO_LIBPS2=y
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_SERIAL_8250_MANY_PORTS=y
+CONFIG_SERIAL_8250_DETECT_IRQ=y
+CONFIG_SERIAL_8250_RSA=y
+CONFIG_NVRAM=y
+CONFIG_I2C=y
+CONFIG_I2C_CHARDEV=y
+CONFIG_I2C_MPC=y
+CONFIG_SPI=y
+CONFIG_SPI_BITBANG=y
+CONFIG_SPI_FSL_SPI=y
+CONFIG_SPI_FSL_ESPI=y
+CONFIG_GPIOLIB=y
+# CONFIG_HWMON is not set
+CONFIG_WATCHDOG=y
+CONFIG_WATCHDOG_NOWAYOUT=y
+CONFIG_BOOKE_WDT=y
+CONFIG_VIDEO_OUTPUT_CONTROL=y
+CONFIG_USB=y
+CONFIG_USB_EHCI_HCD=y
+# CONFIG_USB_EHCI_TT_NEWSCHED is not set
+CONFIG_USB_EHCI_FSL=y
+CONFIG_USB_STORAGE=y
+CONFIG_MMC=y
+CONFIG_MMC_UNSAFE_RESUME=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_PLTFM=y
+CONFIG_MMC_SDHCI_OF_ESDHC=y
+CONFIG_RTC_CLASS=y
+CONFIG_RTC_DRV_DS1307=y
+CONFIG_RTC_DRV_DS3232=y
+CONFIG_RTC_DRV_CMOS=y
+CONFIG_DMADEVICES=y
+CONFIG_FSL_DMA=y
+# CONFIG_NET_DMA is not set
+CONFIG_ASYNC_TX_DMA=y
+CONFIG_UIO=y
+CONFIG_EXT2_FS=y
+CONFIG_EXT3_FS=y
+# CONFIG_EXT3_DEFAULTS_TO_ORDERED is not set
+CONFIG_XFS_FS=y
+CONFIG_ISO9660_FS=m
+CONFIG_JOLIET=y
+CONFIG_ZISOFS=y
+CONFIG_UDF_FS=m
+CONFIG_MSDOS_FS=m
+CONFIG_VFAT_FS=y
+CONFIG_NTFS_FS=y
+CONFIG_PROC_KCORE=y
+CONFIG_TMPFS=y
+CONFIG_JFFS2_FS=y
+CONFIG_JFFS2_FS_DEBUG=1
+CONFIG_JFFS2_FS_WBUF_VERIFY=y
+CONFIG_JFFS2_SUMMARY=y
+CONFIG_JFFS2_FS_XATTR=y
+CONFIG_JFFS2_COMPRESSION_OPTIONS=y
+CONFIG_JFFS2_LZO=y
+CONFIG_JFFS2_RUBIN=y
+CONFIG_UBIFS_FS=y
+CONFIG_NFS_FS=y
+CONFIG_NFS_V4=y
+CONFIG_ROOT_NFS=y
+CONFIG_NFSD=y
+CONFIG_CIFS=y
+CONFIG_NLS_CODEPAGE_437=y
+CONFIG_NLS_ISO8859_1=y
+CONFIG_CRC_T10DIF=y
+# CONFIG_FTRACE is not set
+CONFIG_CRYPTO_NULL=y
+CONFIG_CRYPTO_CTR=y
+CONFIG_CRYPTO_PCBC=y
+CONFIG_CRYPTO_XCBC=y
+CONFIG_CRYPTO_SHA512=y
+# CONFIG_CRYPTO_ANSI_CPRNG is not set
+CONFIG_CRYPTO_DEV_FSL_CAAM=y
+CONFIG_CRYPTO_DEV_FSL_CAAM_INTC=y
-- 
1.7.6.GIT

^ permalink raw reply related

* Re: [PATCH v6 08/46] CPU hotplug: Provide APIs to prevent CPU offline from atomic context
From: Michel Lespinasse @ 2013-02-19  9:40 UTC (permalink / raw)
  To: Srivatsa S. Bhat
  Cc: linux-doc, peterz, fweisbec, linux-kernel, mingo, linux-arch,
	linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
	rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev, oleg,
	vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <51227810.6090009@linux.vnet.ibm.com>

On Tue, Feb 19, 2013 at 2:50 AM, Srivatsa S. Bhat
<srivatsa.bhat@linux.vnet.ibm.com> wrote:
> But, the whole intention behind removing the parts depending on the
> recursive property of rwlocks would be to make it easier to make rwlocks
> fair (going forward) right? Then, that won't work for CPU hotplug, because,
> just like we have a legitimate reason to have recursive
> get_online_cpus_atomic(), we also have a legitimate reason to have
> unfairness in locking (i.e., for deadlock-safety). So we simply can't
> afford to make the locking fair - we'll end up in too many deadlock
> possibilities, as hinted in the changelog of patch 1.

Grumpf - I hadn't realized that making the underlying rwlock fair
would break your hotplug use case. But you are right, it would. Oh
well :/

> So the only long-term solution I can think of is to decouple
> percpu-rwlocks and rwlock_t (like what Tejun suggested) by implementing
> our own unfair locking scheme inside. What do you think?

I have no idea how hard would it be to change get_online_cpus_atomic()
call sites so that the hotplug rwlock read side has a defined order vs
other locks (thus making sure the situation you describe in patch 1
doesn't happen). I agree we shouldn't base our short term plans around
that, but maybe that's doable in the long term ???

Otherwise, I think we should add some big-fat-warning that percpu
rwlocks don't have reader/writer fairness, that the hotplug use case
actually depends on the unfairness / would break if the rwlock was
made fair, and that any new uses of percpu rwlocks should be very
carefully considered because of the reader/writer fairness issues.
Maybe even give percpu rwlocks a less generic sounding name, given how
constrained they are by the hotplug use case.

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

^ permalink raw reply

* Re: [PATCH v6 08/46] CPU hotplug: Provide APIs to prevent CPU offline from atomic context
From: Srivatsa S. Bhat @ 2013-02-19  9:55 UTC (permalink / raw)
  To: Michel Lespinasse
  Cc: linux-doc, peterz, fweisbec, linux-kernel, mingo, linux-arch,
	linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
	rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev, oleg,
	vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <CANN689E-98zK69pGeojbm30n3sswQYVBvhk3i3OzqvbeqSrSrg@mail.gmail.com>

On 02/19/2013 03:10 PM, Michel Lespinasse wrote:
> On Tue, Feb 19, 2013 at 2:50 AM, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>> But, the whole intention behind removing the parts depending on the
>> recursive property of rwlocks would be to make it easier to make rwlocks
>> fair (going forward) right? Then, that won't work for CPU hotplug, because,
>> just like we have a legitimate reason to have recursive
>> get_online_cpus_atomic(), we also have a legitimate reason to have
>> unfairness in locking (i.e., for deadlock-safety). So we simply can't
>> afford to make the locking fair - we'll end up in too many deadlock
>> possibilities, as hinted in the changelog of patch 1.
> 
> Grumpf - I hadn't realized that making the underlying rwlock fair
> would break your hotplug use case. But you are right, it would. Oh
> well :/
> 

Yeah :-/

>> So the only long-term solution I can think of is to decouple
>> percpu-rwlocks and rwlock_t (like what Tejun suggested) by implementing
>> our own unfair locking scheme inside. What do you think?
> 
> I have no idea how hard would it be to change get_online_cpus_atomic()
> call sites so that the hotplug rwlock read side has a defined order vs
> other locks (thus making sure the situation you describe in patch 1
> doesn't happen). I agree we shouldn't base our short term plans around
> that, but maybe that's doable in the long term ???
>

I think it should be possible in the longer term. I'm expecting it to be
*much much* harder to audit and convert (requiring a lot of subsystem
knowledge of each subsystem that we are touching), than the simpler
tree-wide conversion that I did in this patchset... but I don't think it
is impossible.
 
> Otherwise, I think we should add some big-fat-warning that percpu
> rwlocks don't have reader/writer fairness, that the hotplug use case
> actually depends on the unfairness / would break if the rwlock was
> made fair, and that any new uses of percpu rwlocks should be very
> carefully considered because of the reader/writer fairness issues.

In fact, when I started out, I actually contained all the new locking code
inside CPU hotplug itself, and didn't even expose it as a generic percpu
rwlock in some of the previous versions of this patchset... :-)

But now that we already have a generic locking scheme exposed, we could
add a warning against using it without due consideration.

> Maybe even give percpu rwlocks a less generic sounding name, given how
> constrained they are by the hotplug use case.
 
I wouldn't go that far... ;-) Unfairness is not a show-stopper right?
IMHO, the warning/documentation should suffice for anybody wanting to
try out this locking scheme for other use-cases.

Regards,
Srivatsa S. Bhat

^ permalink raw reply

* Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Diana Craciun @ 2013-02-19 10:04 UTC (permalink / raw)
  To: Varun Sethi
  Cc: joro, stuart.yoder, linux-kernel, iommu, scottwood, linuxppc-dev
In-Reply-To: <1361191939-21260-7-git-send-email-Varun.Sethi@freescale.com>

On 02/18/2013 02:52 PM, Varun Sethi wrote:
> +
> +#define PAACE_TCEF_FORMAT0_8B   0x00
> +#define PAACE_TCEF_FORMAT1_RSVD 0x01
> +
> +#define PAACE_NUMBER_ENTRIES    0x1FF

Where is this number coming from?

Diana

^ permalink raw reply

* RE: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Sethi Varun-B16395 @ 2013-02-19 10:27 UTC (permalink / raw)
  To: Craciun Diana Madalina-STFD002
  Cc: Wood Scott-B07421, joro@8bytes.org, linux-kernel@vger.kernel.org,
	Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <51234E27.9020604@freescale.com>



> -----Original Message-----
> From: Craciun Diana Madalina-STFD002
> Sent: Tuesday, February 19, 2013 3:34 PM
> To: Sethi Varun-B16395
> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org; Wood Scott-B07421; joro@8bytes.org; Yoder
> Stuart-B08248
> Subject: Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU
> API implementation.
>=20
> On 02/18/2013 02:52 PM, Varun Sethi wrote:
> > +
> > +#define PAACE_TCEF_FORMAT0_8B   0x00
> > +#define PAACE_TCEF_FORMAT1_RSVD 0x01
> > +
> > +#define PAACE_NUMBER_ENTRIES    0x1FF
>=20
> Where is this number coming from?
>=20
This is currently hard coded. We will not require these many entries once w=
e implement the LIODN allocation scheme.

-Varun

^ permalink raw reply

* Re: [PATCH v5 00/45] CPU hotplug: stop_machine()-free CPU hotplug
From: Vincent Guittot @ 2013-02-19 10:33 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-doc, peterz, fweisbec, linux-kernel, walken, mingo,
	linux-arch, Russell King - ARM Linux, xiaoguangrong, wangyun,
	paulmck, nikunj, linux-pm, Rusty Russell, rjw, namhyung, tglx,
	linux-arm-kernel, netdev, oleg, sbw, Srivatsa S. Bhat, tj, akpm,
	linuxppc-dev
In-Reply-To: <1361217204.23152.171.camel@gandalf.local.home>

On 18 February 2013 20:53, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 2013-02-18 at 17:50 +0100, Vincent Guittot wrote:
>
>> yes for sure.
>> The problem is more linked to cpuidle and function tracer.
>>
>> cpu hotplug and function tracer work when cpuidle is disable.
>> cpu hotplug and cpuidle works if i don't enable function tracer.
>> my platform is dead as soon as I enable function tracer if cpuidle is
>> enabled. It looks like some notrace are missing in my platform driver
>> but we haven't completely fix the issue yet
>>
>
> You can bisect to find out exactly what function is the problem:
>
>  cat /debug/tracing/available_filter_functions > t
>
> f(t) {
>  num=`wc -l t`
>  sed -ne "1,${num}p" t > t1
>  let num=num+1
>  sed -ne "${num},$p" t > t2
>
>  cat t1 > /debug/tracing/set_ftrace_filter
>  # note this may take a long time to finish
>
>  echo function > /debug/tracing/current_tracer
>
>  <failed? bisect f(t1), if not bisect f(t2)>
> }
>

Thanks, i'm going to have a look

Vincent

> -- Steve
>
>
>

^ permalink raw reply

* RE: [PATCH v6 08/46] CPU hotplug: Provide APIs to prevent CPU offline from atomic context
From: David Laight @ 2013-02-19 10:42 UTC (permalink / raw)
  To: Srivatsa S. Bhat, Michel Lespinasse
  Cc: linux-doc, peterz, fweisbec, linux-kernel, mingo, linux-arch,
	linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
	rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev, oleg,
	vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <51234C23.2030909@linux.vnet.ibm.com>

> I wouldn't go that far... ;-) Unfairness is not a show-stopper right?
> IMHO, the warning/documentation should suffice for anybody wanting to
> try out this locking scheme for other use-cases.

I presume that by 'fairness' you mean 'write preference'?

I'd not sure how difficult it would be, but maybe have two functions
for acquiring the lock for read, one blocks if there is a writer
waiting, the other doesn't.

That way you can change the individual call sites separately.

The other place I can imagine a per-cpu rwlock being used
is to allow a driver to disable 'sleep' or software controlled
hardware removal while it performs a sequence of operations.

	David

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox