Re: [PATCH 1/1] hotplug cpu: migrate a task within its cpuset
Hello-
Cliff Wickman wrote:
This patch would insert a preference to migrate such a task to a cpu within
its cpuset (and set its cpus_allowed to its cpuset).
With this patch, migrate the task to:
1) to any cpu on the same node as the disabled cpu, which is both online
and among that task's cpus_allowed
2) to any online cpu within the task's cpuset
3) to any cpu which is both online and among that task's cpus_allowed
I think I disagree with this change.
The kernel shouldn't have to be any smarter than it already is about
moving tasks off an offlined cpu. The only way case 2) can be reached
is if the user has changed a task's cpu affinity. If the user is
sophisticated enough to manipulate tasks' cpu affinity then they can
arrange to migrate tasks as they see fit before offlining a cpu.
Furthermore:
--- morton.070123.orig/kernel/sched.c
+++ morton.070123/kernel/sched.c
@@ -5170,6 +5170,12 @@ restart:
if (dest_cpu == NR_CPUS)
dest_cpu = any_online_cpu(p-cpus_allowed);
+ /* try to stay on the same cpuset */
+ if (dest_cpu == NR_CPUS) {
+ p-cpus_allowed = cpuset_cpus_allowed(p);
+ dest_cpu = any_online_cpu(p-cpus_allowed);
+ }
It's not okay to call cpuset_cpus_allowed in this context -- local
irqs are supposed to have been disabled by the caller of
move_task_off_dead_cpu and cpuset_cpus_allowed acquires a mutex.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] hotplug cpu: migrate a task within its cpuset
Hi-
Ingo Molnar wrote:
* Cliff Wickman [EMAIL PROTECTED] wrote:
With this patch, migrate the task to:
1) to any cpu on the same node as the disabled cpu, which is both online
and among that task's cpus_allowed
2) to any online cpu within the task's cpuset
3) to any cpu which is both online and among that task's cpus_allowed
Diffed against 2.6.21-rc3 (Andrew's current top of tree)
looks good to me.
Acked-by: Ingo Molnar [EMAIL PROTECTED]
+ /* try to stay on the same cpuset */
+ if (dest_cpu == NR_CPUS) {
+ p-cpus_allowed = cpuset_cpus_allowed(p);
+ dest_cpu = any_online_cpu(p-cpus_allowed);
+ }
what's the practical effect of this - when moving the last CPU offline
from a node you got jobs migrated to really alien nodes? Thus i think we
should queue this up for v2.6.21 too, correct? It's a NOP on systems
that do not set up cpusets, so it's low-risk.
See my earlier reply to this patch. Calling cpuset_cpus_allowed
(which takes a mutex) here is a bug, since move_task_off_dead_cpu must
be called with interrupts disabled.
btw., unrelated to your patch, there's this bit right after the code
above:
/* No more Mr. Nice Guy. */
if (dest_cpu == NR_CPUS) {
rq = task_rq_lock(p, flags);
cpus_setall(p-cpus_allowed);
dest_cpu = any_online_cpu(p-cpus_allowed);
out of consistency, shouldnt the cpus_setall() rather be:
p-cpus_allowed = cpu_possible_map;
? It shouldnt make any real difference but it looks more consistent.
The default value of cpus_allowed is CPU_MASK_ALL, I thought -- at
least that's what we set init's to early on. Though, as you say, it
shouldn't make any difference.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] hotplug cpu: migrate a task within its cpuset
Robin Holt wrote: On Fri, Mar 09, 2007 at 05:58:59PM -0600, Nathan Lynch wrote: Hello- Cliff Wickman wrote: This patch would insert a preference to migrate such a task to a cpu within its cpuset (and set its cpus_allowed to its cpuset). With this patch, migrate the task to: 1) to any cpu on the same node as the disabled cpu, which is both online and among that task's cpus_allowed 2) to any online cpu within the task's cpuset 3) to any cpu which is both online and among that task's cpus_allowed I think I disagree with this change. The kernel shouldn't have to be any smarter than it already is about moving tasks off an offlined cpu. The only way case 2) can be reached is if the user has changed a task's cpu affinity. If the user is sophisticated enough to manipulate tasks' cpu affinity then they can arrange to migrate tasks as they see fit before offlining a cpu. You are assuming some sort of interlock between the admin and the user. Oh, the horror! ;-) While this may be true on your own personal desktop, I don't think you can expect this to be true on a development machine shared by hundreds of users and admin'd by a group of people. Actually, I would hope that a large development environment where users are given fine-grained control over their resource usage would 1) allow interested tasks to register for notification before a resource is taken away, and 2) that this system of registration and notification is implemented in userspace. Use d-bus or something. Additionally, ia64 is gaining support for offlining a cpu which is giving cache errors. Okay. Consider the case where a task's cpuset consists only of threads or cores that share cache. Keeping the task in its cpuset when offlining due to cache errors is exactly the wrong thing to do. Anyway, it looks to me like task-cpus_allowed should already reflect task's cpuset (cpuset.c:attach_task calls sched.c:set_cpus_allowed), so now I'm missing the point of the patch. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/8] Use process freezer for cpu-hotplug
Gautham R Shenoy wrote:
This patch implements process_freezer based cpu-hotplug
core.
The sailent features are:
o No more (un)lock_cpu_hotplug.
o No more CPU_LOCK_ACQUIRE and CPU_LOCK_RELEASE. Hence no per-subsystem
hotcpu mutexes.
o Calls freeze_process/thaw_processes at the beginning/end of
the hotplug operation.
...
@@ -133,7 +111,11 @@ static int _cpu_down(unsigned int cpu)
if (!cpu_online(cpu))
return -EINVAL;
- raw_notifier_call_chain(cpu_chain, CPU_LOCK_ACQUIRE, hcpu);
+ if (freeze_processes(FE_HOTPLUG_CPU)) {
+ thaw_processes(FE_HOTPLUG_CPU);
+ return -EBUSY;
+ }
+
If I'm understanding correctly, this will cause
# echo 0 /sys/devices/system/cpu/cpuX/online
to sometimes fail, and userspace is expected to try again? This will
break existing applications.
Perhaps drivers/base/cpu.c:store_online should retry as long as
cpu_up/down return -EBUSY. That would avoid a userspace-visible
interface change.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/8] Use process freezer for cpu-hotplug
Ingo Molnar wrote:
* Nathan Lynch [EMAIL PROTECTED] wrote:
- raw_notifier_call_chain(cpu_chain, CPU_LOCK_ACQUIRE, hcpu);
+ if (freeze_processes(FE_HOTPLUG_CPU)) {
+ thaw_processes(FE_HOTPLUG_CPU);
+ return -EBUSY;
+ }
+
If I'm understanding correctly, this will cause
# echo 0 /sys/devices/system/cpu/cpuX/online
to sometimes fail, and userspace is expected to try again? This will
break existing applications.
Perhaps drivers/base/cpu.c:store_online should retry as long as
cpu_up/down return -EBUSY. That would avoid a userspace-visible
interface change.
yeah. I'd even suggest a freeze_processes_nofail() API instead, that
does this internally, without burdening the callsites. (and once the
freezer becomes complete then freeze_processes_nofail() ==
freeze_processes())
Yeah, I just realized that an implementation of my proposal would busy
loop in the kernel forever if a silly admin tried to offline the last
cpu (we're already using -EBUSY for that case), so
freeze_processes_nofail is a better idea :-)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: getting processor numbers
Ulrich Drepper wrote: Siddha, Suresh B wrote:a Not all of the cpu* directories in /sys/devices/system/cpu may be online. Apparently this information isn't needed. It's very easy to verify: $ ls /sys/devices/system/cpu/*/online /sys/devices/system/cpu/cpu1/online /sys/devices/system/cpu/cpu2/online /sys/devices/system/cpu/cpu3/online This is a quad core machine and cpu0 doesn't have the 'online' file (2.6.19 kernel). So, if nobody noticed this it's not needed and we can just remove CPUs from /sys/devices/system/cpu when they are brought offline, right? No... the online sysfs files are used to show and change cpus' online/offline state. You wouldn't be able to bring an offlined cpu back online again. cpu0 doesn't have an online file on machines which don't support offlining of the boot cpu. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] prom_find_machine_type typo breaks pSeries lpar boot
Hi- B. N. Poornima wrote: On Fri, 2005-06-03 at 14:25 -0500, Nathan Lynch wrote: Typo in prom_find_machine_type from Ben's recent patch ppc64: Fix result code handling in prom_init prevents pSeries LPAR systems from booting. Hello, Same typo has creped in again from 2.6.17 kernel onwards and causing open firmware exception in pSeries systems. Looking at the current code, I don't see how this analysis could be correct. The patch that is provided solves the problem. Which patch? Did you forget to include it? It would be best if you sent your problem report and any patches to [EMAIL PROTECTED] linuxppc64-dev is dead. Thanks. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fake NUMA emulation for PowerPC
Hi Balbir- Balbir Singh wrote: Here's a dumb simple implementation of fake NUMA nodes for PowerPC. Fake NUMA nodes can be specified using the following command line option numa=fake=node range node range is of the format range1,range2,...rangeN Each of the rangeX parameters is passed using memparse(). I find the patch useful for fake NUMA emulation on my simple PowerPC machine. I've tested it on a non-numa box with the following arguments numa=fake=1G numa=fake=1G,2G name=fake=1G,512M,2G numa=fake=1500M,2800M mem=3500M numa=fake=1G mem=512M numa=fake=1G mem=1G So this doesn't appear to allow one to assign cpus to fake nodes? Do all cpus just get assigned to node 0 with numa=fake? A different approach that occurs to me is to use kexec with a doctored device tree (i.e. with the ibm,associativity properties modified to reflect your desired topology). Perhaps a little bit obscure, but it seems more flexible. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fix bloat-o-meter for ppc64
bloat-o-meter assumes that a '.' anywhere in a symbol's name means that it is static and prepends 'static.' to the first part of the symbol name, discarding the portion of the name that follows the '.'. However, the names of function entry points begin with '.' in the ppc64 ABI. This causes all function text size changes to be accounted to a single 'static.' entry in the output when comparing ppc64 kernels. Change getsizes() to ignore the first character of the symbol name when searching for '.'. Signed-off-by: Nathan Lynch [EMAIL PROTECTED] --- scripts/bloat-o-meter |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/scripts/bloat-o-meter b/scripts/bloat-o-meter index ce59fc2..6501a50 100755 --- a/scripts/bloat-o-meter +++ b/scripts/bloat-o-meter @@ -18,7 +18,8 @@ def getsizes(file): for l in os.popen(nm --size-sort + file).readlines(): size, type, name = l[:-1].split() if type in tTdDbB: -if . in name: name = static. + name.split(.)[0] +# function names begin with '.' on 64-bit powerpc +if . in name[1:]: name = static. + name.split(.)[0] sym[name] = sym.get(name, 0) + int(size, 16) return sym -- 1.5.3.7.1112.g9758e -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 2/4] Rename lock_cpu_hotplug to get_online_cpus
Gautham R Shenoy wrote:
Hi Nathan,
Hi Gautham-
Gautham R Shenoy wrote:
Replace all lock_cpu_hotplug/unlock_cpu_hotplug from the kernel and use
get_online_cpus and put_online_cpus instead as it highlights
the refcount semantics in these operations.
Something other than get_online_cpus, please? lock_cpu_hotplug()
protects cpu_present_map as well as cpu_online_map. For example, some
of the powerpc code modified in this patch is made a bit less clear
because it is manipulating cpu_present_map, not cpu_online_map.
A quick look at the code, and I am wondering why is lock_cpu_hotplug()
used there in the first place. It doesn't look like we require any
protection against cpus coming up/ going down in the code below,
since the cpu-hotplug operation doesn't affect the cpu_present_map.
The locking is necessary. Changes to cpu_online_map and
cpu_present_map must be serialized; otherwise you could end up trying
to online a cpu as it is being removed (i.e. cleared from
cpu_present_map). Online operations must check that a cpu is present
before bringing it up (kernel/cpu.c):
/* Requires cpu_add_remove_lock to be held */
static int __cpuinit _cpu_up(unsigned int cpu, int tasks_frozen)
{
int ret, nr_calls = 0;
void *hcpu = (void *)(long)cpu;
unsigned long mod = tasks_frozen ? CPU_TASKS_FROZEN : 0;
if (cpu_online(cpu) || !cpu_present(cpu))
return -EINVAL;
Can't we use another mutex here instead of the cpu_hotplug mutex here ?
I guess so, but I don't really see the need...
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 2/4] Rename lock_cpu_hotplug to get_online_cpus
Gautham R Shenoy wrote: On Thu, Oct 18, 2007 at 03:22:21AM -0500, Nathan Lynch wrote: Gautham R Shenoy wrote: Hi Nathan, Hi Gautham- Gautham R Shenoy wrote: Replace all lock_cpu_hotplug/unlock_cpu_hotplug from the kernel and use get_online_cpus and put_online_cpus instead as it highlights the refcount semantics in these operations. Something other than get_online_cpus, please? lock_cpu_hotplug() protects cpu_present_map as well as cpu_online_map. For example, some of the powerpc code modified in this patch is made a bit less clear because it is manipulating cpu_present_map, not cpu_online_map. A quick look at the code, and I am wondering why is lock_cpu_hotplug() used there in the first place. It doesn't look like we require any protection against cpus coming up/ going down in the code below, since the cpu-hotplug operation doesn't affect the cpu_present_map. The locking is necessary. Changes to cpu_online_map and cpu_present_map must be serialized; otherwise you could end up trying to online a cpu as it is being removed (i.e. cleared from cpu_present_map). Online operations must check that a cpu is present before bringing it up (kernel/cpu.c): Fair enough! But we are not protecting the cpu_present_map here using lock_cpu_hotplug(), now are we? Yes, we are. In addition to the above, updates to cpu_present_map have to be serialized. pseries_add_processor can be summed up as find the first N unset bits in cpu_present_map and set them. That's not an atomic operation, so some kind of mutual exclusion is needed. The lock_cpu_hotplug() here, ensures that no cpu-hotplug operations occur in parallel with a processor add or a processor remove. That's one important effect, but not the only one (see above). IOW, we're still ensuring that the cpu_online_map doesn't change while we're changing the cpu_present_map. So I don't see why the name get_online_cpus() should be a problem here. The naming is a problem IMO for two reasons: - lock_cpu_hotplug() protects cpu_present_map as well as cpu_online_map (sigh, I see that Documentation/cpu-hotplug.txt disagrees with me, but my statement holds for powerpc, at least). - get_online_cpus() implies reference count semantics (as stated in the changelog) but AFAICT it really has a reference count implementation with read-write locking semantics. Hmm, I think there's another problem here. With your changes, code which relies on the mutual exclusion behavior of lock_cpu_hotplug() (such as pseries_add/remove_processor) will now be able to run concurrently. Probably those functions should use cpu_hotplug_begin/end instead. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 2/4] Rename lock_cpu_hotplug to get_online_cpus
Gautham R Shenoy wrote: Gautham R Shenoy wrote: On Thu, Oct 18, 2007 at 03:22:21AM -0500, Nathan Lynch wrote: Gautham R Shenoy wrote: Gautham R Shenoy wrote: Replace all lock_cpu_hotplug/unlock_cpu_hotplug from the kernel and use get_online_cpus and put_online_cpus instead as it highlights the refcount semantics in these operations. Something other than get_online_cpus, please? lock_cpu_hotplug() protects cpu_present_map as well as cpu_online_map. For example, some of the powerpc code modified in this patch is made a bit less clear because it is manipulating cpu_present_map, not cpu_online_map. A quick look at the code, and I am wondering why is lock_cpu_hotplug() used there in the first place. It doesn't look like we require any protection against cpus coming up/ going down in the code below, since the cpu-hotplug operation doesn't affect the cpu_present_map. The locking is necessary. Changes to cpu_online_map and cpu_present_map must be serialized; otherwise you could end up trying to online a cpu as it is being removed (i.e. cleared from cpu_present_map). Online operations must check that a cpu is present before bringing it up (kernel/cpu.c): Fair enough! But we are not protecting the cpu_present_map here using lock_cpu_hotplug(), now are we? Yes, we are. In addition to the above, updates to cpu_present_map have to be serialized. pseries_add_processor can be summed up as find the first N unset bits in cpu_present_map and set them. That's not an atomic operation, so some kind of mutual exclusion is needed. Okay. But other than pseries_add_processor and pseries_remove_processor, are there any other places where we _change_ the cpu_present_map ? Other arch code e.g. ia64 changes it for add/remove also. But I fail to see how it matters. I agree that we need some kind of synchronization for threads which read the cpu_present_map. But probably we can use a seperate mutex for that. That would be needless complexity. The naming is a problem IMO for two reasons: - lock_cpu_hotplug() protects cpu_present_map as well as cpu_online_map (sigh, I see that Documentation/cpu-hotplug.txt disagrees with me, but my statement holds for powerpc, at least). - get_online_cpus() implies reference count semantics (as stated in the changelog) but AFAICT it really has a reference count implementation with read-write locking semantics. Hmm, I think there's another problem here. With your changes, code which relies on the mutual exclusion behavior of lock_cpu_hotplug() (such as pseries_add/remove_processor) will now be able to run concurrently. Probably those functions should use cpu_hotplug_begin/end instead. One of the primary reasons to move away from the mutex semantics for cpu-hotplug protection, was that there were a lot of places where lock_cpu_hotplug() was used for protecting local data structures too, when they had nothing to do with cpu-hotplug as such, and it resulted in a whole mess. It took people quite sometime to sort things out with the cpufreq subsystem. cpu_present_map isn't a local data structure any more than cpu_online_map, and it is quite relevant to cpu hotplug. We have to maintain the invariant that the set of cpus online is a subset of cpus present. Probably it would be a lot cleaner if we use get_online_cpus() for protection against cpu-hotplug and use specific mutexes for serializing accesses to local data structures. Thoughts? I don't feel like I'm getting through here. Let me restate. If I'm reading them correctly, these patches are changing the behavior of lock_cpu_hotplug() from mutex-style locking to a kind of read-write locking. I think that's fine, but the naming of the new API poorly reflects its real behavior. Conversion of lock_cpu_hotplug() users should be done with care. Most of them - those that need one of the cpu maps to remain unchanged during a critical section - can be considered readers. But a few (such as pseries_add_processor() and pseries_remove_processor()) are writers, because they modify one of the maps. So, why not: get_cpus_online - cpumaps_read_lock put_cpus_online - cpumaps_read_unlock cpu_hotplug_begin - cpumaps_write_lock cpu_hotplug_end - cpumaps_write_unlock Or something similar? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ledtrig-cpu: kill useless mutex to fix sleep in atomic context
Seeing the following every time the CPU enters or leaves idle on a
Beagleboard:
BUG: sleeping function called from invalid context at kernel/mutex.c:269
in_atomic(): 1, irqs_disabled(): 0, pid: 0, name: swapper/0
no locks held by swapper/0/0.
[c001659c] (unwind_backtrace+0x0/0xf8) from [c05aaa7c]
(mutex_lock_nested+0x24/0x380)
[c05aaa7c] (mutex_lock_nested+0x24/0x380) from [c043bd1c]
(ledtrig_cpu+0x38/0x88)
[c043bd1c] (ledtrig_cpu+0x38/0x88) from [c000f4b0] (cpu_idle+0xf4/0x120)
[c000f4b0] (cpu_idle+0xf4/0x120) from [c07e47c8] (start_kernel+0x2bc/0x30c)
Miles Lane has reported seeing similar splats during system suspend.
The mutex in struct led_trigger_cpu appears to have no function: it
resides in a per-cpu data structure which never changes after the
trigger is registered. So just remove it.
Reported-by: Miles Lane miles.l...@gmail.com
Signed-off-by: Nathan Lynch n...@pobox.com
---
drivers/leds/ledtrig-cpu.c | 21 -
1 file changed, 21 deletions(-)
diff --git a/drivers/leds/ledtrig-cpu.c b/drivers/leds/ledtrig-cpu.c
index b312056..4239b39 100644
--- a/drivers/leds/ledtrig-cpu.c
+++ b/drivers/leds/ledtrig-cpu.c
@@ -33,8 +33,6 @@
struct led_trigger_cpu {
char name[MAX_NAME_LEN];
struct led_trigger *_trig;
- struct mutex lock;
- int lock_is_inited;
};
static DEFINE_PER_CPU(struct led_trigger_cpu, cpu_trig);
@@ -50,12 +48,6 @@ void ledtrig_cpu(enum cpu_led_event ledevt)
{
struct led_trigger_cpu *trig = __get_cpu_var(cpu_trig);
- /* mutex lock should be initialized before calling mutex_call() */
- if (!trig-lock_is_inited)
- return;
-
- mutex_lock(trig-lock);
-
/* Locate the correct CPU LED */
switch (ledevt) {
case CPU_LED_IDLE_END:
@@ -75,8 +67,6 @@ void ledtrig_cpu(enum cpu_led_event ledevt)
/* Will leave the LED as it is */
break;
}
-
- mutex_unlock(trig-lock);
}
EXPORT_SYMBOL(ledtrig_cpu);
@@ -117,14 +107,9 @@ static int __init ledtrig_cpu_init(void)
for_each_possible_cpu(cpu) {
struct led_trigger_cpu *trig = per_cpu(cpu_trig, cpu);
- mutex_init(trig-lock);
-
snprintf(trig-name, MAX_NAME_LEN, cpu%d, cpu);
- mutex_lock(trig-lock);
led_trigger_register_simple(trig-name, trig-_trig);
- trig-lock_is_inited = 1;
- mutex_unlock(trig-lock);
}
register_syscore_ops(ledtrig_cpu_syscore_ops);
@@ -142,15 +127,9 @@ static void __exit ledtrig_cpu_exit(void)
for_each_possible_cpu(cpu) {
struct led_trigger_cpu *trig = per_cpu(cpu_trig, cpu);
- mutex_lock(trig-lock);
-
led_trigger_unregister_simple(trig-_trig);
trig-_trig = NULL;
memset(trig-name, 0, MAX_NAME_LEN);
- trig-lock_is_inited = 0;
-
- mutex_unlock(trig-lock);
- mutex_destroy(trig-lock);
}
unregister_syscore_ops(ledtrig_cpu_syscore_ops);
--
1.7.11.7
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ledtrig-cpu: use spin_lock to replace mutex lock
Hi Bryan,
On Thu, 2012-10-18 at 11:18 -0700, Bryan Wu wrote:
@@ -117,14 +117,14 @@ static int __init ledtrig_cpu_init(void)
for_each_possible_cpu(cpu) {
struct led_trigger_cpu *trig = per_cpu(cpu_trig, cpu);
- mutex_init(trig-lock);
+ spin_lock_init(trig-lock);
snprintf(trig-name, MAX_NAME_LEN, cpu%d, cpu);
- mutex_lock(trig-lock);
+ spin_lock(trig-lock);
led_trigger_register_simple(trig-name, trig-_trig);
trig-lock_is_inited = 1;
- mutex_unlock(trig-lock);
+ spin_unlock(trig-lock);
I wouldn't know how to fix the original problem, but I don't think this
patch is okay -- led_trigger_register_simple() does things that
potentially sleep (GFP_KERNEL allocation, down_write), so it's not safe
to call while holding a spinlock.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/12] IB/ehca: Replace get_paca()-paca_index by the more portable smp_processor_id()
Hi, Joachim Fenkes wrote: Signed-off-by: Joachim Fenkes [EMAIL PROTECTED] --- drivers/infiniband/hw/ehca/ehca_tools.h | 14 +++--- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/infiniband/hw/ehca/ehca_tools.h b/drivers/infiniband/hw/ehca/ehca_tools.h index f9b264b..863f972 100644 --- a/drivers/infiniband/hw/ehca/ehca_tools.h +++ b/drivers/infiniband/hw/ehca/ehca_tools.h @@ -73,37 +73,37 @@ extern int ehca_debug_level; if (unlikely(ehca_debug_level)) \ dev_printk(KERN_DEBUG, (ib_dev)-dma_device, \ PU%04x EHCA_DBG:%s format \n, \ -get_paca()-paca_index, __FUNCTION__, \ +smp_processor_id(), __FUNCTION__, \ ## arg); \ } while (0) #define ehca_info(ib_dev, format, arg...) \ dev_info((ib_dev)-dma_device, PU%04x EHCA_INFO:%s format \n, \ - get_paca()-paca_index, __FUNCTION__, ## arg) + smp_processor_id(), __FUNCTION__, ## arg) #define ehca_warn(ib_dev, format, arg...) \ dev_warn((ib_dev)-dma_device, PU%04x EHCA_WARN:%s format \n, \ - get_paca()-paca_index, __FUNCTION__, ## arg) + smp_processor_id(), __FUNCTION__, ## arg) #define ehca_err(ib_dev, format, arg...) \ dev_err((ib_dev)-dma_device, PU%04x EHCA_ERR:%s format \n, \ - get_paca()-paca_index, __FUNCTION__, ## arg) + smp_processor_id(), __FUNCTION__, ## arg) I think I see these macros used in preemptible code (e.g. ehca_probe), where smp_processor_id() will print a warning when CONFIG_DEBUG_PREEMPT=y. Probably better to use raw_smp_processor_id. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.23] ibmebus: Prevent bus_id collisions
Hi- Joachim Fenkes wrote: Previously, ibmebus derived a device's bus_id from its location code. The location code is not guaranteed to be unique, so we might get bus_id collisions if two devices share the same location code. The OFDT full_name, however, is unique, so we use that instead. This is a userspace-visible change, but I guess it's unavoidable. Will anything break? Also, I dislike this approach of duplicating the firmware device tree path in sysfs. Are GX/ibmebus devices guaranteed to be children of the same node in the OF device tree? If so, their unit addresses will be unique, and therefore suitable values for bus_id. I believe this is what the powerpc vio bus code does. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.23] ibmebus: Prevent bus_id collisions
Hi Joachim- Joachim Fenkes wrote: Nathan Lynch [EMAIL PROTECTED] wrote on 29.08.2007 20:12:32: Will anything break? Nope. Userspace programs should not depend on ibmebus' way of naming the devices; especially since some overly long loc_codes tended to be truncated and thus rendered useless. I have tested IBM's DLPAR tools against the changed kernel, and they didn't break. Okay. Also, I dislike this approach of duplicating the firmware device tree path in sysfs. Why? Any specific reasons for your dislike? struct device's bus_id field is but 20 bytes in size. Too close for comfort? Are GX/ibmebus devices guaranteed to be children of the same node in the OF device tree? If so, their unit addresses will be unique, and therefore suitable values for bus_id. I believe this is what the powerpc vio bus code does. While there's no such guarantee (as in officially signed document), yes, I expect future GX devices to also appear beneath the OFDT root node. For the existing devices, the unit addresses are already part of the device name, so I save the need to use sprintf() again. Plus, I rather like using the full_name since it also contains a descriptive name as opposed to being just nondescript numbers, helping the layman (ie user) to make sense out of a dev_id. Okay, but your layman isn't supposed to be relying on any user-friendly properties of the name :) Hope he doesn't work on a distro installer. Anyway, if you're still confident in this approach, I relent. :) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 2/4] Rename lock_cpu_hotplug to get_online_cpus
Hi Gautham-
Gautham R Shenoy wrote:
Replace all lock_cpu_hotplug/unlock_cpu_hotplug from the kernel and use
get_online_cpus and put_online_cpus instead as it highlights
the refcount semantics in these operations.
Something other than get_online_cpus, please? lock_cpu_hotplug()
protects cpu_present_map as well as cpu_online_map. For example, some
of the powerpc code modified in this patch is made a bit less clear
because it is manipulating cpu_present_map, not cpu_online_map.
Index: linux-2.6.23/arch/powerpc/platforms/pseries/hotplug-cpu.c
===
--- linux-2.6.23.orig/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ linux-2.6.23/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -151,7 +151,7 @@ static int pseries_add_processor(struct
for (i = 0; i nthreads; i++)
cpu_set(i, tmp);
- lock_cpu_hotplug();
+ get_online_cpus();
BUG_ON(!cpus_subset(cpu_present_map, cpu_possible_map));
@@ -188,7 +188,7 @@ static int pseries_add_processor(struct
}
err = 0;
out_unlock:
- unlock_cpu_hotplug();
+ put_online_cpus();
return err;
}
@@ -209,7 +209,7 @@ static void pseries_remove_processor(str
nthreads = len / sizeof(u32);
- lock_cpu_hotplug();
+ get_online_cpus();
for (i = 0; i nthreads; i++) {
for_each_present_cpu(cpu) {
if (get_hard_smp_processor_id(cpu) != intserv[i])
@@ -223,7 +223,7 @@ static void pseries_remove_processor(str
printk(KERN_WARNING Could not find cpu to remove
with physical id 0x%x\n, intserv[i]);
}
- unlock_cpu_hotplug();
+ put_online_cpus();
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ACPI] Re: [RFC 5/6]clean cpu state after hotremove CPU
On Tue, Apr 05, 2005 at 09:55:06AM +0800, Li Shaohua wrote: On Mon, 2005-04-04 at 23:33, Nathan Lynch wrote: No. It should make zero difference to the scheduler whether the play dead cpu hotplug or physical hotplug is being used. Keeping some fields like 'cpu_load' are meanless for a hotadded CPU to me. Just ignore them? Reinitializing such things during the CPU_UP_PREPARE case in migration_call should be sufficient, if it's not done already. Nathan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] generate hotplug events for cpu online
We already do kobject_hotplug for cpu offline; this adds a kobject_hotplug call for the online case. This is being requested by developers of an application which wants to be notified about both kinds of events. Signed-off-by: Nathan Lynch [EMAIL PROTECTED] Index: linux-2.6.12-rc2-mm3/drivers/base/cpu.c === --- linux-2.6.12-rc2-mm3.orig/drivers/base/cpu.c2005-03-02 01:37:53.0 -0600 +++ linux-2.6.12-rc2-mm3/drivers/base/cpu.c 2005-04-14 10:56:29.0 -0500 @@ -37,6 +37,8 @@ static ssize_t store_online(struct sys_d break; case '1': ret = cpu_up(cpu-sysdev.id); + if (!ret) + kobject_hotplug(dev-kobj, KOBJ_ONLINE); break; default: ret = -EINVAL; - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
2.6-bk: cpu hotplug + preempt = smp_processor_id warnings galore
Hi- With 2.6.11-rc3-bk7 on ppc64 I am seeing lots of smp_processor_id warnings whenever I hotplug cpus: # echo 0 /sys/devices/system/cpu/cpu1/online cpu 1 (hwid 1) Ready to die... BUG: using smp_processor_id() in preemptible [0001] code: ksoftirqd/1/5 caller is .ksoftirqd+0xbc/0x1f8 Call Trace: [c000fffbbce0] [] 0x (unreliable) [c000fffbbd60] [c01c9f1c] .smp_processor_id+0x154/0x168 [c000fffbbe20] [c005f414] .ksoftirqd+0xbc/0x1f8 [c000fffbbed0] [c00764cc] .kthread+0x128/0x134 [c000fffbbf90] [c0014248] .kernel_thread+0x4c/0x6c I believe the above warning is caused by the local_softirq_pending call on a foreign cpu before ksoftirqd/1 has been stopped. Looking at the code, I think this doesn't indicate a real bug, but it would be better if ksoftirqd didn't check local_softirq_pending after it's been kicked off its cpu, right? # echo 1 /sys/devices/system/cpu/cpu1/online BUG: using smp_processor_id() in preemptible [0001] code: swapper/0 caller is .dedicated_idle+0x68/0x22c Call Trace: [c000fffafc50] [] 0x (unreliable) [c000fffafcd0] [c01c9f1c] .smp_processor_id+0x154/0x168 [c000fffafd90] [c000f998] .dedicated_idle+0x68/0x22c [c000fffafe80] [c000fce8] .cpu_idle+0x34/0x4c [c000fffaff00] [c003a744] .start_secondary+0x10c/0x150 [c000fffaff90] [c000bd28] .enable_64b_mode+0x0/0x28 Should ppc64 simply use _smp_processor_id() in its idle loop code (like i386)? If I online and offline cpus rapidly enough I can eventually get the following: printk: 49 messages suppressed. BUG: using smp_processor_id() in preemptible [0001] code: events/3/1262 caller is .cache_reap+0x21c/0x2b8 Call Trace: [c000ed67bb90] [] 0x (unreliable) [c000ed67bc10] [c01c9f1c] .smp_processor_id+0x154/0x168 [c000ed67bcd0] [c00938e8] .cache_reap+0x21c/0x2b8 [c000ed67bda0] [c006f4bc] .worker_thread+0x230/0x310 [c000ed67bed0] [c00764cc] .kthread+0x128/0x134 [c000ed67bf90] [c0014248] .kernel_thread+0x4c/0x6c And this will repeat over and over even after I stop hotplugging cpus... from the same events thread so I think it's somehow gotten stuck? Anything I can do to further debug? Nathan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6-bk: cpu hotplug + preempt = smp_processor_id warnings galore
On Sat, Feb 12, 2005 at 12:56:54AM +0100, Matthias-Christian Ott wrote: Nathan Lynch wrote: With 2.6.11-rc3-bk7 on ppc64 I am seeing lots of smp_processor_id warnings whenever I hotplug cpus: ... Use get_cpu() (It disables preemption) or __smp_processor_id () (on a smp). It's not necessarily that simple (ok, maybe the idle loop warning is). But at least one of the warnings I listed appears to be caused by a kernel thread that is normally bound to a particular cpu trying to do normal processing on another cpu before it has stopped. Injudicious use of __smp_processor_id or get_cpu in this case would only obscure the problem. Nathan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6-bk: cpu hotplug + preempt = smp_processor_id warnings galore
On Sat, Feb 12, 2005 at 11:59:04AM -0700, Zwane Mwaikambo wrote:
How about;
Index: linux-2.6.11-rc3-mm2/kernel/softirq.c
===
RCS file: /home/cvsroot/linux-2.6.11-rc3-mm2/kernel/softirq.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 softirq.c
--- linux-2.6.11-rc3-mm2/kernel/softirq.c 11 Feb 2005 05:14:57 -
1.1.1.1
+++ linux-2.6.11-rc3-mm2/kernel/softirq.c 12 Feb 2005 18:24:54 -
@@ -355,8 +355,12 @@ static int ksoftirqd(void * __bind_cpu)
set_current_state(TASK_INTERRUPTIBLE);
while (!kthread_should_stop()) {
- if (!local_softirq_pending())
+ preempt_disable();
+ if (!local_softirq_pending()) {
+ preempt_enable_no_resched();
schedule();
+ preempt_disable();
+ }
__set_current_state(TASK_RUNNING);
@@ -364,14 +368,14 @@ static int ksoftirqd(void * __bind_cpu)
/* Preempt disable stops cpu going offline.
If already offline, we'll be on wrong CPU:
don't process */
- preempt_disable();
if (cpu_is_offline((long)__bind_cpu))
goto wait_to_die;
do_softirq();
- preempt_enable();
+ preempt_enable_no_resched();
cond_resched();
+ preempt_disable();
}
-
+ preempt_enable();
set_current_state(TASK_INTERRUPTIBLE);
}
__set_current_state(TASK_RUNNING);
Works ok here.
It looks as if we need to explicitly bind worker threads to a newly
onlined cpu. This gets rid of the smp_processor_id warnings from
cache_reap. Adding a little more instrumentation to the debug
smp_processor_id showed that new worker threads were actually running
on the wrong cpu...
Does this look ok?
Index: linux-2.6.11-rc4-bk2/kernel/workqueue.c
===
--- linux-2.6.11-rc4-bk2.orig/kernel/workqueue.c2005-02-14
11:13:08.0 -0600
+++ linux-2.6.11-rc4-bk2/kernel/workqueue.c 2005-02-14 15:18:35.0
-0600
@@ -485,8 +485,10 @@
case CPU_ONLINE:
/* Kick off worker threads. */
- list_for_each_entry(wq, workqueues, list)
+ list_for_each_entry(wq, workqueues, list) {
+ kthread_bind(wq-cpu_wq[hotcpu].thread, hotcpu);
wake_up_process(wq-cpu_wq[hotcpu].thread);
+ }
break;
case CPU_UP_CANCELED:
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6-bk: cpu hotplug + preempt = smp_processor_id warnings galore
On Tue, Feb 15, 2005 at 08:29:33AM -0700, Zwane Mwaikambo wrote: On Mon, 14 Feb 2005, Nathan Lynch wrote: It looks as if we need to explicitly bind worker threads to a newly onlined cpu. This gets rid of the smp_processor_id warnings from cache_reap. Adding a little more instrumentation to the debug smp_processor_id showed that new worker threads were actually running on the wrong cpu... Does this look ok? Yeah, does that patch suffice for all the warnings? Nope, ksoftirqd still requires your patch in order to kill the warnings there. Nathan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] kthread_bind new worker threads when onlining cpu
Hi Andrew-
On Tue, Feb 15, 2005 at 08:02:17AM +0100, Ingo Molnar wrote:
* Nathan Lynch [EMAIL PROTECTED] wrote:
It looks as if we need to explicitly bind worker threads to a newly
onlined cpu. This gets rid of the smp_processor_id warnings from
cache_reap. Adding a little more instrumentation to the debug
smp_processor_id showed that new worker threads were actually running
on the wrong cpu...
Does this look ok?
indeed - looks much better than the 'turn off the warning' solution.
Acked-by: Ingo Molnar [EMAIL PROTECTED]
We weren't binding new worker threads to their cpu when onlining.
Using preempt and the debug version of smp_processor_id found this.
Signed-off-by: Nathan Lynch [EMAIL PROTECTED]
Index: linux-2.6.11-rc4-bk2/kernel/workqueue.c
===
--- linux-2.6.11-rc4-bk2.orig/kernel/workqueue.c2005-02-14
11:13:08.0 -0600
+++ linux-2.6.11-rc4-bk2/kernel/workqueue.c 2005-02-14 15:18:35.0
-0600
@@ -485,8 +485,10 @@
case CPU_ONLINE:
/* Kick off worker threads. */
- list_for_each_entry(wq, workqueues, list)
+ list_for_each_entry(wq, workqueues, list) {
+ kthread_bind(wq-cpu_wq[hotcpu].thread, hotcpu);
wake_up_process(wq-cpu_wq[hotcpu].thread);
+ }
break;
case CPU_UP_CANCELED:
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 22/82] remove linux/version.h from drivers/message/fus ion
Matt Domsch wrote:
On Tue, Jul 19, 2005 at 06:07:41PM -0600, Moore, Eric Dean wrote:
On Tuesday, July 12, 2005 8:17 PM, Matt Domsch wrote:
In general, this construct:
-#if (LINUX_VERSION_CODE KERNEL_VERSION(2,6,6))
-static int inline scsi_device_online(struct scsi_device *sdev)
-{
- return sdev-online;
-}
-#endif
is better tested as:
#ifndef scsi_device_inline
static int inline scsi_device_online(struct scsi_device *sdev)
{
return sdev-online;
}
#endif
when you can. It cleanly eliminates the version test, and tests for
exactly what you're looking for - is this function defined.
What you illustrated above is not going to work.
If your doing #ifndef around a function, such as scsi_device_online, it's
not going to compile
when scsi_device_online is already implemented in the kernel tree.
The routine scsi_device_online is a function, not a define. For a define
this would work.
Sure it does, function names are defined symbols.
$ cat foo.c
static int foo(void) { return 0; }
#ifndef foo
static int foo(void) { return 0; }
#endif
$ gcc -c foo.c
foo.c:3: error: redefinition of 'foo'
foo.c:1: error: previous definition of 'foo' was here
I believe #ifdef/#ifndef can test only preprocessor symbols.
Nathan
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
topology api confusion
We need some clarity on how asm-generic/topology.h is intended to be
used. I suspect that it's supposed to be unconditionally included at
the end of the architecture's topology.h so that any elements which
are undefined by the arch have sensible default definitions. Looking
at 2.6.13-rc3, this is what ppc64, ia64, and x86_64 currently do,
however i386 does not (i386 pulls in the generic version only when
!CONFIG_NUMA).
The #ifndef guards around each element of the topology api
cannot serve their apparent intended purpose when the architecture
implements a given bit as a function instead of a macro
(e.g. cpu_to_node in ppc64):
asm-generic/topology.h:
#ifndef cpu_to_node
#define cpu_to_node(cpu)(0)
#endif
asm-ppc64/topology.h:
static inline int cpu_to_node(int cpu)
{
int node;
node = numa_cpu_lookup_table[cpu];
Since ppc64 unconditionally includes asm-generic/topology.h, all uses
of cpu_to_node are preprocessed to (0). Similar damage occurs with
every other topology function which happens to be a real function
instead of a macro. I'm surprised my ppc64 numa systems even boot ;)
If the intent is that the architecture is free to define only a subset
of the api and include the generic header for fallback definitions,
then we need to do the #ifndef __HAVE_ARCH_FOO thing, no? That is,
the code above would look like:
asm-generic/topology.h:
#ifndef __HAVE_ARCH_CPU_TO_NODE
#define cpu_to_node(cpu)(0)
#endif
asm-ppc64/topology.h:
#define __HAVE_ARCH_CPU_TO_NODE
static inline int cpu_to_node(int cpu)
{
int node;
node = numa_cpu_lookup_table[cpu];
Thought I'd ask for input first since this would involve a sweep of
include/asm-*.
Nathan
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: topology api confusion
Matthew Dobson wrote: Nathan Lynch wrote: We need some clarity on how asm-generic/topology.h is intended to be used. I suspect that it's supposed to be unconditionally included at the end of the architecture's topology.h so that any elements which are undefined by the arch have sensible default definitions. Looking at 2.6.13-rc3, this is what ppc64, ia64, and x86_64 currently do, however i386 does not (i386 pulls in the generic version only when !CONFIG_NUMA). The #ifndef guards around each element of the topology api cannot serve their apparent intended purpose when the architecture implements a given bit as a function instead of a macro (e.g. cpu_to_node in ppc64): When I originally wrote this all up, I had planned it to be used the way i386 does: offer a full implementation of topology if appropriate, else include the generic sane default. It has since changed to work more like you described: implement some (or all) of the topology functions, then include the generic one to define any you missed. OK. You are correct in noticing that things should (though apparently don't?) go wonky when arches define their topology functions as *functions*, rather than macros. It hasn't really seemed to bite anyone yet, so I've left it alone, though to be honest, it is as surprising to me that it works as it is to you. I've resisted creating #ifdef ARCH_HAVE_XXX all over the place, though maybe it is finally time? Things _do_ go wonky, but likely only on ppc64 -- all cpus show up in all nodes' cpumaps in sysfs. The other architectures which provide overrides and unconditionally include the generic topology.h define only macros iirc. If i386 were to include the generic topology.h it would have similar issues since it uses functions for some things too. Thought I'd ask for input first since this would involve a sweep of include/asm-*. It would indeed... I'd be more than happy to look at any patches you care to generate. As I said, it seems to work, though I'm certain it's all held together by GCC black magic voodoo, and probably a little duct-tape. A more obviously correct solution would not be a bad thing. :) I've got the changes ready, just need to test them a little more. Thanks. Nathan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] generate hotplug events for cpu online
On Thu, Apr 21, 2005 at 08:46:56PM +0200, Pavel Machek wrote: Hi! We already do kobject_hotplug for cpu offline; this adds a kobject_hotplug call for the online case. This is being requested by developers of an application which wants to be notified about both kinds of events. I'm afraid of bad interactions with swsusp/S3 on smp. We offline cpus there, but we probably do not want userland to know... The kobject_hotplug calls for cpu online and offline are present only in the code paths where the operation is initiated through the sysfs interface, so I don't think you have to worry. Nathan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CPU hotplug on i386
On Wed, Mar 16, 2005 at 02:21:52PM +0100, Pavel Machek wrote: Hi! I tried to solve long-standing uglyness in swsusp cmp code by calling cpu hotplug... only to find out that CONFIG_CPU_HOTPLUG is not available on i386. Is there way to enable CPU_HOTPLUG on i386? i386 cpu hotplug has been in -mm for a while. Don't know when (if ever) it will get merged. Nathan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CPU hotplug on i386
Hi again-
On Wed, Mar 16, 2005 at 02:21:52PM +0100, Pavel Machek wrote:
+ int cpu, error;
+ cpus_clear(frozen_cpus);
+ printk(Freezing cpus...\n);
+ for_each_online_cpu(cpu) {
+ if (!cpu)
+ continue;
+ cpu_set(cpu, frozen_cpus);
+ error = cpu_down(cpu);
+ if (!error)
+ continue;
+ printk(Error taking cpu %d down: %d\n, cpu, error);
+ panic(Too many cpus);
}
- printk(ok\n);
+ BUG_ON(smp_processor_id() != 0);
}
void enable_nonboot_cpus(void)
{
- printk(Restarting CPUs);
- atomic_set(freeze, 0);
- while (atomic_read(cpu_counter)) {
- cpu_relax();
- barrier();
+ int cpu, error;
+ printk(Thawing cpus...\n);
+ for_each_cpu_mask(cpu, frozen_cpus) {
+ if (!cpu)
+ continue;
+ error = cpu_up(cpu);
+ if (!error)
+ continue;
+ printk(Error taking cpu %d up: %d\n, cpu, error);
+ panic(Not enough cpus);
}
If you're going to depend on CONFIG_HOTPLUG_CPU for swsusp on smp, why
not offline the non-boot cpus from userspace? Same goes for onlining
additional cpus during resume.
Nathan
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][2.6.12-rc1-mm1] fix compile error in ppc64 prom.c
On Mon, Mar 21, 2005 at 04:19:01PM +0100, Mikael Pettersson wrote: Compiling 2.6.12-rc1-mm1 for ppc64 fails with: arch/ppc64/kernel/prom.c:1691: error: syntax error before 'prom_reconfig_notifier' arch/ppc64/kernel/prom.c:1692: error: field name not in record or union initializer arch/ppc64/kernel/prom.c:1692: error: (near initialization for 'prom_reconfig_nb') arch/ppc64/kernel/prom.c:1692: warning: initialization makes pointer from integer without a cast make[1]: *** [arch/ppc64/kernel/prom.o] Error 1 make: *** [arch/ppc64/kernel] Error 2 Fix: repair the obvious syntax error (missing =). Thanks for the fix; the mistake was mine. Lest Andrew and Paulus think I'm sending untested patches, the compiler I'm using (gcc 3.3.3-hammer) does not give an error or even a warning. Sorry for the inconvenience; I'll have to upgrade to a less forgiving version of gcc. Nathan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
ppc64 pSeries build broken in 2.6.12-rc1-bk1
Hi-
It seems that the pSeries reconfig patch series which Paul sent on
March 17th to Andrew on my behalf was incompletely merged into bk? It
looks like only the last two of the series are in -bk as of today
(2.6.12-rc1-mm1 has them all).
Subject lines and URLs for the series in question:
[PATCH 1/8] PPC64 preliminary changes to OF fixup functions
http://lkml.org/lkml/2005/3/17/41
[PATCH 2/8] PPC64 make OF node fixup code usable at runtime
http://lkml.org/lkml/2005/3/17/40
[PATCH 3/8] PPC64 introduce pSeries_reconfig.[ch]
http://lkml.org/lkml/2005/3/17/43
[PATCH 4/8] PPC64 prom.c: use pSeries reconfig notifier
http://lkml.org/lkml/2005/3/17/56
[PATCH 5/8] PPC64 pci_dn.c: use pSeries reconfig notifier
http://lkml.org/lkml/2005/3/17/57
[PATCH 6/8] PPC64 pSeries_iommu.c: use pSeries reconfig notifier
http://lkml.org/lkml/2005/3/17/54
[PATCH 7/8] PPC64 use pSeries reconfig notifier for cpu DLPAR
http://lkml.org/lkml/2005/3/17/44
[PATCH 8/8] PPC64 make cpu hotplug play well with maxcpus and smt-enabled
http://lkml.org/lkml/2005/3/17/55
With 2.6.12-rc1-bk1, if CONFIG_PPC_PSERIES and CONFIG_SMP are enabled,
the build errors out:
CC arch/ppc64/kernel/pSeries_smp.o
arch/ppc64/kernel/pSeries_smp.c:47:34: asm/pSeries_reconfig.h: No such
file or directory
If people would like something to use in the meantime, below is a
throwaway patch to work around the breakage (not intended for
inclusion). Disabling CONFIG_PPC_PSERIES should work around it also.
The real fix is to either revert patches 7 and 8, or merge 1-6 :)
Nathan
Index: linux-2.6.12-rc1-bk1/arch/ppc64/kernel/pSeries_smp.c
===
--- linux-2.6.12-rc1-bk1.orig/arch/ppc64/kernel/pSeries_smp.c 2005-03-21
20:28:22.0 +
+++ linux-2.6.12-rc1-bk1/arch/ppc64/kernel/pSeries_smp.c2005-03-21
21:00:16.0 +
@@ -44,7 +44,7 @@
#include asm/system.h
#include asm/rtas.h
#include asm/plpar_wrappers.h
-#include asm/pSeries_reconfig.h
+/* #include asm/pSeries_reconfig.h */
#include mpic.h
@@ -135,6 +135,7 @@ void pSeries_cpu_die(unsigned int cpu)
* the logical ids for sibling SMT threads x and y are adjacent, such
* that x^1 == y and y^1 == x.
*/
+#if 0
static int pSeries_add_processor(struct device_node *np)
{
unsigned int cpu;
@@ -247,7 +248,7 @@ static int pSeries_smp_notifier(struct n
static struct notifier_block pSeries_smp_nb = {
.notifier_call = pSeries_smp_notifier,
};
-
+#endif /* 0 */
#endif /* CONFIG_HOTPLUG_CPU */
/**
@@ -421,9 +422,11 @@ void __init smp_init_pSeries(void)
smp_ops-cpu_die = pSeries_cpu_die;
/* Processors can be added/removed only on LPAR */
+#if 0
if (systemcfg-platform == PLATFORM_PSERIES_LPAR)
pSeries_reconfig_notifier_register(pSeries_smp_nb);
#endif
+#endif
/* Mark threads which are still spinning in hold loops. */
if (cur_cpu_spec-cpu_features CPU_FTR_SMT)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][2.6.12-rc1-mm1] fix compile error in ppc64 prom.c
On Tue, Mar 22, 2005 at 08:55:15AM +1100, Paul Mackerras wrote: Mikael Pettersson writes: Compiling 2.6.12-rc1-mm1 for ppc64 fails with: arch/ppc64/kernel/prom.c:1691: error: syntax error before 'prom_reconfig_notifier' Currently prom.c is in a mess because Linus applied the last 2 of 8 patches from Nathan Lynch but not the first 6. :-P Actually, this one is my fault, although unless I'm really missing something gcc 3.4.2 silently accepts the invalid syntax. All eight of the patches from the pSeries reconfig series are present in 2.6.12-rc1-mm1. The error Mikael reported is unrelated to the state of Linus' tree, and his patch is correct. (The mistake is present in both versions of the patches which I posted for review on linuxppc64-dev; nobody caught it.) Nathan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ACPI] Re: [RFC 5/6]clean cpu state after hotremove CPU
On Mon, Apr 04, 2005 at 01:42:18PM +0800, Li Shaohua wrote:
Hi,
On Mon, 2005-04-04 at 13:28, Nathan Lynch wrote:
On Mon, Apr 04, 2005 at 10:07:02AM +0800, Li Shaohua wrote:
Clean up all CPU states including its runqueue and idle thread,
so we can use boot time code without any changes.
Note this makes /sys/devices/system/cpu/cpux/online unworkable.
In what sense does it make the online attribute unworkable?
I removed the idle thread and other CPU states, and makes the dead CPU
into a 'halt' busy loop.
diff -puN kernel/exit.c~cpu_state_clean kernel/exit.c
--- linux-2.6.11/kernel/exit.c~cpu_state_clean2005-03-31
10:50:27.0 +0800
+++ linux-2.6.11-root/kernel/exit.c 2005-03-31 10:50:27.0
+0800
@@ -845,6 +845,65 @@ fastcall NORET_TYPE void do_exit(long co
for (;;) ;
}
+#ifdef CONFIG_STR_SMP
+void do_exit_idle(void)
+{
+ struct task_struct *tsk = current;
+ int group_dead;
+
+ BUG_ON(tsk-pid);
+ BUG_ON(tsk-mm);
+
+ if (tsk-io_context)
+ exit_io_context();
+ tsk-flags |= PF_EXITING;
+ tsk-it_virt_expires = cputime_zero;
+ tsk-it_prof_expires = cputime_zero;
+ tsk-it_sched_expires = 0;
+
+ acct_update_integrals(tsk);
+ update_mem_hiwater(tsk);
+ group_dead = atomic_dec_and_test(tsk-signal-live);
+ if (group_dead) {
+ del_timer_sync(tsk-signal-real_timer);
+ acct_process(-1);
+ }
+ exit_mm(tsk);
+
+ exit_sem(tsk);
+ __exit_files(tsk);
+ __exit_fs(tsk);
+ exit_namespace(tsk);
+ exit_thread();
+ exit_keys(tsk);
+
+ if (group_dead tsk-signal-leader)
+ disassociate_ctty(1);
+
+ module_put(tsk-thread_info-exec_domain-module);
+ if (tsk-binfmt)
+ module_put(tsk-binfmt-module);
+
+ tsk-exit_code = -1;
+ tsk-exit_state = EXIT_DEAD;
+
+ /* in release_task */
+ atomic_dec(tsk-user-processes);
+ write_lock_irq(tasklist_lock);
+ __exit_signal(tsk);
+ __exit_sighand(tsk);
+ write_unlock_irq(tasklist_lock);
+ release_thread(tsk);
+ put_task_struct(tsk);
+
+ tsk-flags |= PF_DEAD;
+#ifdef CONFIG_NUMA
+ mpol_free(tsk-mempolicy);
+ tsk-mempolicy = NULL;
+#endif
+}
+#endif
I don't understand why this is needed at all. It looks like a fair
amount of code from do_exit is being duplicated here.
Yes, exactly. Someone who understand do_exit please help clean up the
code. I'd like to remove the idle thread, since the smpboot code will
create a new idle thread.
I'd say fix the smpboot code so that it doesn't create new idle tasks
except during boot.
We've been
doing cpu removal on ppc64 logical partitions for a while and never
needed to do anything like this.
Did it remove idle thread? or dead cpu is in a busy loop of idle?
Neither. The cpu is definitely offline, but there is no reason to
free the idle thread.
Maybe idle_task_exit would suffice?
idle_task_exit seems just drop mm. We need destroy the idle task for
physical CPU hotplug, right?
No.
I don't understand the need for this, either. The existing cpu
hotplug notifier in the scheduler takes care of initializing the sched
domains and groups appropriately for online/offline events; why do you
need to touch the runqueue structures?
If a CPU is physically hotremoved from the system, shouldn't we clean
its runqueue?
No. It should make zero difference to the scheduler whether the play
dead cpu hotplug or physical hotplug is being used.
Nathan
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ACPI] Re: [RFC 5/6]clean cpu state after hotremove CPU
Hi Nigel! On Tue, Apr 05, 2005 at 08:14:25AM +1000, Nigel Cunningham wrote: On Tue, 2005-04-05 at 01:33, Nathan Lynch wrote: Yes, exactly. Someone who understand do_exit please help clean up the code. I'd like to remove the idle thread, since the smpboot code will create a new idle thread. I'd say fix the smpboot code so that it doesn't create new idle tasks except during boot. Would that mean that CPUs that were physically hotplugged wouldn't get idle threads? No, that wouldn't work. I am saying that there's little to gain by adding all this complexity for destroying the idle tasks when it's fairly simple to create num_possible_cpus() - 1 idle tasks* to accommodate any additional cpus which may come along. This is what ppc64 does now, and it should be feasible on any architecture which supports cpu hotplug. Nathan * num_possible_cpus() - 1 because the idle task for the boot cpu is created in sched_init. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] explicitly bind idle tasks
On Sun, Feb 27, 2005 at 02:49:28PM -0800, Andrew Morton wrote:
Benjamin Herrenschmidt [EMAIL PROTECTED] wrote:
- if (cpu_is_offline(smp_processor_id())
+if (cpu_is_offline(_smp_processor_id())
system_state == SYSTEM_RUNNING)
cpu_die();
}
_
This is the idle loop. Is that ever supposed to be preempted ?
Nope, it's a false positive. We had to do the same in x86's idle loop and
probably others will hit it.
Perhaps I'm missing something, but is there any reason we can't do
the following? I've tested it on ppc64, doesn't seem to break anything.
With hotplug cpu and preempt, we tend to see smp_processor_id warnings
from idle loop code because it's always checking whether its cpu has
gone offline. Replacing every use of smp_processor_id with
_smp_processor_id in all idle loop code is one solution; another way
is explicitly binding idle threads to their cpus (the smp_processor_id
warning does not fire if the caller is bound only to the calling cpu).
This has the (admittedly slight) advantage of letting us know if an
idle thread ever runs on the wrong cpu.
Signed-off-by: Nathan Lynch [EMAIL PROTECTED]
Index: linux-2.6.11-rc5-mm1/init/main.c
===
--- linux-2.6.11-rc5-mm1.orig/init/main.c 2005-03-02 00:12:07.0
+
+++ linux-2.6.11-rc5-mm1/init/main.c2005-03-02 00:53:04.0 +
@@ -638,6 +638,10 @@
{
lock_kernel();
/*
+* init can run on any cpu.
+*/
+ set_cpus_allowed(current, CPU_MASK_ALL);
+ /*
* Tell the world that we're going to be the grim
* reaper of innocent orphaned children.
*
Index: linux-2.6.11-rc5-mm1/kernel/sched.c
===
--- linux-2.6.11-rc5-mm1.orig/kernel/sched.c2005-03-02 00:12:07.0
+
+++ linux-2.6.11-rc5-mm1/kernel/sched.c 2005-03-02 00:47:14.0 +
@@ -4092,6 +4092,7 @@
idle-array = NULL;
idle-prio = MAX_PRIO;
idle-state = TASK_RUNNING;
+ idle-cpus_allowed = cpumask_of_cpu(cpu);
set_task_cpu(idle, cpu);
spin_lock_irqsave(rq-lock, flags);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ppc64: fix use kref for device_node refcounting
On Mon, 2005-01-24 at 02:15 -0800, Andrew Morton wrote:
ppc64-use-kref-for-device_node-refcounting.patch
ppc64: use kref for device_node refcounting
This introduced an unbalanced get/put in of_add_node which would cause
newly-added device nodes to be prematurely freed. Sorry for the
screwup, a more rigorously tested fix follows.
Signed-off-by: Nathan Lynch [EMAIL PROTECTED]
---
diff -puN arch/ppc64/kernel/prom.c~fix-kref-devnode arch/ppc64/kernel/prom.c
--- linux-2.6.11-rc2-mm1/arch/ppc64/kernel/prom.c~fix-kref-devnode
2005-01-25 21:10:50.0 -0600
+++ linux-2.6.11-rc2-mm1-nathanl/arch/ppc64/kernel/prom.c 2005-01-25
21:14:02.0 -0600
@@ -1771,6 +1771,7 @@ int of_add_node(const char *path, struct
np-properties = proplist;
OF_MARK_DYNAMIC(np);
kref_init(np-kref);
+ of_node_get(np);
np-parent = derive_parent(path);
if (!np-parent) {
kfree(np);
_
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] unexport register_cpu and unregister_cpu
http://linus.bkbits.net:8080/linux-2.5/[EMAIL
PROTECTED]|src/|src/drivers|src/drivers/base|related/drivers/base/cpu.c
This changeset introduced exports for register_cpu and unregister_cpu
right after 2.6.10. As far as I can tell these are not called from any
code which can be built as a module, and I can't think of a good reason
why any out of tree code would use them. Unless I've missed something,
can we remove them before 2.6.11?
Build-tested for ia64 and i386.
Signed-off-by: Nathan Lynch [EMAIL PROTECTED]
Index: linux-2.6.11-rc2-mm1/drivers/base/cpu.c
===
--- linux-2.6.11-rc2-mm1.orig/drivers/base/cpu.c2005-01-25
23:50:02.677255800 -0600
+++ linux-2.6.11-rc2-mm1/drivers/base/cpu.c 2005-01-25 23:56:28.436611464
-0600
@@ -64,7 +64,6 @@
return;
}
-EXPORT_SYMBOL(unregister_cpu);
#else /* ... !CONFIG_HOTPLUG_CPU */
static inline void register_cpu_control(struct cpu *cpu)
{
@@ -96,9 +95,6 @@
register_cpu_control(cpu);
return error;
}
-#ifdef CONFIG_HOTPLUG_CPU
-EXPORT_SYMBOL(register_cpu);
-#endif
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] unexport register_cpu and unregister_cpu
On Wed, 2005-01-26 at 10:22 -0800, Keshavamurthy Anil S wrote: On Wed, Jan 26, 2005 at 12:55:47AM -0600, Nathan Lynch wrote: http://linus.bkbits.net:8080/linux-2.5/[EMAIL PROTECTED]|src/|src/drivers|src/drivers/base|related/drivers/base/cpu.c This changeset introduced exports for register_cpu and unregister_cpu right after 2.6.10. As far as I can tell these are not called from any code which can be built as a module, and I can't think of a good reason why any out of tree code would use them. Unless I've missed something, can we remove them before 2.6.11? No this is not correct. ACPI processor.ko driver which supports physical CPU hotplug needs register_cpu() and unregister_cpu() functions for dynamically hotadd/hotremove support of the processors. I do not understand your objection. The processor module does not call the interfaces in question directly. They are called only from arch setup code (e.g. arch/ia64/kernel/topology.c) which is never built as a module. Please see drivers/acpi/processor_core.c acpi_processor_hotadd_init() - arch_register_cpu() - -register_cpu(). Sure -- the arch_register_cpu and arch_unregister_cpu symbols need to be exported for this use (and they are). Exporting register_cpu and unregister_cpu is unnecessary. I double-checked an ia64 build with CONFIG_ACPI_HOTPLUG_CPU=y and CONFIG_ACPI_PROCESSOR=m and saw no errors or warnings caused by the change... Nathan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.11-rc2-mm2 - freeing b_committed_data
Hi- With both 2.6.11-rc2-mm1 and -mm2 I'm seeing this message occasionally on a ppc64 box with ext3 filesystems: __journal_remove_journal_head: freeing b_committed_data Is this cause for concern? Nathan # # Automatically generated make config: don't edit # Linux kernel version: 2.6.11-rc2-mm2 # Sat Jan 29 15:32:58 2005 # CONFIG_64BIT=y CONFIG_MMU=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_GENERIC_ISA_DMA=y CONFIG_HAVE_DEC_LOCK=y CONFIG_EARLY_PRINTK=y CONFIG_COMPAT=y CONFIG_FRAME_POINTER=y CONFIG_FORCE_MAX_ZONEORDER=13 # # Code maturity level options # CONFIG_EXPERIMENTAL=y CONFIG_CLEAN_COMPILE=y CONFIG_LOCK_KERNEL=y # # General setup # CONFIG_LOCALVERSION=-ntl CONFIG_SWAP=y CONFIG_SYSVIPC=y # CONFIG_POSIX_MQUEUE is not set CONFIG_BSD_PROCESS_ACCT=y # CONFIG_BSD_PROCESS_ACCT_V3 is not set CONFIG_SYSCTL=y # CONFIG_AUDIT is not set CONFIG_LOG_BUF_SHIFT=19 CONFIG_HOTPLUG=y CONFIG_KOBJECT_UEVENT=y CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y # CONFIG_CPUSETS is not set # CONFIG_EMBEDDED is not set CONFIG_KALLSYMS=y CONFIG_KALLSYMS_ALL=y # CONFIG_KALLSYMS_EXTRA_PASS is not set CONFIG_FUTEX=y CONFIG_EPOLL=y # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set # CONFIG_LTT is not set CONFIG_SHMEM=y CONFIG_CC_ALIGN_FUNCTIONS=0 CONFIG_CC_ALIGN_LABELS=0 CONFIG_CC_ALIGN_LOOPS=0 CONFIG_CC_ALIGN_JUMPS=0 # CONFIG_TINY_SHMEM is not set # # Loadable module support # CONFIG_MODULES=y CONFIG_MODULE_UNLOAD=y # CONFIG_MODULE_FORCE_UNLOAD is not set CONFIG_OBSOLETE_MODPARM=y # CONFIG_MODVERSIONS is not set # CONFIG_MODULE_SRCVERSION_ALL is not set CONFIG_KMOD=y CONFIG_STOP_MACHINE=y CONFIG_SYSVIPC_COMPAT=y # # Platform support # # CONFIG_PPC_ISERIES is not set CONFIG_PPC_MULTIPLATFORM=y CONFIG_PPC_PSERIES=y # CONFIG_PPC_PMAC is not set # CONFIG_PPC_MAPLE is not set CONFIG_PPC=y CONFIG_PPC64=y CONFIG_PPC_OF=y CONFIG_ALTIVEC=y CONFIG_PPC_SPLPAR=y CONFIG_IBMVIO=y # CONFIG_U3_DART is not set # CONFIG_BOOTX_TEXT is not set # CONFIG_POWER4_ONLY is not set # CONFIG_IOMMU_VMERGE is not set CONFIG_SMP=y CONFIG_NR_CPUS=128 CONFIG_HMT=y CONFIG_DISCONTIGMEM=y CONFIG_NUMA=y CONFIG_SCHED_SMT=y CONFIG_PREEMPT=y CONFIG_PREEMPT_BKL=y CONFIG_EEH=y CONFIG_GENERIC_HARDIRQS=y CONFIG_PPC_RTAS=y # CONFIG_RTAS_FLASH is not set CONFIG_SCANLOG=y CONFIG_LPARCFG=y # # General setup # CONFIG_PCI=y CONFIG_PCI_DOMAINS=y CONFIG_BINFMT_ELF=y # CONFIG_BINFMT_MISC is not set CONFIG_PCI_LEGACY_PROC=y CONFIG_PCI_NAMES=y CONFIG_HOTPLUG_CPU=y # # PCCARD (PCMCIA/CardBus) support # # CONFIG_PCCARD is not set # # PC-card bridges # # # PCI Hotplug Support # # CONFIG_HOTPLUG_PCI is not set CONFIG_PROC_DEVICETREE=y # CONFIG_CMDLINE_BOOL is not set # # Device Drivers # # # Generic Driver Options # CONFIG_STANDALONE=y CONFIG_PREVENT_FIRMWARE_BUILD=y CONFIG_FW_LOADER=y # CONFIG_DEBUG_DRIVER is not set # # Connector - unified userspace - kernelspace linker # # CONFIG_CONNECTOR is not set # # Memory Technology Devices (MTD) # # CONFIG_MTD is not set # # Parallel port support # # CONFIG_PARPORT is not set # # Plug and Play support # # # Block devices # CONFIG_BLK_DEV_FD=y # CONFIG_BLK_CPQ_DA is not set # CONFIG_BLK_CPQ_CISS_DA is not set # CONFIG_BLK_DEV_DAC960 is not set # CONFIG_BLK_DEV_UMEM is not set # CONFIG_BLK_DEV_COW_COMMON is not set CONFIG_BLK_DEV_LOOP=y # CONFIG_BLK_DEV_CRYPTOLOOP is not set # CONFIG_BLK_DEV_NBD is not set # CONFIG_BLK_DEV_SX8 is not set CONFIG_BLK_DEV_RAM=y CONFIG_BLK_DEV_RAM_COUNT=16 CONFIG_BLK_DEV_RAM_SIZE=4096 CONFIG_BLK_DEV_INITRD=y CONFIG_INITRAMFS_SOURCE= # CONFIG_CDROM_PKTCDVD is not set # # IO Schedulers # CONFIG_IOSCHED_NOOP=y CONFIG_IOSCHED_AS=y CONFIG_IOSCHED_DEADLINE=y CONFIG_IOSCHED_CFQ=y # CONFIG_ATA_OVER_ETH is not set # # ATA/ATAPI/MFM/RLL support # CONFIG_IDE=y CONFIG_BLK_DEV_IDE=y # # Please see Documentation/ide.txt for help/info on IDE drives # # CONFIG_BLK_DEV_IDE_SATA is not set CONFIG_BLK_DEV_IDEDISK=y # CONFIG_IDEDISK_MULTI_MODE is not set CONFIG_BLK_DEV_IDECD=y # CONFIG_BLK_DEV_IDETAPE is not set # CONFIG_BLK_DEV_IDEFLOPPY is not set # CONFIG_BLK_DEV_IDESCSI is not set # CONFIG_IDE_TASK_IOCTL is not set # # IDE chipset support/bugfixes # CONFIG_IDE_GENERIC=y CONFIG_BLK_DEV_IDEPCI=y CONFIG_IDEPCI_SHARE_IRQ=y # CONFIG_BLK_DEV_OFFBOARD is not set CONFIG_BLK_DEV_GENERIC=y # CONFIG_BLK_DEV_OPTI621 is not set # CONFIG_BLK_DEV_SL82C105 is not set CONFIG_BLK_DEV_IDEDMA_PCI=y # CONFIG_BLK_DEV_IDEDMA_FORCED is not set CONFIG_IDEDMA_PCI_AUTO=y # CONFIG_IDEDMA_ONLYDISK is not set # CONFIG_BLK_DEV_AEC62XX is not set # CONFIG_BLK_DEV_ALI15X3 is not set # CONFIG_BLK_DEV_AMD74XX is not set # CONFIG_BLK_DEV_CMD64X is not set # CONFIG_BLK_DEV_TRIFLEX is not set # CONFIG_BLK_DEV_CY82C693 is not set # CONFIG_BLK_DEV_CS5520 is not set # CONFIG_BLK_DEV_CS5530 is not set # CONFIG_BLK_DEV_HPT34X is not set # CONFIG_BLK_DEV_HPT366 is not set # CONFIG_BLK_DEV_SC1200 is not set # CONFIG_BLK_DEV_PIIX is not set # CONFIG_BLK_DEV_NS87415 is not set CONFIG_BLK_DEV_PDC202XX_OLD=y # CONFIG_PDC202XX_BURST is not set
Re: [RFC/PATCH] add support for sysdev class attributes
On Tue, 2005-01-11 at 11:29 -0800, Greg KH wrote:
On Mon, Jan 10, 2005 at 09:58:03AM -0600, Nathan Lynch wrote:
On Fri, 2005-01-07 at 21:07 -0800, Greg KH wrote:
On Fri, Jan 07, 2005 at 04:28:12PM -0600, Nathan Lynch wrote:
@@ -88,6 +123,12 @@ int sysdev_class_register(struct sysdev_
INIT_LIST_HEAD(cls-drivers);
cls-kset.subsys = system_subsys;
kset_set_kset_s(cls, system_subsys);
+
+ /* I'm not going to claim to understand this; see
+* fs/sysfs/file::check_perm for how sysfs_ops are selected
+*/
+ cls-kset.kobj.ktype = sysdev_class_ktype;
+
I think you need to understand this, and then submit a patch without
such a comment :)
And probably without such code, as I don't think you need to do that.
Sure, now I'm not sure how I convinced myself that bit was needed.
Things work fine without it.
OK I'm an idiot, because things certainly do not work without somehow
associating the new sysfs_ops with the sysdev_class being registered.
Otherwise, sysfs winds up using sysdev_store/sysdev_show, since
ktype_sysdev is the kobj_type for the system subsystem.
Since sysdev_register is explicitly associating ktype_sysdev with the
sysdev being registered anyway, I think it should be fine to define
system_subsys with sysdev_class_ktype for its kobj_type. What do you
say?
Before I repatch, does sysdev_class_ktype need a release function?
Why would it not?
I'm not sure what it would actually do... it seems that we would need
it if sysdevs were dynamically allocated, but they're not. Not that
they couldn't be, but that's existing practice afaict. In any case,
it's a matter for a separate patch, I would say.
Updated patch below:
Add support for system device class attributes. I would like to have
this for doing cpu add and remove on ppc64, and I think the memory
hotplug people probably will want it, too.
o Add the necessary show and store methods and related data
structures.
o Add sysdev_create_file and sysdev_remove_file.
o Make the system subsys' ktype ktype_sysdev_class instead of
ktype_sysdev, which should be used only for sysdevs.
Signed-off-by: Nathan Lynch [EMAIL PROTECTED]
---
diff -puN drivers/base/sys.c~sysdev_class-attr-support drivers/base/sys.c
--- linux-2.6.11-rc1-bk1/drivers/base/sys.c~sysdev_class-attr-support
2005-01-17 17:04:16.0 -0600
+++ linux-2.6.11-rc1-bk1-nathanl/drivers/base/sys.c 2005-01-17
17:32:25.0 -0600
@@ -76,10 +76,45 @@ void sysdev_remove_file(struct sys_devic
EXPORT_SYMBOL_GPL(sysdev_create_file);
EXPORT_SYMBOL_GPL(sysdev_remove_file);
+#define to_sysdev_class(k) container_of(to_kset(k), struct sysdev_class, kset)
+#define to_sysdev_class_attr(a) container_of(a, struct sysdev_class_attribute,
attr)
+
+static ssize_t
+sysdev_class_show(struct kobject * kobj, struct attribute * attr, char *
buffer)
+{
+ struct sysdev_class * sysdev_class = to_sysdev_class(kobj);
+ struct sysdev_class_attribute * sysdev_class_attr =
to_sysdev_class_attr(attr);
+
+ if (sysdev_class_attr-show)
+ return sysdev_class_attr-show(sysdev_class, buffer);
+ return 0;
+}
+
+static ssize_t
+sysdev_class_store(struct kobject * kobj, struct attribute * attr,
+const char * buffer, size_t count)
+{
+ struct sysdev_class * sysdev_class = to_sysdev_class(kobj);
+ struct sysdev_class_attribute * sysdev_class_attr =
to_sysdev_class_attr(attr);
+
+ if (sysdev_class_attr-store)
+ return sysdev_class_attr-store(sysdev_class, buffer, count);
+ return 0;
+}
+
+static struct sysfs_ops sysdev_class_sysfs_ops = {
+ .show = sysdev_class_show,
+ .store = sysdev_class_store,
+};
+
+static struct kobj_type sysdev_class_ktype = {
+ .sysfs_ops = sysdev_class_sysfs_ops,
+};
+
/*
* declare system_subsys
*/
-decl_subsys(system, ktype_sysdev, NULL);
+decl_subsys(system, sysdev_class_ktype, NULL);
int sysdev_class_register(struct sysdev_class * cls)
{
@@ -98,6 +133,19 @@ void sysdev_class_unregister(struct sysd
kset_unregister(cls-kset);
}
+int sysdev_class_create_file(struct sysdev_class *s, struct
sysdev_class_attribute *a)
+{
+ return sysfs_create_file(s-kset.kobj, a-attr);
+}
+
+
+void sysdev_class_remove_file(struct sysdev_class *s, struct
sysdev_class_attribute *a)
+{
+ sysfs_remove_file(s-kset.kobj, a-attr);
+}
+
+EXPORT_SYMBOL_GPL(sysdev_class_create_file);
+EXPORT_SYMBOL_GPL(sysdev_class_remove_file);
EXPORT_SYMBOL_GPL(sysdev_class_register);
EXPORT_SYMBOL_GPL(sysdev_class_unregister);
diff -puN include/linux/sysdev.h~sysdev_class-attr-support
include/linux/sysdev.h
--- linux-2.6.11-rc1-bk1/include/linux/sysdev.h~sysdev_class-attr-support
2005-01-17 17:04:16.0 -0600
+++ linux-2.6.11-rc1-bk1-nathanl/include/linux/sysdev.h 2005-01-17
17:04:16.0 -0600
@@ -40,6 +40,21 @@ struct sysdev_class
Re: [PATCH 1/2] leds-lp5521: fix a build warning
On Wed, 2013-01-23 at 08:07 +, Kim, Milo wrote:
This patch removes a build warning below.(ARCH=x86_64)
drivers/leds/leds-lp5521.c: In function lp5521_firmware_loaded:
drivers/leds/leds-lp5521.c:257:4: warning: format %d expects argument of type
in
t, but argument 3 has type size_t [-Wformat]
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/leds/leds-lp5521.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
index 80adfb4..74946f4 100644
--- a/drivers/leds/leds-lp5521.c
+++ b/drivers/leds/leds-lp5521.c
@@ -253,7 +253,7 @@ static void lp5521_firmware_loaded(struct lp55xx_chip
*chip)
const struct firmware *fw = chip-fw;
if (fw-size LP5521_PROGRAM_LENGTH) {
- dev_err(chip-cl-dev, firmware data size overflow: %d\n,
+ dev_err(chip-cl-dev, firmware data size overflow: %zd\n,
fw-size);
Documentation/printk-formats.txt says %zu is to be used for size_t.
Same comment goes for patch 2/2.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] leds: simply LED trigger list management
On Thu, 2013-01-17 at 01:06 +, Kim, Milo wrote:
@@ -242,17 +233,15 @@ EXPORT_SYMBOL_GPL(led_trigger_unregister);
void led_trigger_event(struct led_trigger *trig,
enum led_brightness brightness)
{
- struct list_head *entry;
+ struct led_classdev *led_cdev;
if (!trig)
return;
read_lock(trig-leddev_list_lock);
- list_for_each(entry, trig-led_cdevs) {
- struct led_classdev *led_cdev;
-
- led_cdev = list_entry(entry, struct led_classdev, trig_list);
- led_set_brightness(led_cdev, brightness);
+ list_for_each_entry(led_cdev, leds_list, node) {
+ if (led_cdev-trigger == trig)
+ led_set_brightness(led_cdev, brightness);
}
read_unlock(trig-leddev_list_lock);
Continuing to use trig-leddev_list_lock doesn't seem right. Shouldn't
traversal of leds_list be guarded by the leds_list_lock rwsem? And if
so, is it safe to use a potentially-blocking lock in this context?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] leds: simply LED trigger list management
On Fri, 2013-01-18 at 00:00 +, Kim, Milo wrote:
-Original Message-
From: Nathan Lynch [mailto:n...@pobox.com]
On Thu, 2013-01-17 at 01:06 +, Kim, Milo wrote:
@@ -242,17 +233,15 @@ EXPORT_SYMBOL_GPL(led_trigger_unregister);
void led_trigger_event(struct led_trigger *trig,
enum led_brightness brightness)
{
- struct list_head *entry;
+ struct led_classdev *led_cdev;
if (!trig)
return;
read_lock(trig-leddev_list_lock);
- list_for_each(entry, trig-led_cdevs) {
- struct led_classdev *led_cdev;
-
- led_cdev = list_entry(entry, struct led_classdev,
trig_list);
- led_set_brightness(led_cdev, brightness);
+ list_for_each_entry(led_cdev, leds_list, node) {
+ if (led_cdev-trigger == trig)
+ led_set_brightness(led_cdev, brightness);
}
read_unlock(trig-leddev_list_lock);
Continuing to use trig-leddev_list_lock doesn't seem right. Shouldn't
traversal of leds_list be guarded by the leds_list_lock rwsem? And if
so, is it safe to use a potentially-blocking lock in this context?
(Sorry for the typo in title: 'simply' - 'simplify')
Thanks for your opinion. I agree with you.
The read_lock()/unlock() of 'leddev_list_lock' should be replaced with
down_read()/up_read() of 'leds_list_lock'.
Then, RW lock of led_trigger can be removed also.
But led_trigger_event() can be called from atomic/interrupt context, no?
We can't use a rwsem in this path.
And I meant to mention earlier -- this change would cause this function
to scan all led devices in the system, whereas right now it consults
only the leds that are associated with the trigger. That seems like a
step backwards for a potentially performance-sensitive path.
BTW, I need more education about the concurrency.
We can see complex RW down/up safe code with list management in LED class
driver.
RW semaphores are 'leds_list_lock', 'triggers_list_lock' and 'trigger_lock'.
Are those are safe access in case a user-space via sysfs and driver API calls
by
LED device(s) can happen at the same time?
If so, can we make them more simple?
I think *entry point* of access can be wrapped with semaphores or MUTEX
rather than guard code for accessing LED lists one by one.
It seems to me that the relatively fine-grained lists and locks that
exist right now provide a certain level of flexibility and performance
that would be hard to achieve with a coarser model.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Hotplug_sig] [patch 1/1] Hot plug CPU to support physical add of new processors (i386)
[EMAIL PROTECTED] wrote:
+#ifdef CONFIG_HOTPLUG_CPU
+ if (cpu_online(cpu)) {
+#else
if (cpu_online(cpu) || !cpu_present(cpu)) {
+#endif
ret = -EINVAL;
goto out;
}
Why this change? I think the cpu_present check is needed for ppc64
since it has non-present cpus in sysfs.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Hotplug_sig] [patch 1/1] Hot plug CPU to support physical add of new processors (i386)
Protasevich, Natalie wrote:
+#ifdef CONFIG_HOTPLUG_CPU
+ if (cpu_online(cpu)) {
+#else
if (cpu_online(cpu) || !cpu_present(cpu)) {
+#endif
ret = -EINVAL;
goto out;
}
Why this change? I think the cpu_present check is needed for
ppc64 since it has non-present cpus in sysfs.
The new processor was never brought up, its bit is only set in
cpu_possible_map, but not in present map.
If a cpu is physically present and is capable of being onlined it
should be marked in cpu_present_map, please. This change would break
ppc64; can you rework it?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2.6.20] tasks cannot run on cpus onlined after boot
Commit 5c1e176781f43bc902a51e5832f789756bff911b (sched: force /sbin/init off isolated cpus) sets init's cpus_allowed to a subset of cpu_online_map at boot time, which means that tasks won't be scheduled on cpus that are added to the system later. Make init's cpus_allowed a subset of cpu_possible_map instead. This should still preserve the behavior that Nick's change intended. Thanks to Giuliano Pochini for reporting this and testing the fix: http://ozlabs.org/pipermail/linuxppc-dev/2006-December/029397.html Signed-off-by: Nathan Lynch [EMAIL PROTECTED] --- This is a regression from 2.6.18. Assuming this change is okay, this should go to -stable for 2.6.19.x. diff --git a/kernel/sched.c b/kernel/sched.c index b515e3c..3c8b1c5 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -6875,7 +6875,7 @@ void __init sched_init_smp(void) lock_cpu_hotplug(); arch_init_sched_domains(cpu_online_map); - cpus_andnot(non_isolated_cpus, cpu_online_map, cpu_isolated_map); + cpus_andnot(non_isolated_cpus, cpu_possible_map, cpu_isolated_map); if (cpus_empty(non_isolated_cpus)) cpu_set(smp_processor_id(), non_isolated_cpus); unlock_cpu_hotplug(); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: topology api confusion
Anton Blanchard wrote: Hi, We need some clarity on how asm-generic/topology.h is intended to be used. I suspect that it's supposed to be unconditionally included at the end of the architecture's topology.h so that any elements which are undefined by the arch have sensible default definitions. Looking at 2.6.13-rc3, this is what ppc64, ia64, and x86_64 currently do, however i386 does not (i386 pulls in the generic version only when !CONFIG_NUMA). The #ifndef guards around each element of the topology api cannot serve their apparent intended purpose when the architecture implements a given bit as a function instead of a macro (e.g. cpu_to_node in ppc64): Since it doesnt look like this will be resolved by 2.6.13 and NUMA is currently completely broken on ppc64, how does this patch look? Yes, this change is the least risk for now, thanks. -- Dont include asm-generic/topology.h unconditionally, we end up overriding all the ppc64 specific functions when NUMA is on. Signed-off-by: Anton Blanchard [EMAIL PROTECTED] Index: linux-2.6.git-work/include/asm-ppc64/topology.h === --- linux-2.6.git-work.orig/include/asm-ppc64/topology.h 2005-07-30 23:49:56.0 +1000 +++ linux-2.6.git-work/include/asm-ppc64/topology.h 2005-08-01 14:43:49.0 +1000 @@ -33,6 +33,7 @@ return first_cpu(tmp); } +#define pcibus_to_node(node)(-1) #define pcibus_to_cpumask(bus) (cpu_online_map) #define nr_cpus_node(node) (nr_cpus_in_node[node]) @@ -59,8 +60,10 @@ .nr_balance_failed = 0,\ } -#endif /* CONFIG_NUMA */ +#else #include asm-generic/topology.h +#endif /* CONFIG_NUMA */ + #endif /* _ASM_PPC64_TOPOLOGY_H */ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH/RFC 2.6.21 3/5] ehca: completion queue: remove use of do_mmap()
Christoph Hellwig wrote: On Thu, Jan 11, 2007 at 08:08:36PM +0100, Hoang-Nam Nguyen wrote: spin_lock_irqsave(ehca_cq_idr_lock, flags); while (my_cq-nr_callbacks) yield(); Calling yield is a very bad idea in general. You should probably add a waitqueue that gets woken when nr_callbacks reaches zero to sleep effectively here. Isn't that code outright buggy? Calling into the scheduler with a spinlock held and local interrupts disabled... - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CPU hotplug broken with 2GB VMSPLIT
Jens Axboe wrote: On Thu, Nov 30 2006, Jens Axboe wrote: Hi, Just got a new notebook (Lenovo X60), setup a custom kernel and then I noticed that suspend to ram doesn't work anymore. The machine suspends just fine, on resume it brings back the text display but reboots after it has stalled for a few seconds. On the suggestion of Pavel, I tried testing CPU hotplug, and indeed he was right: I can offline 1 of the cores fine, bringing it back online freezes the machine for 3-4 seconds and then reboots. carl:/sys/devices/system/cpu/cpu1 # echo 0 online carl:/sys/devices/system/cpu/cpu1 # dmesg Breaking affinity for irq 219 CPU 1 is now offline SMP alternatives: switching to UP code carl:/sys/devices/system/cpu/cpu1 # echo 1 online Read from remote host carl: Connection reset by peer Booting with maxcpus=1 and resume works fine. Does this ring a bell with anyone? With highmem enabled and the standard vmsplit, cpu hotplug works fine for me. Some more clues - booting with noreplacement doesn't fix it, so I think the alternatives code is off the hook. I don't think this adds any new information, but it has been open awhile: http://bugme.osdl.org/show_bug.cgi?id=6542 I was able to narrow it down to the vmsplit setting but I wasn't able to debug it further. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CPU hotplug broken with 2GB VMSPLIT
Nathan Lynch wrote: Jens Axboe wrote: On Thu, Nov 30 2006, Jens Axboe wrote: Hi, Just got a new notebook (Lenovo X60), setup a custom kernel and then I noticed that suspend to ram doesn't work anymore. The machine suspends just fine, on resume it brings back the text display but reboots after it has stalled for a few seconds. On the suggestion of Pavel, I tried testing CPU hotplug, and indeed he was right: I can offline 1 of the cores fine, bringing it back online freezes the machine for 3-4 seconds and then reboots. carl:/sys/devices/system/cpu/cpu1 # echo 0 online carl:/sys/devices/system/cpu/cpu1 # dmesg Breaking affinity for irq 219 CPU 1 is now offline SMP alternatives: switching to UP code carl:/sys/devices/system/cpu/cpu1 # echo 1 online Read from remote host carl: Connection reset by peer Booting with maxcpus=1 and resume works fine. Does this ring a bell with anyone? With highmem enabled and the standard vmsplit, cpu hotplug works fine for me. Some more clues - booting with noreplacement doesn't fix it, so I think the alternatives code is off the hook. I don't think this adds any new information, but it has been open awhile: http://bugme.osdl.org/show_bug.cgi?id=6542 I was able to narrow it down to the vmsplit setting but I wasn't able to debug it further. Hmm, I'm pretty sure this is the same problem I reported in March, there might be some more information in that thread: http://marc.theaimsgroup.com/?t=11403936312r=1w=1 but I didn't realize it was vmsplit-related at that time. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c
Greg KH wrote: On Fri, Jan 25, 2008 at 01:10:48PM -0600, Nathan Lynch wrote: Jan-Bernd Themann wrote: On Thursday 10 January 2008 18:34, Greg KH wrote: The structure device_driver(in device.h) has a member struct driver_private which contains the member kobj (according to drivers/base/base.h). But in device.h struct driver_private has been declared localy and neither defined nor included from base.h. So my effort to use driver-driver_private-obj also does not work. (I am surprised from where do you access the struct device_driver) That is because a driver should not be accessing such a field. And especially not in this manner, why would this driver be creating a symlink that has already been created by the driver core? This whole thing can just be removed with no problems. Can you try just removing the ehea_driver_sysfs_add and ehea_driver_sysfs_remove functions to verify this as I don't have the hardware present to test it out. The eHEA driver tries to orginize its sys-entries as close as possible to other ethernet drivers. Each eHEA NIC has multiple ports which is not that common in PCI. This means that each port is represented by a subdirectory which has not the driver sys-link, only the root directory has. Some tools expect to have this driver link in each port directory. That is the reason why this link is created manually. Are there any other ways to create this link? This is now broken in mainline... drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_add': drivers/net/ehea/ehea_main.c:2812: error: 'struct device_driver' has no member named 'kobj' drivers/net/ehea/ehea_main.c:2815: error: 'struct device_driver' has no member named 'kobj' drivers/net/ehea/ehea_main.c:2818: error: 'struct device_driver' has no member named 'kobj' drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_remove': drivers/net/ehea/ehea_main.c:2830: error: 'struct device_driver' has no member named 'kobj' Does the patch below fix this? That driver should not have been trying to create symlinks that the driver core has already created for it. Yes, it fixes the build error, by just removing the code that got broken. Jan-Bernd gave a rationale for creating the symlink that didn't really seem to be answered. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c
Jan-Bernd Themann wrote: Hi, sorry for answering so late, I'm only tracking netdev and ppc mailing list. On Thursday 10 January 2008 18:34, Greg KH wrote: The structure device_driver(in device.h) has a member struct driver_private which contains the member kobj (according to drivers/base/base.h). But in device.h struct driver_private has been declared localy and neither defined nor included from base.h. So my effort to use driver-driver_private-obj also does not work. (I am surprised from where do you access the struct device_driver) That is because a driver should not be accessing such a field. And especially not in this manner, why would this driver be creating a symlink that has already been created by the driver core? This whole thing can just be removed with no problems. Can you try just removing the ehea_driver_sysfs_add and ehea_driver_sysfs_remove functions to verify this as I don't have the hardware present to test it out. The eHEA driver tries to orginize its sys-entries as close as possible to other ethernet drivers. Each eHEA NIC has multiple ports which is not that common in PCI. This means that each port is represented by a subdirectory which has not the driver sys-link, only the root directory has. Some tools expect to have this driver link in each port directory. That is the reason why this link is created manually. Are there any other ways to create this link? This is now broken in mainline... drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_add': drivers/net/ehea/ehea_main.c:2812: error: 'struct device_driver' has no member named 'kobj' drivers/net/ehea/ehea_main.c:2815: error: 'struct device_driver' has no member named 'kobj' drivers/net/ehea/ehea_main.c:2818: error: 'struct device_driver' has no member named 'kobj' drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_remove': drivers/net/ehea/ehea_main.c:2830: error: 'struct device_driver' has no member named 'kobj' -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] sym53c8xx: fix bad memset argument in sym_set_cam_result_error
On a big powerpc box I got the following oops with 2.6.24-git2: sym0: 1010-66 rev 0x1 at pci :d0:01.0 irq 215 sym0: No NVRAM, ID 7, Fast-80, LVD, parity checking sym0: SCSI BUS has been reset. scsi0 : sym-2.2.3 target0:0:8: FAST-40 WIDE SCSI 80.0 MB/s ST (25 ns, offset 31) scsi 0:0:8:0: Direct-Access IBM ST318305LC C509 PQ: 0 ANSI: 3 target0:0:8: tagged command queuing enabled, command queue depth 16. target0:0:8: Beginning Domain Validation target0:0:8: asynchronous target0:0:8: wide asynchronous target0:0:8: FAST-80 WIDE SCSI 160.0 MB/s DT (12.5 ns, offset 31) target0:0:8: FAST-80 WIDE SCSI 160.0 MB/s DT (12.5 ns, offset 31) Unable to handle kernel paging request for data at address 0x Faulting instruction address: 0xc0038460 cpu 0x25: Vector: 300 (Data Access) at [cf567840] pc: c0038460: .memcpy+0x60/0x280 lr: d0050280: .sym_set_cam_result_error+0xfc/0x1e0 [sym53c8xx] sp: cf567ac0 msr: 80009032 dar: 0 dsisr: 4200 current = 0xc06d1e0af0a0 paca= 0xc04afc00 pid = 0, comm = swapper enter ? for help [link register ] d0050280 .sym_set_cam_result_error+0xfc/0x1e0 [sym53c8xx] [cf567ac0] cf567b80 (unreliable) [cf567b80] d00552b8 .sym_complete_error+0x12c/0x1bc [sym53c8xx] [cf567c20] d00561a4 .sym_int_sir+0xaa4/0x1718 [sym53c8xx] [cf567d00] d0057e8c .sym_interrupt+0x4e4/0x6ec [sym53c8xx] [cf567dc0] d004fdf4 .sym53c8xx_intr+0x6c/0xdc [sym53c8xx] [cf567e50] c00a83e0 .handle_IRQ_event+0x7c/0xec [cf567ef0] c00aa344 .handle_fasteoi_irq+0x130/0x1f0 [cf567f90] c002a538 .call_handle_irq+0x1c/0x2c [c04d5e0b3a90] c000c320 .do_IRQ+0x108/0x1d0 [c04d5e0b3b20] c0004790 hardware_interrupt_entry+0x18/0x1c The memset() in sym_set_cam_result_error() would appear to be trashing the scsi_cmnd struct instead of clearing sense_buffer. Signed-off-by: Nathan Lynch [EMAIL PROTECTED] --- drivers/scsi/sym53c8xx_2/sym_glue.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c index 21e926d..e939f38 100644 --- a/drivers/scsi/sym53c8xx_2/sym_glue.c +++ b/drivers/scsi/sym53c8xx_2/sym_glue.c @@ -207,7 +207,7 @@ void sym_set_cam_result_error(struct sym_hcb *np, struct sym_ccb *cp, int resid) /* * Bounce back the sense data to user. */ - memset(cmd-sense_buffer, 0, SCSI_SENSE_BUFFERSIZE); + memset(cmd-sense_buffer, 0, SCSI_SENSE_BUFFERSIZE); memcpy(cmd-sense_buffer, cp-sns_bbuf, min(SCSI_SENSE_BUFFERSIZE, SYM_SNS_BBUF_LEN)); #if 0 -- 1.5.3.7.1112.g9758e -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.24-rc1 freezes on powerbook at first boot stage
(cc'ing linuxppc-dev, see http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg221770.html for original post and .config) Elimar Riesebieter wrote: On Wed, 24 Oct 2007 the mental interface of Elimar Riesebieter told: [...] The kernel is loaded from firmware but freezes at the moment to load the radeon framebuffer. I can't get any debug info (don't know how?). Screen dump till freeze: Using PowerMac machine description Total memory = 1024MB; using 2048kB for hash table (at cfe0) Linux version 2.6.24-rc1-aragorn ([EMAIL PROTECTED]) (gcc version 4.2.3 20071014 (prelelease) (Debian 4.2.2-3)) #1 Wed Oct 24 12:48:27 CEST 2007 Found UniNorth memory controller host bridge @ 0xf800 revision: 0xd2 Mapped at 0xfdfc Found a Intrepid mac-io controller, rev: 0, mapped at 0xfdf4 PowerMac motherboard: PowerBook G4 15 console [udbg0] enabled setup_arch: bootmem Found UniNorth PCI host bridge at 0xf000. Firmware bus number: 0-1 Found UniNorth PCI host bridge at 0xf200. Firmware bus number: 0-1 Found UniNorth PCI host bridge at 0xf400. Firmware bus number: 0-1 via-pmu: Server Mode is disabled PMU driver v2 initialized for Core99, firmware: 0c nvram: Checking bank 0... nvram: gen0=741, gen1=740 nvram: Active bank is: 0 nvram: OF partition at 0x410 nvram: XP partition at 0x1020 nvram: NR partition at 0x1120 arch: exit Zone PFN ranges: DMA 0 - 196608 Normal 196608 - 196608 HighMem196608 - 262144 Movable zone start PFN for each node early_node_map[1] active PFN ranges 0:0 - 262144 Built 1 zonelists in Zone order. Total pages: 260096 Kernel command line: root=/[EMAIL PROTECTED]/[EMAIL PROTECTED]/[EMAIL PROTECTED]:5 root=/dev/hda5 mpic: Setting up MPIC MPIC 1version 1.2 at 8004, max 4 CPUs mpic: ISU size: 64, shift: 6, mask: 3f mpic: Initializing for 64 sources PID hash table entries: 4096 (order: 12, 16384 bytes) GMT Delta read from XPRAM: 0 minutes, DST: off clocksource: timebase mult[d9038e4] shift [22] registered clockevent: decremeter mult[4b7] shift[16] cpu[0] Console: colour dummy device 80x25 console handover: boot [udbg0] - real [tty0] Does 2.6.23 (or any earlier kernel) work? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: DEFINE_IDR() and the layer cache
Hi Joachim- Joachim Fenkes wrote: idr_init(), when called for the first time, sets up the layer cache. idr_get_new() and friends expect the cache to exist. Now, what happens if the first call to idr_get_new() targets an idr initialized using DEFINE_IDR() before idr_init() has been called at least once? Could this happen? unnamed_dev_init() is the first caller of idr_init() and it happens to be called before the first use of idr_get_new(): [0.831907] [c010615a] dump_stack+0x16/0x18 [0.831945] [c01c2714] idr_init+0x3c/0x9c [0.831985] [c038cab8] unnamed_dev_init+0xd/0xf [0.832027] [c037b987] start_kernel+0x307/0x341 [0.832068] [] 0x0 [0.832149] === [0.832881] Mount-cache hash table entries: 512 [0.833432] [c010615a] dump_stack+0x16/0x18 [0.833470] [c01c298f] idr_get_new_above_int+0x43/0x222 [0.833541] [c01c2b7b] idr_get_new+0xd/0x28 [0.833605] [c0169243] set_anon_super+0x36/0xa0 [0.833667] [c01696be] sget+0x234/0x2c9 [0.833722] [c016a01e] get_sb_single+0x24/0x8e [0.833781] [c01a2468] sysfs_get_sb+0x1c/0x1e [0.833867] [c0169b63] vfs_kern_mount+0x41/0x70 [0.833927] [c0169ba8] kern_mount+0x16/0x18 [0.833984] [c038e494] sysfs_init+0x57/0xa5 [0.834026] [c038cfe3] mnt_init+0xc5/0x1cb [0.834064] [c038cce8] vfs_caches_init+0x141/0x152 [0.834104] [c037b996] start_kernel+0x316/0x341 [0.834144] [] 0x0 [0.834174] === It does seem fragile, though. AFAICT there's no guarantee that a DEFINE_IDR user couldn't call idr_get_new() before unnamed_dev_init() runs. Also, init_id_cache() in idr.c is racy wrt itself. If the first two uses of idr_init() occur concurrently, the code could attempt to create idr_layer_cache twice. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Weird oopses with 2.6.22-rc7
Hello- Christian Kujau wrote: Hi there, Since a few days I'm experiencing weird oopses on my iBook/G4 with ubuntu's 2.6.22-7-powerpc and also with a vanilla 2.6.22-rc7-git8. Now for the weird part: The oops happens when doing make install for a piece of software (xine-ui media player) as *root*. No kidding, it does NOT happen when I compile the software and do make install as a normal user. This is 100% reproducible, here're some technical infos before I go on describing whe I did: http://nerdbynature.de/bits/2.6.22-rc7-git8/ Here's the first oops from the dmesg file: Unrecoverable FP Unavailable Exception 801 at efff20b0 Oops: Unrecoverable FP Unavailable Exception, sig: 6 [#1] PREEMPT PowerMac Modules linked in: NIP: efff20b0 LR: c008dff4 CTR: efff20b0 REGS: ef7cbd90 TRAP: 0801 Not tainted t8) MSR: 9032 EE,ME,IR,DR CR: 28028428 XER: TASK = cfd9b800[1625] 'touch' THREAD: ef7ca000 GPR00: efff20b0 ef7cbe40 cfd9b800 ec049240 0002 ef7cbea0 0ff29604 GPR08: 0001 eff78d00 ef7ca000 28022422 NIP [efff20b0] 0xefff20b0 LR [c008dff4] permission+0x78/0x170 Call Trace: [ef7cbe40] [00010942] 0x10942 (unreliable) [ef7cbe60] [c00ac440] do_utimes+0x1cc/0x1f4 [ef7cbf10] [c00ac630] sys_utimensat+0xac/0x138 [ef7cbf40] [c00118ec] ret_from_syscall+0x0/0x38 Any ideas which magic syscall could cause these oopses? Have you tried 2.6.22? Linus committed a utimensat-related oops fix right before releasing it: http://git.kernel.org/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=1e5de2837c166535f9bb4232bfe97ea1f9fc7a1c - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] remove unused lock_cpu_hotplug_interruptible definition
aa95387774039096c11803c04011f1aa42d85758 removed the implementation of
lock_cpu_hotplug_interruptible and all users of it. This stub
definition for !CONFIG_HOTPLUG_CPU was left over -- kill it now.
Signed-off-by: Nathan Lynch [EMAIL PROTECTED]
---
include/linux/cpu.h |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 3b2df25..c2236bb 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -120,7 +120,6 @@ static inline void cpuhotplug_mutex_unlock(struct mutex
*cpu_hp_mutex)
#define lock_cpu_hotplug() do { } while (0)
#define unlock_cpu_hotplug() do { } while (0)
-#define lock_cpu_hotplug_interruptible() 0
#define hotcpu_notifier(fn, pri) do { (void)(fn); } while (0)
#define register_hotcpu_notifier(nb) do { (void)(nb); } while (0)
#define unregister_hotcpu_notifier(nb) do { (void)(nb); } while (0)
--
1.5.2
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: /sys/devices/system/cpu/*: Present cpus or Possible cpus
Hi Gautham- Gautham R Shenoy wrote: Looking at the topology_init() code, I observe that the meaning of the cpuX/ directory entries in /sys/devices/system/cpu/ might be different for different architectures. Looks like, in case of i386, ia64, m32, mips etc, the cpuX directory entries represent the present cpus. However, in case of powerpc, s390 etc, the cpuX entries represent the possible cpus. Wondering if there is any particular reason for this discrepancy. I believe that the powerpc behavior was established before cpu_present_map was introduced. I am not entirely surely if it's due cpu hotplug because both i386 and powerpc support it! powerpc also supports processor add and remove (as opposed to online/offline); i386 does not AFAIK. I think this may be a reason for the difference. When I do a echo 1 /sys/devices/system/cpu/cpuX/online on a power box as root, I might get -bash: echo: write error: Invalid argument because cpuX might not be present! In case of lpar, cpu_present_map need not necessarily be equal to cpu_possible_map, so the above error is observable. Working as intended. You have to add a cpu to the partition before you can online it. Is this discrepency intentional ? Or is it due to the fact that in most cases, cpu_present_map == cpu_possible_map, so lets not bother about it :-? I think it's the inevitable result when architectures are free to invent their own versions of the same sysfs interface. But is it really causing a problem in this case? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: randomized placement of x86_64 vdso
On 04/23/2014 11:30 AM, H. Peter Anvin wrote:
On 04/21/2014 09:52 AM, Nathan Lynch wrote:
Hi x86/vdso people,
I've been working on adding a vDSO to 32-bit ARM, and Kees suggested I
look at x86_64's algorithm for placing the vDSO at a randomized offset
above the stack VMA. I found that when the stack top occupies the
last slot in the PTE (is that the right term?), the vdso_addr routine
returns an address below mm-start_stack, equivalent to
(mm-start_stack PAGE_MASK). For instance if mm-start_stack is
0x7fff3c96, vdso_addr returns 0x7fff3000.
Since the address returned is always already occupied by the stack,
get_unmapped_area detects the collision and falls back to
vm_unmapped_area. This results in the vdso being placed in the
address space next to libraries etc. While this is generally
unnoticeable and doesn't break anything, it does mean that the vdso is
placed below the stack when there is actually room above the stack.
To me it also seems uncomfortably close to placing the vdso in the way
of downward expansion of the stack.
I don't have a patch because I'm not sure what the algorithm should
be, but thought I would bring it up as vdso_addr doesn't seem to be
behaving as intended in all cases.
If the stack occupies the last possible page, how can you say there is
space above the stack?
Sorry for being unclear. I probably am getting terminology wrong. What
I'm trying to express is that if the stack top is in the last page of
its last-level page table (which may be the last possible page, but
that's not really the interesting case), vdso_addr returns an address
below mm-start_stack.
If you do a lot of execs with the following debug patch applied,
you should see occasional prints like:
got addr 0x7f9a2ba16000, asked 0x7fffa7bff000, start_stack=0x7fffa7bffc96
got addr 0x7f3877ff1000, asked 0x7fffd9bff000, start_stack=0x7fffd9bffc96
got addr 0x7f96e3637000, asked 0x739ff000, start_stack=0x739ffc96
got addr 0x7fb70588d000, asked 0x7fff271ff000, start_stack=0x7fff271ffc96
got addr 0x7f7957171000, asked 0x7fff71dff000, start_stack=0x7fff71dffc96
Hopefully this better illustrates.
diff --git a/arch/x86/vdso/vma.c b/arch/x86/vdso/vma.c
index 1ad102613127..06c51329d1b3 100644
--- a/arch/x86/vdso/vma.c
+++ b/arch/x86/vdso/vma.c
@@ -157,15 +157,17 @@ static int setup_additional_pages(struct linux_binprm
*bprm,
unsigned size)
{
struct mm_struct *mm = current-mm;
- unsigned long addr;
+ unsigned long addr, hint;
int ret;
if (!vdso_enabled)
return 0;
down_write(mm-mmap_sem);
- addr = vdso_addr(mm-start_stack, size);
- addr = get_unmapped_area(NULL, addr, size, 0, 0);
+ hint = vdso_addr(mm-start_stack, size);
+ addr = get_unmapped_area(NULL, hint, size, 0, 0);
+ if (addr != hint)
+ pr_info(got addr 0x%lx, asked 0x%lx\n, addr, hint);
if (IS_ERR_VALUE(addr)) {
ret = addr;
goto up_fail;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
randomized placement of x86_64 vdso
Hi x86/vdso people, I've been working on adding a vDSO to 32-bit ARM, and Kees suggested I look at x86_64's algorithm for placing the vDSO at a randomized offset above the stack VMA. I found that when the stack top occupies the last slot in the PTE (is that the right term?), the vdso_addr routine returns an address below mm-start_stack, equivalent to (mm-start_stack PAGE_MASK). For instance if mm-start_stack is 0x7fff3c96, vdso_addr returns 0x7fff3000. Since the address returned is always already occupied by the stack, get_unmapped_area detects the collision and falls back to vm_unmapped_area. This results in the vdso being placed in the address space next to libraries etc. While this is generally unnoticeable and doesn't break anything, it does mean that the vdso is placed below the stack when there is actually room above the stack. To me it also seems uncomfortably close to placing the vdso in the way of downward expansion of the stack. I don't have a patch because I'm not sure what the algorithm should be, but thought I would bring it up as vdso_addr doesn't seem to be behaving as intended in all cases. Thanks, Nathan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Oops: 17 SMP ARM (v3.16-rc2)
On 06/30/2014 07:30 AM, Fredrik Noring wrote: On Fri, Jun 27, 2014 at 04:16:57PM +, Fredrik Noring wrote: Please find below a trace that appeared once with 3.16-rc2. Perhaps it is of some interest? It's not that serious... I know that the FEC ethernet driver is horrendously racy (I have had a patch set for about the last six months which fixes some of its problems) but as I've had a lot of patches to deal with, and it's been pushed to the back of the queue... The races don't lead to data corruption though, merely timeouts and some lost packets. It seems to be a compiler issue, where (GCC) 4.8.2 does not produce a properly working kernel. Happily, (Fedora 2013.11.24-2.fc19) 4.8.1 appears to do a lot better. No crashes so far with v3.16-rc2! Did you narrow it down to a particular GCC bug? The symptoms you reported remind me of: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854 Sadly, unpatched GCC 4.8.1 and 4.8.2 are unsuitable for building ARM kernels. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation
On 09/22/2014 10:39 AM, Will Deacon wrote: On Thu, Sep 18, 2014 at 03:59:32PM +0100, Nathan Lynch wrote: This series contains the necessary changes to allow architected timer access from user-space on 32-bit ARM. This allows the VDSO to support high resolution timestamps for clock_gettime and gettimeofday. This also merges substantially similar code from arm and arm64 into the core arm_arch_timer driver. The functional changes are: - When available, CNTVCT is made readable by user space on arm, as it is on arm64. - The clocksource name becomes arch_mem_counter if CP15 access to the counter is not available. These changes have been carried as part of the ARM VDSO patch set over the last several months, but I am splitting them out here as I assume they should go through the clocksource maintainers. For the series: Acked-by: Will Deacon will.dea...@arm.com I'm not sure which tree the arch-timer stuff usually goes through, but the arm/arm64 bits look fine so I'm happy for them to merged together. Thanks Will. MAINTAINERS does not have a specific entry for drivers/clocksource/arm_arch_timer.c, so I am working under the assumption that the series would go through Daniel or Thomas. Daniel and/or Thomas, can you take this series please? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation
On 09/22/2014 05:30 PM, Russell King - ARM Linux wrote: On Mon, Sep 22, 2014 at 04:39:19PM +0100, Will Deacon wrote: On Thu, Sep 18, 2014 at 03:59:32PM +0100, Nathan Lynch wrote: This series contains the necessary changes to allow architected timer access from user-space on 32-bit ARM. This allows the VDSO to support high resolution timestamps for clock_gettime and gettimeofday. This also merges substantially similar code from arm and arm64 into the core arm_arch_timer driver. The functional changes are: - When available, CNTVCT is made readable by user space on arm, as it is on arm64. - The clocksource name becomes arch_mem_counter if CP15 access to the counter is not available. These changes have been carried as part of the ARM VDSO patch set over the last several months, but I am splitting them out here as I assume they should go through the clocksource maintainers. For the series: Acked-by: Will Deacon will.dea...@arm.com I'm not sure which tree the arch-timer stuff usually goes through, but the arm/arm64 bits look fine so I'm happy for them to merged together. I raised a while back with Will whether there's much point to having this on ARM. While it's useful for virtualisation, the majority of 32-bit ARM doesn't run virtualised. So there's little point in having the VDSO on the majority of platforms - it will just add additional unnecessary cycles slowing down the system calls that the VDSO is designed to try to speed up. Hmm, this patch set is merely exposing the hardware counter when it is present for the VDSO's use; I take it you have no objection to that? While the 32-bit ARM VDSO I've posted (in a different thread) exploits a facility that is required by the virtualization option in the architecture, its utility is not limited to guest operating systems. So, my view is that this VDSO will only be of very limited use for 32-bit ARM, and should not be exposed to userspace unless there is a reason for it to be exposed (iow, the hardware necessary to support it is present.) My thinking is that it should prove useful in a growing subset of v7 CPUs. It is useful today on Cortex-A15 and -A7, and I believe -A12 and -A17 implement the generic timer facility as well. Now if you're saying that we shouldn't slow down gettimeofday on systems which lack a hardware counter that can be safely exposed to userspace, I can work with that. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clocksource: arch_timer: Allow the device tree to specify the physical timer
On 09/11/2014 10:58 AM, Christopher Covington wrote: On 09/11/2014 11:52 AM, Doug Anderson wrote: diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 5163ec1..8ca07a9 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -649,6 +649,9 @@ static void __init arch_timer_init(struct device_node *np) arch_timer_ppi[i] = irq_of_parse_and_map(np, i); arch_timer_detect_rate(NULL, np); +if (of_property_read_bool(np, arm,use-physical-timer)) +arch_timer_use_virtual = false; + /* * If HYP mode is available, we know that the physical timer * has been configured to be accessible from PL1. Use it, so How's the VDSO supposed to deal with this? It currently does: cycle_now = arch_counter_get_cntvct() Note the VDSO for 32-bit ARM is still out of tree so there's no mainline regression in that respect. I think as long as there's some way for the vdso initialization code to discover that it can't use the virtual counter, it should be tractable. If that involves adding an API to the arch timer code, that can be added along with the vdso code. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation
This series contains the necessary changes to allow architected timer access from user-space on 32-bit ARM. This allows the VDSO to support high resolution timestamps for clock_gettime and gettimeofday. This also merges substantially similar code from arm and arm64 into the core arm_arch_timer driver. The functional changes are: - When available, CNTVCT is made readable by user space on arm, as it is on arm64. - The clocksource name becomes arch_mem_counter if CP15 access to the counter is not available. These changes have been carried as part of the ARM VDSO patch set over the last several months, but I am splitting them out here as I assume they should go through the clocksource maintainers. Changes since v1: - Fix minor checkpatch complaints. - Rework patches 2-4. v1 copied arm64's arch_timer_evtstrm_enable and arch_counter_set_user_access to the driver with slightly different names in one patch, then removed the unused functions in subsequent patches for each of arm and arm64. This seemed kind of awkward to me, so I've reorganized those changes into two patches that should be easier to review. Patch #1 is unchanged. Nathan Lynch (3): clocksource: arm_arch_timer: change clocksource name if CP15 unavailable clocksource: arm_arch_timer: enable counter access for 32-bit ARM clocksource: arm_arch_timer: consolidate arch_timer_evtstrm_enable arch/arm/include/asm/arch_timer.h| 25 arch/arm64/include/asm/arch_timer.h | 31 - drivers/clocksource/arm_arch_timer.c | 44 ++-- 3 files changed, 42 insertions(+), 58 deletions(-) -- 1.9.3 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/3] clocksource: arm_arch_timer: enable counter access for 32-bit ARM
The only difference between arm and arm64's implementations of
arch_counter_set_user_access is that 32-bit ARM does not enable user
access to the virtual counter. We want to enable this access for the
32-bit ARM VDSO, so copy the arm64 version to the driver itself, and
remove the arch-specific implementations.
Signed-off-by: Nathan Lynch nathan_ly...@mentor.com
---
arch/arm/include/asm/arch_timer.h| 14 --
arch/arm64/include/asm/arch_timer.h | 17 -
drivers/clocksource/arm_arch_timer.c | 17 +
3 files changed, 17 insertions(+), 31 deletions(-)
diff --git a/arch/arm/include/asm/arch_timer.h
b/arch/arm/include/asm/arch_timer.h
index 0704e0cf5571..b7ae44464231 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -99,20 +99,6 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
asm volatile(mcr p15, 0, %0, c14, c1, 0 : : r (cntkctl));
}
-static inline void arch_counter_set_user_access(void)
-{
- u32 cntkctl = arch_timer_get_cntkctl();
-
- /* Disable user access to both physical/virtual counters/timers */
- /* Also disable virtual event stream */
- cntkctl = ~(ARCH_TIMER_USR_PT_ACCESS_EN
- | ARCH_TIMER_USR_VT_ACCESS_EN
- | ARCH_TIMER_VIRT_EVT_EN
- | ARCH_TIMER_USR_VCT_ACCESS_EN
- | ARCH_TIMER_USR_PCT_ACCESS_EN);
- arch_timer_set_cntkctl(cntkctl);
-}
-
static inline void arch_timer_evtstrm_enable(int divider)
{
u32 cntkctl = arch_timer_get_cntkctl();
diff --git a/arch/arm64/include/asm/arch_timer.h
b/arch/arm64/include/asm/arch_timer.h
index 9400596a0f39..49e94c677e7a 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -104,23 +104,6 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
asm volatile(msr cntkctl_el1, %0 : : r (cntkctl));
}
-static inline void arch_counter_set_user_access(void)
-{
- u32 cntkctl = arch_timer_get_cntkctl();
-
- /* Disable user access to the timers and the physical counter */
- /* Also disable virtual event stream */
- cntkctl = ~(ARCH_TIMER_USR_PT_ACCESS_EN
- | ARCH_TIMER_USR_VT_ACCESS_EN
- | ARCH_TIMER_VIRT_EVT_EN
- | ARCH_TIMER_USR_PCT_ACCESS_EN);
-
- /* Enable user access to the virtual counter */
- cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
-
- arch_timer_set_cntkctl(cntkctl);
-}
-
static inline void arch_timer_evtstrm_enable(int divider)
{
u32 cntkctl = arch_timer_get_cntkctl();
diff --git a/drivers/clocksource/arm_arch_timer.c
b/drivers/clocksource/arm_arch_timer.c
index c99afdf12e98..31781fdd7c19 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -312,6 +312,23 @@ static void arch_timer_configure_evtstream(void)
arch_timer_evtstrm_enable(min(pos, 15));
}
+static void arch_counter_set_user_access(void)
+{
+ u32 cntkctl = arch_timer_get_cntkctl();
+
+ /* Disable user access to the timers and the physical counter */
+ /* Also disable virtual event stream */
+ cntkctl = ~(ARCH_TIMER_USR_PT_ACCESS_EN
+ | ARCH_TIMER_USR_VT_ACCESS_EN
+ | ARCH_TIMER_VIRT_EVT_EN
+ | ARCH_TIMER_USR_PCT_ACCESS_EN);
+
+ /* Enable user access to the virtual counter */
+ cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
+
+ arch_timer_set_cntkctl(cntkctl);
+}
+
static int arch_timer_setup(struct clock_event_device *clk)
{
__arch_timer_setup(ARCH_CP15_TIMER, clk);
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable
The arm and arm64 VDSOs need CP15 access to the architected counter.
If this is unavailable (which is allowed by ARM v7), indicate this by
changing the clocksource name to arch_mem_counter before registering
the clocksource.
Suggested by Stephen Boyd.
Signed-off-by: Nathan Lynch nathan_ly...@mentor.com
Reviewed-by: Stephen Boyd sb...@codeaurora.org
---
drivers/clocksource/arm_arch_timer.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c
b/drivers/clocksource/arm_arch_timer.c
index 5163ec13429d..c99afdf12e98 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -429,11 +429,19 @@ static void __init arch_counter_register(unsigned type)
u64 start_count;
/* Register the CP15 based counter if we have one */
- if (type ARCH_CP15_TIMER)
+ if (type ARCH_CP15_TIMER) {
arch_timer_read_counter = arch_counter_get_cntvct;
- else
+ } else {
arch_timer_read_counter = arch_counter_get_cntvct_mem;
+ /* If the clocksource name is arch_sys_counter the
+* VDSO will attempt to read the CP15-based counter.
+* Ensure this does not happen when CP15-based
+* counter is not available.
+*/
+ clocksource_counter.name = arch_mem_counter;
+ }
+
start_count = arch_timer_read_counter();
clocksource_register_hz(clocksource_counter, arch_timer_rate);
cyclecounter.mult = clocksource_counter.mult;
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 3/3] clocksource: arm_arch_timer: consolidate arch_timer_evtstrm_enable
The arch_timer_evtstrm_enable hooks in arm and arm64 are substantially
similar, the only difference being a CONFIG_COMPAT-conditional section
which is relevant only for arm64. Copy the arm64 version to the
driver, removing the arch-specific hooks.
Signed-off-by: Nathan Lynch nathan_ly...@mentor.com
---
arch/arm/include/asm/arch_timer.h| 11 ---
arch/arm64/include/asm/arch_timer.h | 14 --
drivers/clocksource/arm_arch_timer.c | 15 +++
3 files changed, 15 insertions(+), 25 deletions(-)
diff --git a/arch/arm/include/asm/arch_timer.h
b/arch/arm/include/asm/arch_timer.h
index b7ae44464231..92793ba69c40 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -99,17 +99,6 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
asm volatile(mcr p15, 0, %0, c14, c1, 0 : : r (cntkctl));
}
-static inline void arch_timer_evtstrm_enable(int divider)
-{
- u32 cntkctl = arch_timer_get_cntkctl();
- cntkctl = ~ARCH_TIMER_EVT_TRIGGER_MASK;
- /* Set the divider and enable virtual event stream */
- cntkctl |= (divider ARCH_TIMER_EVT_TRIGGER_SHIFT)
- | ARCH_TIMER_VIRT_EVT_EN;
- arch_timer_set_cntkctl(cntkctl);
- elf_hwcap |= HWCAP_EVTSTRM;
-}
-
#endif
#endif
diff --git a/arch/arm64/include/asm/arch_timer.h
b/arch/arm64/include/asm/arch_timer.h
index 49e94c677e7a..f19097134b02 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -104,20 +104,6 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
asm volatile(msr cntkctl_el1, %0 : : r (cntkctl));
}
-static inline void arch_timer_evtstrm_enable(int divider)
-{
- u32 cntkctl = arch_timer_get_cntkctl();
- cntkctl = ~ARCH_TIMER_EVT_TRIGGER_MASK;
- /* Set the divider and enable virtual event stream */
- cntkctl |= (divider ARCH_TIMER_EVT_TRIGGER_SHIFT)
- | ARCH_TIMER_VIRT_EVT_EN;
- arch_timer_set_cntkctl(cntkctl);
- elf_hwcap |= HWCAP_EVTSTRM;
-#ifdef CONFIG_COMPAT
- compat_elf_hwcap |= COMPAT_HWCAP_EVTSTRM;
-#endif
-}
-
static inline u64 arch_counter_get_cntvct(void)
{
u64 cval;
diff --git a/drivers/clocksource/arm_arch_timer.c
b/drivers/clocksource/arm_arch_timer.c
index 31781fdd7c19..ce5f99e957b9 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -299,6 +299,21 @@ static void __arch_timer_setup(unsigned type,
clockevents_config_and_register(clk, arch_timer_rate, 0xf, 0x7fff);
}
+static void arch_timer_evtstrm_enable(int divider)
+{
+ u32 cntkctl = arch_timer_get_cntkctl();
+
+ cntkctl = ~ARCH_TIMER_EVT_TRIGGER_MASK;
+ /* Set the divider and enable virtual event stream */
+ cntkctl |= (divider ARCH_TIMER_EVT_TRIGGER_SHIFT)
+ | ARCH_TIMER_VIRT_EVT_EN;
+ arch_timer_set_cntkctl(cntkctl);
+ elf_hwcap |= HWCAP_EVTSTRM;
+#ifdef CONFIG_COMPAT
+ compat_elf_hwcap |= COMPAT_HWCAP_EVTSTRM;
+#endif
+}
+
static void arch_timer_configure_evtstream(void)
{
int evt_stream_div, pos;
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] clocksource: arm_arch_timer: change clocksource name if CP15 unavailable
Hi Daniel, On 09/26/2014 02:04 AM, Daniel Lezcano wrote: On 09/18/2014 04:59 PM, Nathan Lynch wrote: The arm and arm64 VDSOs need CP15 access to the architected counter. If this is unavailable (which is allowed by ARM v7), indicate this by changing the clocksource name to arch_mem_counter before registering the clocksource. Suggested by Stephen Boyd. Signed-off-by: Nathan Lynch nathan_ly...@mentor.com Reviewed-by: Stephen Boyd sb...@codeaurora.org Hi Nathan, is it possible to have the acked-by of the different maintainers for the other patches ? As Will said, he acked the series. Are these patches targeted to be merged for 3.18 ? 3.18 is preferable, if it's not too late. But nothing will break if it needs to wait. Thanks, Nathan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation
On 09/24/2014 09:12 AM, Christopher Covington wrote: Hi Nathan, On 09/22/2014 08:28 PM, Nathan Lynch wrote: On 09/22/2014 05:30 PM, Russell King - ARM Linux wrote: On Mon, Sep 22, 2014 at 04:39:19PM +0100, Will Deacon wrote: On Thu, Sep 18, 2014 at 03:59:32PM +0100, Nathan Lynch wrote: This series contains the necessary changes to allow architected timer access from user-space on 32-bit ARM. This allows the VDSO to support high resolution timestamps for clock_gettime and gettimeofday. This also merges substantially similar code from arm and arm64 into the core arm_arch_timer driver. The functional changes are: - When available, CNTVCT is made readable by user space on arm, as it is on arm64. - The clocksource name becomes arch_mem_counter if CP15 access to the counter is not available. These changes have been carried as part of the ARM VDSO patch set over the last several months, but I am splitting them out here as I assume they should go through the clocksource maintainers. For the series: Acked-by: Will Deacon will.dea...@arm.com I'm not sure which tree the arch-timer stuff usually goes through, but the arm/arm64 bits look fine so I'm happy for them to merged together. I raised a while back with Will whether there's much point to having this on ARM. While it's useful for virtualisation, the majority of 32-bit ARM doesn't run virtualised. So there's little point in having the VDSO on the majority of platforms - it will just add additional unnecessary cycles slowing down the system calls that the VDSO is designed to try to speed up. Hmm, this patch set is merely exposing the hardware counter when it is present for the VDSO's use; I take it you have no objection to that? While the 32-bit ARM VDSO I've posted (in a different thread) exploits a facility that is required by the virtualization option in the architecture, its utility is not limited to guest operating systems. Just to clarify, were the performance improvements you measured from a virtualized guest or native? Yeah I should have been explicit about this. My tests and measurements (and all test results I've received from others, I believe) have been on native/host kernels, not guests. So, my view is that this VDSO will only be of very limited use for 32-bit ARM, and should not be exposed to userspace unless there is a reason for it to be exposed (iow, the hardware necessary to support it is present.) My thinking is that it should prove useful in a growing subset of v7 CPUs. It is useful today on Cortex-A15 and -A7, and I believe -A12 and -A17 implement the generic timer facility as well. I count 18 dts* files that have arm,armv7-timer, including platforms with Krait, Exynos, and Tegra processors. Yup. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/3] arm_arch_timer: VDSO preparation, code consolidation
On 09/24/2014 09:50 AM, Russell King - ARM Linux wrote: On Wed, Sep 24, 2014 at 09:32:54AM -0500, Nathan Lynch wrote: On 09/24/2014 09:12 AM, Christopher Covington wrote: Hi Nathan, On 09/22/2014 08:28 PM, Nathan Lynch wrote: Hmm, this patch set is merely exposing the hardware counter when it is present for the VDSO's use; I take it you have no objection to that? While the 32-bit ARM VDSO I've posted (in a different thread) exploits a facility that is required by the virtualization option in the architecture, its utility is not limited to guest operating systems. Just to clarify, were the performance improvements you measured from a virtualized guest or native? Yeah I should have been explicit about this. My tests and measurements (and all test results I've received from others, I believe) have been on native/host kernels, not guests. Have there been any measurements on systems without the architected timers? I do test on iMX6 regularly. Afraid I don't have any pre-v7 hardware to check though. Here's a report from you from an earlier submission that shows little/no impact: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/267552.html But admittedly vdsotest is just doing rudimentary microbenchmarking. Running a lttng-ust workload that emits tracepoints as fast as possible (lttng-ust calls clock_gettime and getcpu on every tracepoint), I see about 1% degradation on iMX6. I count 18 dts* files that have arm,armv7-timer, including platforms with Krait, Exynos, and Tegra processors. Yup. That's not the full story. Almost every ARM to date has not had an architected timer. Architected timers are a recent addition - as pointed out, a Cortex A7/A12/A15 invention. Most of the platforms I see are Cortex A9 which doesn't have any architected timers. Yes, it may be fun to work on new hardware and make that perform much better than previous, but we should not loose sight that there is older hardware out there, and we shouldn't unnecessarily penalise it when adding new features. Agreed, of course, and I'll include more detailed results from systems without the architected timer in future submissions. What we /need/ to know is what the effect providing a VDSO in an environment without an architected timer (so using the VDSO fallback functions calling the syscalls) and having glibc use it is compared to the current situation where there is no VDSO for glibc to use. If you can show that there's no difference, then I'm happy to go with always providing the VDSO. If there's a detrimental effect (which I suspect there may be, since we now have to have glibc test to see if the VDSO is there, jump to the VDSO, the VDSO then tests whether we have an architected timer, and then we finally get to issue the syscall), then we must avoid providing the VDSO on systems which have no architected timer. One point I would like to raise is that the VDSO provides (or could be made to provide) acceleration for APIs that are unrelated to the architected timer: - clock_gettime with CLOCK_REALTIME_COARSE and CLOCK_MONOTONIC_COARSE. This is currently included. - getcpu, which I had planned on submitting later. I don't know whether the coarse clock support is compelling; they don't seem to be commonly used. But there is a nice 4-5x speedup for those on iMX6. getcpu, on the other hand, is one of the two system calls lttng-ust uses in every tracepoint emitted, and I would like to have it available in the VDSO on all systems capable of supporting the implementation, which may take the form of co-opting TPIDRURW or some other register. So the question of whether to provide the VDSO may not hinge on whether the architected timer is available. None of which is to argue that unnecessarily degrading gettimeofday performance on some systems for the benefit of others is acceptable. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tracing/syscalls: ignore numbers outside NR_syscalls' range
On 10/30/2014 06:35 AM, Russell King - ARM Linux wrote: On Thu, Oct 30, 2014 at 07:30:28AM -0400, Steven Rostedt wrote: On Thu, 30 Oct 2014 11:14:41 + Russell King - ARM Linux li...@arm.linux.org.uk wrote: We have always had syscall number range of 0x90 or so. The tracing design does not expect that. Therefore, the tracing design did not take account of ARM when it was created. Therefore, it's up to the tracing people to decide how to properly fit their ill-designed subsystem into one of the popular and well-established kernel architectures - or at least suggest a way to work around this issue. Fine, lets define a MAX_SYSCALL_NR that is by default NR_syscalls, but an architecture can override it. In trace_syscalls.c, where the checks are done, have this: #ifndef MAX_SYSCALL_NR # define MAX_SYSCALL_NR NR_syscalls #endif change all the checks to test against MAX_SYSCALL_NR instead of NR_syscalls. Then in arch/arm/include/asm/syscall.h have: #define MAX_SYSCALL_NR 0xa0 or whatever would be the highest syscall number for ARM. Or do we just ignore the high special ARM syscalls and treat them (from the tracing point of view) as non-syscalls, avoiding the allocation of something around 1.2MB for the syscall bitmap. I really don't know, I don't use any of this tracing stuff, so it isn't something I care about. Maybe those who do use the facility should have an input here? I checked strace and it knows about ARM's high syscalls. I wouldn't want to go from casually using strace to digging deeper with ftrace only to get the impression that syscalls are disappearing. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RCU bug with v3.17-rc3 ?
On 10/10/2014 11:25 AM, Russell King - ARM Linux wrote:
Right, so GCC 4.8.{1,2} are totally unsuitable for kernel building (and
it seems that this has been known about for some time.)
Looking at http://gcc.gnu.org/PR58854 it seems that all 4.8.x for x 3
are affected, as well as 4.9.0.
We can blacklist these GCC versions quite easily. We already have GCC
3.3 blacklisted, and it's trivial to add others. I would want to include
some proper details about the bug, just like the other existing entries
we already have in asm-offsets.c, where we name the functions that the
compiler is known to break where appropriate.
Before blacklisting anything, it's worth considering that simple version
checks would break existing pre-4.8.3 compilers that have been patched
for PR58854. It looks like Yocto and Buildroot issued releases with
patched 4.8.2 compilers well before the (fixed) 4.8.3 release. I think
the most we can reasonably do without breaking some correctly-behaving
toolchains is to emit a warning.
Hopefully nobody's still using gcc 4.8 from the Linaro 2013.11 toolchain
release -- since it's a 4.8.3 prerelease from before the fix was
committed you'll get GCC_VERSION == 40803 but still generate bad code.
However, I'm rather annoyed that there are people here who have known
for some time that GCC 4.8.1 and GCC 4.8.2 _can_ lead to filesystem
corruption, and have sat on their backsides doing nothing about getting
it blacklisted for something like a year.
Mea culpa, although I hadn't drawn the connection to FS corruption
reports until now. I have known about the issue for some time, but
figured the prevalence of the fix in downstream projects largely
mitigated the issue.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: RCU bug with v3.17-rc3 ?
On 10/10/2014 08:44 PM, Nathan Lynch wrote:
On 10/10/2014 11:25 AM, Russell King - ARM Linux wrote:
Right, so GCC 4.8.{1,2} are totally unsuitable for kernel building (and
it seems that this has been known about for some time.)
Looking at http://gcc.gnu.org/PR58854 it seems that all 4.8.x for x 3
are affected, as well as 4.9.0.
Correction -- 4.9.0 has this fixed, even though the GCC PR shows it as a
known to fail version.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] remoteproc: microblaze: Add support for microblaze on Zynq
On 01/19/2015 04:30 AM, Michal Simek wrote: diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index 5e343bab9458..3955f42e9e9c 100644 --- a/drivers/remoteproc/Kconfig +++ b/drivers/remoteproc/Kconfig @@ -64,4 +64,15 @@ config DA8XX_REMOTEPROC It's safe to say n here if you're not interested in multimedia offloading. +config MB_REMOTEPROC + tristate Support Microblaze remoteproc + depends on ARCH_ZYNQ !DEBUG_SG + select GPIO_XILINX + select REMOTEPROC + select RPMSG + default m + help + Say y here to support Xilinx Microblaze remote processors + on the Xilinx Zynq. + endmenu The dependency on !DEBUG_SG seems unusual, and worth a comment if it's truly needed. And the module should be disabled by default, no? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the tip tree
On 03/30/2015 03:08 AM, Stephen Rothwell wrote: Hi Russell, On Mon, 30 Mar 2015 08:15:37 +0100 Russell King - ARM Linux li...@arm.linux.org.uk wrote: I'll drop the VDSO stuff from the ARM tree; I can't see a way to keep it in my tree and keep my tree buildable without dragging in the tip tree. Does it affect your tree on its own? If not, then it can be fixed when merged as I have done, or if you look at the tip tree, all you really need to merge is tip timers/core branch (which I am sure the tip guys can tell you if it is stable enough) which is about 28 commits ... The ARM VDSO stuff will just have to wait for 4.2 instead. If that works for you. FWIW, Stephen's merge fix is correct and I have run my vdso tests without problems on OMAP5 with next-20150330. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the tip tree
On 03/30/2015 10:08 AM, Russell King - ARM Linux wrote: On Mon, Mar 30, 2015 at 09:57:48AM -0500, Nathan Lynch wrote: On 03/30/2015 03:08 AM, Stephen Rothwell wrote: Hi Russell, On Mon, 30 Mar 2015 08:15:37 +0100 Russell King - ARM Linux li...@arm.linux.org.uk wrote: I'll drop the VDSO stuff from the ARM tree; I can't see a way to keep it in my tree and keep my tree buildable without dragging in the tip tree. Does it affect your tree on its own? If not, then it can be fixed when merged as I have done, or if you look at the tip tree, all you really need to merge is tip timers/core branch (which I am sure the tip guys can tell you if it is stable enough) which is about 28 commits ... The ARM VDSO stuff will just have to wait for 4.2 instead. If that works for you. FWIW, Stephen's merge fix is correct and I have run my vdso tests without problems on OMAP5 with next-20150330. Hopefully, I can pull the tip stuff but if not, I'll try to remember to include Stephen's patch with my pull request, but I can't make any guarantees - Stephen's email will very quickly get buried in my mailbox, and I'll most likely forget about it too... I'm notoriously bad with email... OK, let me know if I can help. I'm happy to remind you about it when the merge window opens. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] clocksource: arm_arch_timer: add arch_timer_okay_for_vdso
The 32-bit ARM VDSO needs to know whether a generic timer is present
and whether it is suitable for use by user space. The VDSO
initialization code currently duplicates some of the logic from the
driver to make this determination, but unfortunately it is incomplete;
it will incorrectly enable the VDSO if HYP mode is available or if no
interrupt is provided for the virtual timer (see arch_timer_init). In
these cases the driver will switch to memory-backed access while the
VDSO will attempt to access the counter using cp15 reads.
Add an arch_timer_okay_for_vdso API which can reliably inform the VDSO
init code whether the arch timer is present and usable.
Signed-off-by: Nathan Lynch nathan_ly...@mentor.com
---
drivers/clocksource/arm_arch_timer.c | 12
include/clocksource/arm_arch_timer.h | 6 ++
2 files changed, 18 insertions(+)
diff --git a/drivers/clocksource/arm_arch_timer.c
b/drivers/clocksource/arm_arch_timer.c
index 0aa135ddbf80..b75215523d2f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -462,6 +462,18 @@ struct timecounter *arch_timer_get_timecounter(void)
return timecounter;
}
+/* The ARM VDSO init code needs to know:
+ * - whether a cp15-based arch timer is present; and if so
+ * - whether the physical or virtual counter is being used.
+ */
+bool arch_timer_okay_for_vdso(void)
+{
+ if (!(arch_timers_present ARCH_CP15_TIMER))
+ return false;
+
+ return arch_timer_use_virtual;
+}
+
static void __init arch_counter_register(unsigned type)
{
u64 start_count;
diff --git a/include/clocksource/arm_arch_timer.h
b/include/clocksource/arm_arch_timer.h
index 9916d0e4eff5..bfc1e95280c4 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -48,6 +48,7 @@ enum arch_timer_reg {
extern u32 arch_timer_get_rate(void);
extern u64 (*arch_timer_read_counter)(void);
extern struct timecounter *arch_timer_get_timecounter(void);
+extern bool arch_timer_okay_for_vdso(void);
#else
@@ -66,6 +67,11 @@ static inline struct timecounter
*arch_timer_get_timecounter(void)
return NULL;
}
+static inline bool arch_timer_okay_for_vdso(void)
+{
+ return false;
+}
+
#endif
#endif
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] ARM: VDSO: use arch_timer_okay_for_vdso
Use the facility now provided by the arm_arch_timer driver to
determine whether there's a usable virtual counter for the VDSO.
Signed-off-by: Nathan Lynch nathan_ly...@mentor.com
---
arch/arm/kernel/vdso.c | 30 +-
1 file changed, 1 insertion(+), 29 deletions(-)
diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
index efe17dd9b921..f06fd6f3f65f 100644
--- a/arch/arm/kernel/vdso.c
+++ b/arch/arm/kernel/vdso.c
@@ -21,7 +21,6 @@
#include linux/err.h
#include linux/kernel.h
#include linux/mm.h
-#include linux/of.h
#include linux/printk.h
#include linux/slab.h
#include linux/timekeeper_internal.h
@@ -69,33 +68,6 @@ struct elfinfo {
*/
static bool cntvct_ok __read_mostly;
-static bool __init cntvct_functional(void)
-{
- struct device_node *np;
- bool ret = false;
-
- if (!IS_ENABLED(CONFIG_ARM_ARCH_TIMER))
- goto out;
-
- /* The arm_arch_timer core should export
-* arch_timer_use_virtual or similar so we don't have to do
-* this.
-*/
- np = of_find_compatible_node(NULL, NULL, arm,armv7-timer);
- if (!np)
- goto out_put;
-
- if (of_property_read_bool(np, arm,cpu-registers-not-fw-configured))
- goto out_put;
-
- ret = true;
-
-out_put:
- of_node_put(np);
-out:
- return ret;
-}
-
static void * __init find_section(Elf32_Ehdr *ehdr, const char *name,
unsigned long *size)
{
@@ -208,7 +180,7 @@ static int __init vdso_init(void)
vdso_total_pages = 1; /* for the data/vvar page */
vdso_total_pages += text_pages;
- cntvct_ok = cntvct_functional();
+ cntvct_ok = arch_timer_okay_for_vdso();
patch_vdso(vdso_start);
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] clocksource: arm_arch_timer: add arch_timer_okay_for_vdso
On 04/27/2015 05:55 AM, Will Deacon wrote:
On Fri, Apr 24, 2015 at 10:43:20PM +0100, Nathan Lynch wrote:
The 32-bit ARM VDSO needs to know whether a generic timer is present
and whether it is suitable for use by user space. The VDSO
initialization code currently duplicates some of the logic from the
driver to make this determination, but unfortunately it is incomplete;
it will incorrectly enable the VDSO if HYP mode is available or if no
interrupt is provided for the virtual timer (see arch_timer_init). In
these cases the driver will switch to memory-backed access while the
VDSO will attempt to access the counter using cp15 reads.
Add an arch_timer_okay_for_vdso API which can reliably inform the VDSO
init code whether the arch timer is present and usable.
Signed-off-by: Nathan Lynch nathan_ly...@mentor.com
---
drivers/clocksource/arm_arch_timer.c | 12
include/clocksource/arm_arch_timer.h | 6 ++
2 files changed, 18 insertions(+)
diff --git a/drivers/clocksource/arm_arch_timer.c
b/drivers/clocksource/arm_arch_timer.c
index 0aa135ddbf80..b75215523d2f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -462,6 +462,18 @@ struct timecounter *arch_timer_get_timecounter(void)
return timecounter;
}
+/* The ARM VDSO init code needs to know:
+ * - whether a cp15-based arch timer is present; and if so
+ * - whether the physical or virtual counter is being used.
+ */
+bool arch_timer_okay_for_vdso(void)
+{
+if (!(arch_timers_present ARCH_CP15_TIMER))
+return false;
+
+return arch_timer_use_virtual;
+}
If we're adding this, then it wouldn't hurt to use the same check in
arch/arm64 when we update_vsyscall(...). Could we also encapsulate the
`current clocksource' knowledge in there too, to remove the hardcoded
arch_sys_counter check from the arch code?
While I think it makes sense to consolidate the current clocksource check,
I view that as distinct from this (which needs to run at boot, before
anything uses the vdso).
I'm actually now unsure about whether the implementation I have here
is correct. Take arch_timer_init:
static void __init arch_timer_init(void)
{
/*
* If HYP mode is available, we know that the physical timer
* has been configured to be accessible from PL1. Use it, so
* that a guest can use the virtual timer instead.
*
* If no interrupt provided for virtual timer, we'll have to
* stick to the physical timer. It'd better be accessible...
*/
if (is_hyp_mode_available() || !arch_timer_ppi[VIRT_PPI]) {
arch_timer_use_virtual = false;
if (!arch_timer_ppi[PHYS_SECURE_PPI] ||
!arch_timer_ppi[PHYS_NONSECURE_PPI]) {
pr_warn(arch_timer: No interrupt available, giving
up\n);
return;
}
}
arch_timer_register();
arch_timer_common_init();
}
I assume this has been working fine for arm64 up to this point --
i.e. arch_timer_use_virtual is false, but the VDSO continues to use
the virtual counter and gets correct results? If so, I don't see any
reason this wouldn't be true for ARM. So I'm thinking
arch_timer_use_virtual isn't the right proxy for determining
whether the VDSO can work correctly.
The reason I initially turned to arch_timer_use_virtual is in
arch_timer_of_init:
/*
* If we cannot rely on firmware initializing the timer registers then
* we should use the physical timers instead.
*/
if (IS_ENABLED(CONFIG_ARM)
of_property_read_bool(np, arm,cpu-registers-not-fw-configured))
arch_timer_use_virtual = false;
I definitely need to catch that case, and this is currently duplicated in
arch/arm/kernel/vdso.c. Maybe this should toggle an additional boolean,
say arch_timer_cntvct_ok, which captures the information that is truly
of interest with respect to the VDSO using the virtual counter.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: vdso: .gitignore: ignore generated vdso.so.raw and vdsomunge
On 04/28/2015 03:32 PM, Andrey Skvortsov wrote: After kernel is built 'git status' reports: # Untracked files: # (use git add file... to include in what will be committed) # #arch/arm/vdso/vdso.so.raw #arch/arm/vdso/vdsomunge These files are created during build process and removed with 'make clean'. This patch makes git to ignore them. Thanks, but this should be fixed already by: 2b507a2d9c7f ARM: 8343/1: VDSO: add build artifacts to .gitignore -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] ARM: VDSO: fix coarse clock monotonicity regression
Since 906c55579a63 (timekeeping: Copy the shadow-timekeeper over the
real timekeeper last) it has become possible on ARM to:
- Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
via syscall.
- Subsequently obtain a timestamp for the same clock ID via VDSO which
predates the first timestamp (by one jiffy).
This is because ARM's update_vsyscall is deriving the coarse time
using the __current_kernel_time interface, when it should really be
using the timekeeper object provided to it by the timekeeping core.
It happened to work before only because __current_kernel_time would
access the same timekeeper object which had been passed to
update_vsyscall. This is no longer the case.
Signed-off-by: Nathan Lynch nathan_ly...@mentor.com
---
arch/arm/kernel/vdso.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
index efe17dd9b921..c8b243c1aef8 100644
--- a/arch/arm/kernel/vdso.c
+++ b/arch/arm/kernel/vdso.c
@@ -296,7 +296,6 @@ static bool tk_is_cntvct(const struct timekeeper *tk)
*/
void update_vsyscall(struct timekeeper *tk)
{
- struct timespec xtime_coarse;
struct timespec64 *wtm = tk-wall_to_monotonic;
if (!cntvct_ok) {
@@ -308,10 +307,10 @@ void update_vsyscall(struct timekeeper *tk)
vdso_write_begin(vdso_data);
- xtime_coarse = __current_kernel_time();
vdso_data-tk_is_cntvct = tk_is_cntvct(tk);
- vdso_data-xtime_coarse_sec = xtime_coarse.tv_sec;
- vdso_data-xtime_coarse_nsec= xtime_coarse.tv_nsec;
+ vdso_data-xtime_coarse_sec = tk-xtime_sec;
+ vdso_data-xtime_coarse_nsec= tk-tkr_mono.xtime_nsec
+ tk-tkr_mono.shift;
vdso_data-wtm_clock_sec= wtm-tv_sec;
vdso_data-wtm_clock_nsec = wtm-tv_nsec;
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] fix vdso coarse clock monotonicity regressions
Commit 906c55579a63 (timekeeping: Copy the shadow-timekeeper over the real timekeeper last) made it so the user can observe the coarse clocks going backwards on arm and arm64, if they're really looking for it. Technically these are fixing regressions versus 4.1, but I won't be bothered if they don't make 4.2 final at this late stage, since only the (seldom-used?) coarse clocks are affected. I'd like to collect review/acks for these now and make sure they at least make it into 4.3-rc1 (and -stable after that). Nathan Lynch (2): ARM: VDSO: fix coarse clock monotonicity regression arm64: VDSO: fix coarse clock monotonicity regression arch/arm/kernel/vdso.c | 7 +++ arch/arm64/kernel/vdso.c | 7 +++ 2 files changed, 6 insertions(+), 8 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] arm64: VDSO: fix coarse clock monotonicity regression
Since 906c55579a63 (timekeeping: Copy the shadow-timekeeper over the
real timekeeper last) it has become possible on arm64 to:
- Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
via syscall.
- Subsequently obtain a timestamp for the same clock ID via VDSO which
predates the first timestamp (by one jiffy).
This is because arm64's update_vsyscall is deriving the coarse time
using the __current_kernel_time interface, when it should really be
using the timekeeper object provided to it by the timekeeping core.
It happened to work before only because __current_kernel_time would
access the same timekeeper object which had been passed to
update_vsyscall. This is no longer the case.
Signed-off-by: Nathan Lynch nathan_ly...@mentor.com
---
arch/arm64/kernel/vdso.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index ec37ab3f524f..97bc68f4c689 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -199,16 +199,15 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
*/
void update_vsyscall(struct timekeeper *tk)
{
- struct timespec xtime_coarse;
u32 use_syscall = strcmp(tk-tkr_mono.clock-name, arch_sys_counter);
++vdso_data-tb_seq_count;
smp_wmb();
- xtime_coarse = __current_kernel_time();
vdso_data-use_syscall = use_syscall;
- vdso_data-xtime_coarse_sec = xtime_coarse.tv_sec;
- vdso_data-xtime_coarse_nsec= xtime_coarse.tv_nsec;
+ vdso_data-xtime_coarse_sec = tk-xtime_sec;
+ vdso_data-xtime_coarse_nsec= tk-tkr_mono.xtime_nsec
+ tk-tkr_mono.shift;
vdso_data-wtm_clock_sec= tk-wall_to_monotonic.tv_sec;
vdso_data-wtm_clock_nsec = tk-wall_to_monotonic.tv_nsec;
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/vdso: Emit a GNU hash
On 08/06/2015 05:52 PM, Andy Lutomirski wrote: [adding lots of cc's] On Thu, Aug 6, 2015 at 2:45 PM, Andy Lutomirski l...@kernel.org wrote: From: Andy Lutomirski l...@amacapital.net Some dynamic loaders may be slightly faster if a GNU hash is available. Strangely, this seems to have no effect at all on the vdso size. FWIW, I see arch/x86/entry/vdso/vdso64.so increase by 168 bytes here, using GCC 4.9.2, Binutils 2.24 as packaged by Fedora 21. I see a similar increase when I make the equivalent change for ARM. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] ARM: VDSO: fix coarse clock monotonicity regression
Since 906c55579a63 (timekeeping: Copy the shadow-timekeeper over the
real timekeeper last) it has become possible on ARM to:
- Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
via syscall.
- Subsequently obtain a timestamp for the same clock ID via VDSO which
predates the first timestamp (by one jiffy).
This is because ARM's update_vsyscall is deriving the coarse time
using the __current_kernel_time interface, when it should really be
using the timekeeper object provided to it by the timekeeping core.
It happened to work before only because __current_kernel_time would
access the same timekeeper object which had been passed to
update_vsyscall. This is no longer the case.
Signed-off-by: Nathan Lynch nathan_ly...@mentor.com
---
Changes since v1:
- Add u32 cast to nsec calculation.
arch/arm/kernel/vdso.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
index efe17dd9b921..54a5aeab988d 100644
--- a/arch/arm/kernel/vdso.c
+++ b/arch/arm/kernel/vdso.c
@@ -296,7 +296,6 @@ static bool tk_is_cntvct(const struct timekeeper *tk)
*/
void update_vsyscall(struct timekeeper *tk)
{
- struct timespec xtime_coarse;
struct timespec64 *wtm = tk-wall_to_monotonic;
if (!cntvct_ok) {
@@ -308,10 +307,10 @@ void update_vsyscall(struct timekeeper *tk)
vdso_write_begin(vdso_data);
- xtime_coarse = __current_kernel_time();
vdso_data-tk_is_cntvct = tk_is_cntvct(tk);
- vdso_data-xtime_coarse_sec = xtime_coarse.tv_sec;
- vdso_data-xtime_coarse_nsec= xtime_coarse.tv_nsec;
+ vdso_data-xtime_coarse_sec = tk-xtime_sec;
+ vdso_data-xtime_coarse_nsec= (u32)(tk-tkr_mono.xtime_nsec
+ tk-tkr_mono.shift);
vdso_data-wtm_clock_sec= wtm-tv_sec;
vdso_data-wtm_clock_nsec = wtm-tv_nsec;
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/5] ARM: dts: mt8135: enable basic SMP bringup for mt8135
On 07/14/2015 12:18 AM, Yingjoe Chen wrote:
Add arch timer node to enable arch-timer support. MT8135 firmware
doesn't correctly setup arch-timer frequency and CNTVOFF, add
properties to workaround this.
[...]
+ timer {
+ compatible = arm,armv7-timer;
+ interrupt-parent = gic;
+ interrupts = GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW),
+ GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW),
+ GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW),
+ GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW);
+ clock-frequency = 1300;
+ arm,cpu-registers-not-fw-configured;
It's disappointing to see this property in new DTS submissions. It
prevents taking advantage of the VDSO for gettimeofday/clock_gettime.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h
On 10/14/2015 07:47 AM, H. Nikolaus Schaller wrote:
>> diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
>> index aedec81..27a9a0b 100644
>> --- a/arch/arm/vdso/vdsomunge.c
>> +++ b/arch/arm/vdso/vdsomunge.c
>> @@ -45,7 +45,18 @@
>> * it does.
>> */
>>
>> -#include
>> +#define swab16(x) \
>> +((unsigned short)( \
>> +(((unsigned short)(x) & (unsigned short)0x00ffU) << 8) | \
>> +(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))
>> +
>> +#define swab32(x) \
>> +((unsigned int)( \
>> +(((unsigned int)(x) & (unsigned int)0x00ffUL) << 24) | \
>> +(((unsigned int)(x) & (unsigned int)0xff00UL) << 8) | \
>> +(((unsigned int)(x) & (unsigned int)0x00ffUL) >> 8) | \
>> +(((unsigned int)(x) & (unsigned int)0xff00UL) >> 24) ))
>> +
>> #include
>> #include
>> #include
>> @@ -104,17 +115,17 @@ static void cleanup(void)
>>
>> static Elf32_Word read_elf_word(Elf32_Word word, bool swap)
>> {
>> -return swap ? bswap_32(word) : word;
>> +return swap ? swab32(word) : word;
>> }
>>
>> static Elf32_Half read_elf_half(Elf32_Half half, bool swap)
>> {
>> -return swap ? bswap_16(half) : half;
>> +return swap ? swab16(half) : half;
>> }
>>
>> static void write_elf_word(Elf32_Word val, Elf32_Word *dst, bool swap)
>> {
>> -*dst = swap ? bswap_32(val) : val;
>> +*dst = swap ? swab32(val) : val;
>> }
> ping.
Sorry for the delay.
This is okay but I'd prefer the swab macros to be below the #include
lines, and it would be easier for me to deal with a patch that isn't
whitespace-damaged.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h
On 10/15/2015 12:52 AM, H. Nikolaus Schaller wrote:
> Am 14.10.2015 um 16:16 schrieb Nathan Lynch <nathan_ly...@mentor.com>:
>> and it would be easier for me to deal with a patch that isn't
>> whitespace-damaged.
>
> You mean:
>
> ERROR: space prohibited before that close parenthesis ')'
> #46: FILE: arch/arm/vdso/vdsomunge.c:64:
> + (((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))
>
> ERROR: space prohibited before that close parenthesis ')'
> #53: FILE: arch/arm/vdso/vdsomunge.c:71:
> + (((unsigned int)(x) & (unsigned int)0xff00UL) >> 24) ))
>
> Well, I had copied that verbatim from arch/mips/boot/elf2ecoff.c and
> thought that it is better readable, but it is easy to fix.
That's not what I was referring to, but it is fine to fix that too.
What I meant was that the first column of the patch body is corrupted.
Your v2 has hunks like this (bad):
static Elf32_Word read_elf_word(Elf32_Word word, bool swap)
{
- return swap ? bswap_32(word) : word;
+ return swap ? swab32(word) : word;
}
Your v3 has this (good):
static void write_elf_word(Elf32_Word val, Elf32_Word *dst, bool swap)
{
- *dst = swap ? bswap_32(val) : val;
+ *dst = swap ? swab32(val) : val;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h
On 10/15/2015 12:16 PM, H. Nikolaus Schaller wrote: > Does this mean it is ok now in V3 in all cases? I have switched my patch > editor > tool from indirect git imap-send to git send-email (which appears to be more > compatible). v3 applied without issue, so yes, in that respect it is fine. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] ARM: fix vdsomunge not to depend on glibc specific byteswap.h
On 10/16/2015 01:38 AM, H. Nikolaus Schaller wrote:
> diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
> index aedec81..0cebd98 100644
> --- a/arch/arm/vdso/vdsomunge.c
> +++ b/arch/arm/vdso/vdsomunge.c
> @@ -45,7 +45,6 @@
> * it does.
> */
>
> -#include
> #include
> #include
> #include
> @@ -59,6 +58,16 @@
> #include
> #include
>
> +#define swab16(x) \
> + x) & 0x00ff) << 8) | \
> + (((x) & 0xff00) >> 8))
> +
> +#define swab32(x) \
> + x) & 0x00ff) << 24) | \
> + (((x) & 0xff00) << 8) | \
> + (((x) & 0x00ff) >> 8) | \
> + (((x) & 0xff00) << 24))
> +
> #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> #define HOST_ORDER ELFDATA2LSB
> #elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> @@ -104,17 +113,17 @@ static void cleanup(void)
>
> static Elf32_Word read_elf_word(Elf32_Word word, bool swap)
> {
> - return swap ? bswap_32(word) : word;
> + return swap ? swab32(word) : word;
> }
>
> static Elf32_Half read_elf_half(Elf32_Half half, bool swap)
> {
> - return swap ? bswap_16(half) : half;
> + return swap ? swab16(half) : half;
> }
>
> static void write_elf_word(Elf32_Word val, Elf32_Word *dst, bool swap)
> {
> - *dst = swap ? bswap_32(val) : val;
> + *dst = swap ? swab32(val) : val;
> }
I think this looks good now and it checks out in my local testing.
Thanks for your persistence.
Assuming Russell has no further comments I'll put this in his patch
system later today.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] remoteproc: dt: Provide bindings for ST's Remote Processor Controller driver
On 08/26/2015 08:08 AM, Lee Jones wrote: diff --git a/Documentation/devicetree/bindings/remoteproc/st-rproc.txt b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt new file mode 100644 index 000..6a933c1 --- /dev/null +++ b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt @@ -0,0 +1,38 @@ +STMicroelectronics Remote Processor +--- + +This binding provides support for adjunct processors found on ST SoCs. + +The remote processors can be controlled from the bootloader or the primary OS. +If the bootloader starts a remote processor processor the primary OS must detect +its state and act accordingly. + +The node name is significant, as it defines the name of the CPU and the name +of the firmware to load: rproc-name-fw. This business about the firmware name describes a behavior of Linux's core remoteproc code and seems out of place in a binding document. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] remoteproc: debugfs: Add ability to boot remote processor using debugfs
On 08/26/2015 08:08 AM, Lee Jones wrote:
Signed-off-by: Lee Jones lee.jo...@linaro.org
---
drivers/remoteproc/remoteproc_debugfs.c | 25 +
1 file changed, 25 insertions(+)
The commit message should describe why this is needed...
diff --git a/drivers/remoteproc/remoteproc_debugfs.c
b/drivers/remoteproc/remoteproc_debugfs.c
index 9d30809..9620962 100644
--- a/drivers/remoteproc/remoteproc_debugfs.c
+++ b/drivers/remoteproc/remoteproc_debugfs.c
@@ -88,8 +88,33 @@ static ssize_t rproc_state_read(struct file *filp, char
__user *userbuf,
return simple_read_from_buffer(userbuf, count, ppos, buf, i);
}
+static ssize_t rproc_state_write(struct file *filp, const char __user
*userbuf,
+ size_t count, loff_t *ppos)
+{
+ struct rproc *rproc = filp-private_data;
+ char buf[2];
+ int ret;
+
+ ret = copy_from_user(buf, userbuf, 1);
+ if (ret)
+ return -EFAULT;
+
+ switch (buf[0]) {
+ case '1':
+ ret = rproc_boot(rproc);
+ if (ret)
+ dev_warn(rproc-dev, Boot failed: %d\n, ret);
+ break;
+ default:
+ rproc_shutdown(rproc);
+ }
+
+ return count;
+}
... and I suggest that the user interface be reconsidered. If '1' means
boot and literally anything else means shut down then you can't add
operations in the future without potentially breaking things.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] remoteproc: Supply controller driver for ST's Remote Processors
On 08/26/2015 08:08 AM, Lee Jones wrote:
--- /dev/null
+++ b/drivers/remoteproc/st_remoteproc.c
@@ -0,0 +1,300 @@
+/*
+ * ST's Remote Processor Control Driver
+ *
+ * Copyright (C) 2015 STMicroelectronics - All Rights Reserved
+ *
+ * Author: Ludovic Barre ludovic.ba...@st.com
When submitting code you didn't write, I'd say it's better practice to
clearly indicate its provenance in the commit message. E.g. something
like Driver based on code authored by Ludovic Barre for ST. And
obtain signoffs etc if possible.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
Please review the wording here. It's unclear whether this is intended
to be v2-only or v2 or later.
+static int st_rproc_stop(struct rproc *rproc)
+{
+ struct st_rproc *st_rproc = rproc-priv;
+ int err = 0;
+
+ if (st_rproc-config-sw_reset) {
+ err = reset_control_assert(st_rproc-sw_reset);
+ if (err)
+ dev_warn(rproc-dev, Failed to assert S/W Reset\n);
+ }
+
+ if (st_rproc-config-pwr_reset) {
+ err = reset_control_assert(st_rproc-pwr_reset);
+ if (err)
+ dev_warn(rproc-dev, Failed to assert Power Reset\n);
+ }
+
+ clk_disable(st_rproc-clk);
+
+ return 0;
+}
Seems like st_rproc_stop should propagate errors back to its caller
instead of always returning 0.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] soc: ti: reset irq affinity before freeing irq
On 08/27/2015 10:51 AM, Murali Karicheri wrote:
When using accumulator queue for rx side for network driver, following
warning is seen when doing a reboot command from Linux console. This
is because, affinity value is not reset before calling free_irq(). This
patch fixes this.
Deconfiguring network interfaces...
[ 55.176589] [ cut here ]---
[ 55.181232] WARNING: CPU: 0 PID: 2081 at kernel/irq/manage.c:1370
__free_irq+0x208/0x214
The full content of the warning should be included in the commit
message; __free_irq has several potential sources of warning messages,
and line 1370 doesn't correspond to any of them in 4.2-rc8.
Signed-off-by: Murali Karicheri m-kariche...@ti.com
---
- Applies to v4.2.0-rc8
drivers/soc/ti/knav_qmss_acc.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/soc/ti/knav_qmss_acc.c b/drivers/soc/ti/knav_qmss_acc.c
index ef6f69d..b98fe56 100644
--- a/drivers/soc/ti/knav_qmss_acc.c
+++ b/drivers/soc/ti/knav_qmss_acc.c
@@ -261,6 +261,10 @@ static int knav_range_setup_acc_irq(struct
knav_range_info *range,
if (old !new) {
dev_dbg(kdev-dev, setup-acc-irq: freeing %s for channel %s\n,
acc-name, acc-name);
+ ret = irq_set_affinity_hint(irq, NULL);
+ if (ret)
+ dev_warn(range-kdev-dev,
+ Failed to set IRQ affinity\n);
Seems unnecessary to add a warning here.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] soc: ti: reset irq affinity before freeing irq
On 08/27/2015 01:23 PM, Murali Karicheri wrote: On 08/27/2015 12:43 PM, Nathan Lynch wrote: On 08/27/2015 10:51 AM, Murali Karicheri wrote: When using accumulator queue for rx side for network driver, following warning is seen when doing a reboot command from Linux console. This is because, affinity value is not reset before calling free_irq(). This patch fixes this. Deconfiguring network interfaces... [ 55.176589] [ cut here ]--- [ 55.181232] WARNING: CPU: 0 PID: 2081 at kernel/irq/manage.c:1370 __free_irq+0x208/0x214 The full content of the warning should be included in the commit message; __free_irq has several potential sources of warning messages, and line 1370 doesn't correspond to any of them in 4.2-rc8. The log corresponds to 4.1.x kernel. Corresponding WARN_ONCE is #ifdef CONFIG_SMP /* make sure affinity_hint is cleaned up */ if (WARN_ON_ONCE(desc-affinity_hint)) desc-affinity_hint = NULL; #endif Which is line 1922. I can edit to make it 1922. Does that work? Eh, I guess I wasn't thinking clearly about this clearly when I wrote that. (I somehow had the notion that WARN_ON... maybe printed more than just the file and line number, but that is clearly mistaken.) I think your message is fine as is, sorry for the noise. I don't think changing the line number will make it any easier on future readers. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 4/4] remoteproc: debugfs: Add ability to boot remote processor using debugfs
On 08/28/2015 05:31 AM, Lee Jones wrote:
+static ssize_t rproc_state_write(struct file *filp, const char __user
*userbuf,
+ size_t count, loff_t *ppos)
+{
+ struct rproc *rproc = filp-private_data;
+ char buf[2];
+ int ret;
+
+ ret = copy_from_user(buf, userbuf, 1);
+ if (ret)
+ return -EFAULT;
+
+ switch (buf[0]) {
+ case '0':
+ rproc_shutdown(rproc);
+ break;
+ case '1':
+ ret = rproc_boot(rproc);
+ if (ret)
+ dev_warn(rproc-dev, Boot failed: %d\n, ret);
+ break;
+ default:
+ dev_err(rproc-dev, Unrecognised option: %x\n, buf[1]);
+ return -EINVAL;
This prints uninitialized kernel stack contents instead of what was
copied from user space.
Is the dev_err statement really necessary anyway?
+ }
+
+ return count;
+}
If rproc_boot fails, that should be reflected in the syscall result.
This interface is essentially exposing the remoteproc-power refcount to
user space; is that okay? Seems like it makes it easy to underflow
remoteproc-power through successive shutdown calls.
The other debugfs interface in remoteproc that has a write method
(recovery) accepts more expressive string commands as opposed to 0/1.
It would be more consistent for this interface to take commands such as
boot and shutdown IMO.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/4] remoteproc: Supply controller driver for ST's Remote Processors
On 08/28/2015 05:31 AM, Lee Jones wrote:
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 28c711f..72e97d7 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -77,4 +77,13 @@ config DA8XX_REMOTEPROC
It's safe to say n here if you're not interested in multimedia
offloading.
+config ST_REMOTEPROC
+ tristate ST remoteproc support
+ depends on ARCH_STI
+ select REMOTEPROC
+ help
+ Say y here to support ST's adjunct processors via the remote
+ processor framework.
+ This can be either built-in or a loadable module.
+
The code uses reset_control_* APIs, so this should depend on
RESET_CONTROLLER, no?
+/*
+ * ST's Remote Processor Control Driver
+ *
+ * Copyright (C) 2015 STMicroelectronics - All Rights Reserved
+ *
+ * Author: Ludovic Barre ludovic.ba...@st.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ */
OK, but:
+MODULE_LICENSE(GPL v2);
These are not in agreement. You want GPL for MODULE_LICENSE if you
intend v2 or later.
+static int st_rproc_stop(struct rproc *rproc)
+{
+ struct st_rproc *st_rproc = rproc-priv;
+ int ret, err = 0;
+
+ if (st_rproc-config-sw_reset) {
+ ret = reset_control_assert(st_rproc-sw_reset);
+ if (ret)
+ dev_err(rproc-dev, Failed to assert S/W Reset\n);
+ }
+
+ if (st_rproc-config-pwr_reset) {
+ err = reset_control_assert(st_rproc-pwr_reset);
+ if (err)
+ dev_err(rproc-dev, Failed to assert Power Reset\n);
+ }
+
+ clk_disable(st_rproc-clk);
+
+ return ret ?: err;
+}
Sorry, but I think this is a stylistically inadequate response to my
earlier comments. At least name the status variables sw_ret and pwr_ret
or something. And it looks like ret could be used uninitialized.
Also, do you want to unconditionally call clk_disable even if you've
encountered errors?
+static int st_rproc_start(struct rproc *rproc)
+{
+ struct st_rproc *st_rproc = rproc-priv;
+ int err;
+
+ regmap_update_bits(st_rproc-boot_base, st_rproc-boot_offset,
+st_rproc-config-bootaddr_mask, rproc-bootaddr);
+
+ err = clk_enable(st_rproc-clk);
+ if (err) {
+ dev_err(rproc-dev, Failed to enable clock\n);
+ return err;
+ }
+
+ if (st_rproc-config-sw_reset) {
+ err = reset_control_deassert(st_rproc-sw_reset);
+ if (err) {
+ dev_err(rproc-dev, Failed to deassert S/W Reset\n);
+ return err;
+ }
+ }
+
+ if (st_rproc-config-pwr_reset) {
+ err = reset_control_deassert(st_rproc-pwr_reset);
+ if (err) {
+ dev_err(rproc-dev, Failed to deassert Power
Reset\n);
+ return err;
+ }
+ }
+
+ dev_info(rproc-dev, Started from 0x%x\n, rproc-bootaddr);
+
+ return 0;
+}
Does this want to unwind any of its operations if it encounters a failure?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
