* Re: [PATCH] Add support for the ESTeem 195E (PPC405EP) SBC
From: Josh Boyer @ 2009-07-31 2:38 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, Solomon Peachy
In-Reply-To: <20090731004524.GA10243@yookeroo.seuss>
On Fri, Jul 31, 2009 at 10:45:24AM +1000, David Gibson wrote:
>On Thu, Jul 30, 2009 at 04:08:49PM -0400, Josh Boyer wrote:
>> On Thu, Jul 30, 2009 at 03:45:06PM -0400, Solomon Peachy wrote:
>> >On Thu, Jul 30, 2009 at 10:06:30AM -0400, Josh Boyer wrote:
>[snip]
>> >> >+ UART0: serial@ef600400 {
>> >> >+ device_type = "serial";
>> >> >+ compatible = "ns16550";
>> >> >+ reg = <0xef600400 0x00000008>;
>> >> >+ virtual-reg = <0xef600400>;
>> >> >+ clock-frequency = <0>; /* Filled in by zImage */
>> >> >+ current-speed = <0x9600>;
>> >>
>> >> Just a question, but is the baud supposed to be 38400 or 9600? At first glance
>> >> it almost seems like a typo :).
>> >
>> >It's supposed to be 38400 baud, hence the explicit 0x in front. (I lost
>> >count of the number of times I saw '38400' listed in various dts
>> >files...)
>>
>> Cool. Just checking.
>
>Um.. except, surely it's clearer to just list 38400 in decimal, rather
>than 0x9600 which people are very likely to misread as 9600.
That was my point, yes. It is clearer, yes. It's his board though, so I'm
not going to nit-pick about it.
josh
^ permalink raw reply
* Re: [PATCH 3/20] powerpc/mm: Add HW threads support to no_hash TLB management
From: Kumar Gala @ 2009-07-31 3:12 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <20090724091523.8AD8CDDD1B@ozlabs.org>
On Jul 24, 2009, at 4:15 AM, Benjamin Herrenschmidt wrote:
> The current "no hash" MMU context management code is written with
> the assumption that one CPU == one TLB. This is not the case on
> implementations that support HW multithreading, where several
> linux CPUs can share the same TLB.
>
> This adds some basic support for this to our context management
> and our TLB flushing code.
>
> It also cleans up the optional debugging output a bit
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
I'm getting this nice oops on 32-bit book-e SMP (and I'm guessing its
because of this patch)
Unable to handle kernel paging request for data at address 0x00000000
Faulting instruction address: 0xc0016dac
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=8 MPC8572 DS
Modules linked in:
NIP: c0016dac LR: c0016d58 CTR: 0000001e
REGS: eed77ce0 TRAP: 0300 Not tainted (2.6.31-rc4-00442-gdb4c9c5)
MSR: 00021000 <ME,CE> CR: 24288482 XER: 20000000
DEAR: 00000000, ESR: 00000000
TASK = eecfe140[1581] 'msgctl08' THREAD: eed76000 CPU: 0
GPR00: 00400000 eed77d90 eecfe140 00000000 00000000 00000001 c05bf074
c05c0cf4
GPR08: 00000003 00000002 ff7fffff 00000000 00009b05 1004f894 c05bdd24
00000001
GPR16: ffffffff c05ab890 c05c0ce8 c04e0f58 c04da364 c05c0000 00000000
c04cfa04
GPR24: 00000002 00000000 00000000 c05c0cd8 00000080 00000000 ef056380
00000017
NIP [c0016dac] switch_mmu_context+0x15c/0x520
LR [c0016d58] switch_mmu_context+0x108/0x520
Call Trace:
[eed77d90] [c0016d58] switch_mmu_context+0x108/0x520 (unreliable)
[eed77df0] [c040efec] schedule+0x2bc/0x800
[eed77e70] [c01b9268] do_msgrcv+0x198/0x420
[eed77ef0] [c01b9520] sys_msgrcv+0x30/0xa0
[eed77f10] [c0003fe8] sys_ipc+0x1a8/0x2c0
[eed77f40] [c00116c4] ret_from_syscall+0x0/0x3c
Instruction dump:
57402834 7c00f850 3920fffe 5d2a003e 397b0010 5500103a 7ceb0214 60000000
60000000 81670000 39080001 38e70004 <7c0be82e> 7c005038 7c0be92e
81260000
---[ end trace 3c4c3106446e1bd8 ]---
- k
^ permalink raw reply
* Re: [PATCH 3/20] powerpc/mm: Add HW threads support to no_hash TLB management
From: Kumar Gala @ 2009-07-31 3:35 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <2E027F3C-8FAA-42EC-99B2-9B7EC470094E@kernel.crashing.org>
On Jul 30, 2009, at 10:12 PM, Kumar Gala wrote:
>
> On Jul 24, 2009, at 4:15 AM, Benjamin Herrenschmidt wrote:
>
>> The current "no hash" MMU context management code is written with
>> the assumption that one CPU == one TLB. This is not the case on
>> implementations that support HW multithreading, where several
>> linux CPUs can share the same TLB.
>>
>> This adds some basic support for this to our context management
>> and our TLB flushing code.
>>
>> It also cleans up the optional debugging output a bit
>>
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> ---
>
> I'm getting this nice oops on 32-bit book-e SMP (and I'm guessing
> its because of this patch)
>
> Unable to handle kernel paging request for data at address 0x00000000
> Faulting instruction address: 0xc0016dac
> Oops: Kernel access of bad area, sig: 11 [#1]
> SMP NR_CPUS=8 MPC8572 DS
> Modules linked in:
> NIP: c0016dac LR: c0016d58 CTR: 0000001e
> REGS: eed77ce0 TRAP: 0300 Not tainted (2.6.31-rc4-00442-gdb4c9c5)
> MSR: 00021000 <ME,CE> CR: 24288482 XER: 20000000
> DEAR: 00000000, ESR: 00000000
> TASK = eecfe140[1581] 'msgctl08' THREAD: eed76000 CPU: 0
> GPR00: 00400000 eed77d90 eecfe140 00000000 00000000 00000001
> c05bf074 c05c0cf4
> GPR08: 00000003 00000002 ff7fffff 00000000 00009b05 1004f894
> c05bdd24 00000001
> GPR16: ffffffff c05ab890 c05c0ce8 c04e0f58 c04da364 c05c0000
> 00000000 c04cfa04
> GPR24: 00000002 00000000 00000000 c05c0cd8 00000080 00000000
> ef056380 00000017
> NIP [c0016dac] switch_mmu_context+0x15c/0x520
> LR [c0016d58] switch_mmu_context+0x108/0x520
> Call Trace:
> [eed77d90] [c0016d58] switch_mmu_context+0x108/0x520 (unreliable)
> [eed77df0] [c040efec] schedule+0x2bc/0x800
> [eed77e70] [c01b9268] do_msgrcv+0x198/0x420
> [eed77ef0] [c01b9520] sys_msgrcv+0x30/0xa0
> [eed77f10] [c0003fe8] sys_ipc+0x1a8/0x2c0
> [eed77f40] [c00116c4] ret_from_syscall+0x0/0x3c
> Instruction dump:
> 57402834 7c00f850 3920fffe 5d2a003e 397b0010 5500103a 7ceb0214
> 60000000
> 60000000 81670000 39080001 38e70004 <7c0be82e> 7c005038 7c0be92e
> 81260000
> ---[ end trace 3c4c3106446e1bd8 ]---
On Jul 24, 2009, at 4:15 AM, Benjamin Herrenschmidt wrote:
> @@ -247,15 +261,20 @@ void switch_mmu_context(struct mm_struct
> * local TLB for it and unmark it before we use it
> */
> if (test_bit(id, stale_map[cpu])) {
> - pr_devel("[%d] flushing stale context %d for mm @%p !\n",
> - cpu, id, next);
> + pr_hardcont(" | stale flush %d [%d..%d]",
> + id, cpu_first_thread_in_core(cpu),
> + cpu_last_thread_in_core(cpu));
> +
> local_flush_tlb_mm(next);
>
> /* XXX This clear should ultimately be part of local_flush_tlb_mm */
> - __clear_bit(id, stale_map[cpu]);
> + for (cpu = cpu_first_thread_in_core(cpu);
> + cpu <= cpu_last_thread_in_core(cpu); cpu++)
> + __clear_bit(id, stale_map[cpu]);
> }
This looks a bit dodgy. using 'cpu' as both the loop variable and
what you are computing to determine loop start/end..
Changing this to:
unsigned int i;
...
for (i = cpu_first_thread_in_core(cpu);
i <= cpu_last_thread_in_core(cpu); i++)
__clear_bit(id, stale_map[i]);
seems to clear up the oops.
- k
^ permalink raw reply
* Re: [Patch 0/6] [Patch 0/6] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver VIII
From: David Gibson @ 2009-07-31 6:10 UTC (permalink / raw)
To: K.Prasad
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, paulus,
Alan Stern, Roland McGrath
In-Reply-To: <20090727001152.GA13562@in.ibm.com>
On Mon, Jul 27, 2009 at 05:41:52AM +0530, K.Prasad wrote:
> Hi David,
> I'm back with a new version of patches after a brief hiatus!
>
> After much deliberation about modifying the code to change the timing of signal
> delivery to user-space, it has been decided to retain the existing behaviour
> i.e. SIGTRAP delivered to user-space after execution of causative instruction
> although exception is raised before execution of it.
Ok. Except, presumably for ptrace, since changing that would break
gdb.
> One-shot behaviour will now be restricted only to ptrace
> requests. Kernel-space and non-ptrace user-space requests will
> result in persistent breakpoints.
Ok.
> Reasons
> --------
> - Signal delivery before execution of instruction requires complex workarounds
> - One of the plausible workarounds is a two-pass hw-breakpoint handler which
> delivers the signal after the first pass (with the breakpoints enabled).
> In the second pass, it follows the existing semantics of
> disable_hbp-->enable_ss-->single_step-->disable_ss-->enable_hbp.
Yes, that's the only way I can see to do it.
> - Possibility of nested exceptions is a problem here.
Ok, why?
> - Proper identification of a second-pass of first exception and a new nested
> exception is difficult. Possibility of stray exceptions due to accesses in
> neighbouring memory regions of the breakpoint address further complicates it.
> - Alternatives are i)use one-shot for all user-space requests ii)disable signal
> delivery for non-ptrace requests, allow the user-defined callback routine to
> generate signal.
> - Using one-shot for all user-space requests will break the register/unregister
> interface semantics.
> - Disabling signal delivery for non-ptrace requests is one of the options
> but will be a digression from x86 behaviour, or would require changes in x86
> code too. Even user-defined callback routines cannot deliver signal
> before instruction execution.
>
> Considering all the above, we propose a behaviour that delivers the signal to
> user-space after breakpoint execution. In due course, it will be good to have
> ptrace on PPC64 follow the same behaviour.
Um.. except we can't change ptrace semantics in this way. It could
break existing users.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [Patch 2/6] Introduce PPC64 specific Hardware Breakpoint interfaces
From: David Gibson @ 2009-07-31 6:16 UTC (permalink / raw)
To: K.Prasad
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, paulus,
Alan Stern, Roland McGrath
In-Reply-To: <20090727001317.GC13562@in.ibm.com>
On Mon, Jul 27, 2009 at 05:43:17AM +0530, K.Prasad wrote:
> Introduce PPC64 implementation for the generic hardware breakpoint interfaces
> defined in kernel/hw_breakpoint.c. Enable the HAVE_HW_BREAKPOINT flag and the
> Makefile.
[snip]
> +/*
> + * Handle debug exception notifications.
> + */
> +int __kprobes hw_breakpoint_handler(struct die_args *args)
> +{
> + int rc = NOTIFY_STOP;
> + struct hw_breakpoint *bp;
> + struct pt_regs *regs = args->regs;
> + unsigned long dar = regs->dar;
> + int cpu, is_kernel, stepped = 1;
> +
> + is_kernel = (hbp_kernel_pos == HBP_NUM) ? 0 : 1;
> +
> + /* Disable breakpoints during exception handling */
> + set_dabr(0);
> +
> + cpu = get_cpu();
> + /* Determine whether kernel- or user-space address is the trigger */
> + bp = is_kernel ?
> + per_cpu(this_hbp_kernel[0], cpu) : current->thread.hbp[0];
> + /*
> + * bp can be NULL due to lazy debug register switching
> + * or due to the delay between updates of hbp_kernel_pos
> + * and this_hbp_kernel.
> + */
> + if (!bp)
> + goto out;
> +
> + per_cpu(dabr_data, cpu) = is_kernel ? kdabr : current->thread.dabr;
> +
> + /* Verify if dar lies within the address range occupied by the symbol
> + * being watched. Since we cannot get the symbol size for
> + * user-space requests we skip this check in that case
> + */
> + if (is_kernel &&
> + !((bp->info.address <= dar) &&
> + (dar <= (bp->info.address + bp->info.symbolsize))))
> + /*
> + * This exception is triggered not because of a memory access on
> + * the monitored variable but in the double-word address range
> + * in which it is contained. We will consume this exception,
> + * considering it as 'noise'.
> + */
> + goto out;
> +
> + (bp->triggered)(bp, regs);
It bothers me that the trigger function is executed before the
trapping instruction, but the SIGTRAP occurs afterwards. Since
they're both responses to the trap, it seems logical to me that they
should occur at the same time (from the trapping program's point of
view, at least).
> + /*
> + * Return early without restoring DABR if the breakpoint is from
> + * user-space which always operates in one-shot mode
> + */
> + if (!is_kernel) {
> + rc = NOTIFY_DONE;
> + goto out;
> + }
> +
> + stepped = emulate_step(regs, regs->nip);
> + /*
> + * Single-step the causative instruction manually if
> + * emulate_step() could not execute it
> + */
> + if (stepped == 0) {
> + regs->msr |= MSR_SE;
> + goto out;
> + }
> + set_dabr(per_cpu(dabr_data, cpu));
> +
> +out:
> + /* Enable pre-emption only if single-stepping is finished */
> + if (stepped) {
> + per_cpu(dabr_data, cpu) = 0;
> + put_cpu();
> + }
> + return rc;
> +}
> +
> +/*
> + * Handle single-step exceptions following a DABR hit.
> + */
> +int __kprobes single_step_dabr_instruction(struct die_args *args)
> +{
> + struct pt_regs *regs = args->regs;
> + int cpu = get_cpu();
> + int ret = NOTIFY_DONE;
> + siginfo_t info;
> + unsigned long this_dabr_data = per_cpu(dabr_data, cpu);
> +
> + /*
> + * Check if we are single-stepping as a result of a
> + * previous HW Breakpoint exception
> + */
> + if (this_dabr_data == 0)
> + goto out;
> +
> + regs->msr &= ~MSR_SE;
> + /* Deliver signal to user-space */
> + if (this_dabr_data < TASK_SIZE) {
> + info.si_signo = SIGTRAP;
> + info.si_errno = 0;
> + info.si_code = TRAP_HWBKPT;
> + info.si_addr = (void __user *)(per_cpu(dabr_data, cpu));
> + force_sig_info(SIGTRAP, &info, current);
> + }
> +
> + set_dabr(this_dabr_data);
> + per_cpu(dabr_data, cpu) = 0;
> + ret = NOTIFY_STOP;
> + /*
> + * If single-stepped after hw_breakpoint_handler(), pre-emption is
> + * already disabled.
> + */
> + put_cpu();
> +
> +out:
> + /*
> + * A put_cpu() call is required to complement the get_cpu()
> + * call used initially
> + */
> + put_cpu();
> + return ret;
> +}
> +
> +/*
> + * Handle debug exception notifications.
> + */
> +int __kprobes hw_breakpoint_exceptions_notify(
Um.. there seems to be a pretty glaring problem here, in that AFAICT
in this revision of the series, this function is never invoked, and so
your breakpoint handling code will never be executed. i.e. the
changes to do_dabr to connect your code seem to be missing.
> + struct notifier_block *unused, unsigned long val, void *data)
> +{
> + int ret = NOTIFY_DONE;
> +
> + switch (val) {
> + case DIE_DABR_MATCH:
> + ret = hw_breakpoint_handler(data);
> + break;
> + case DIE_SSTEP:
> + ret = single_step_dabr_instruction(data);
> + break;
> + }
> +
> + return ret;
> +}
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/ptrace.c
> +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> @@ -755,6 +755,10 @@ void user_disable_single_step(struct tas
> clear_tsk_thread_flag(task, TIF_SINGLESTEP);
> }
>
> +void ptrace_triggered(struct hw_breakpoint *bp, struct pt_regs *regs)
> +{
> +}
This is never used, so does not belong in this patch.
> int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> unsigned long data)
> {
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [Patch 3/6] Modify ptrace code to use Hardware Breakpoint interfaces
From: David Gibson @ 2009-07-31 6:18 UTC (permalink / raw)
To: K.Prasad
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, paulus,
Alan Stern, Roland McGrath
In-Reply-To: <20090727001324.GD13562@in.ibm.com>
On Mon, Jul 27, 2009 at 05:48:27AM +0530, K.Prasad wrote:
> Modify the ptrace code to use the hardware breakpoint interfaces for user-space.
>
> Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/ptrace.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/ptrace.c
> +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> @@ -37,6 +37,7 @@
> #include <asm/page.h>
> #include <asm/pgtable.h>
> #include <asm/system.h>
> +#include <asm/hw_breakpoint.h>
>
> /*
> * does not yet catch signals sent when the child dies.
> @@ -757,11 +758,24 @@ void user_disable_single_step(struct tas
>
> void ptrace_triggered(struct hw_breakpoint *bp, struct pt_regs *regs)
> {
> + /*
> + * Unregister the breakpoint request here since ptrace has defined a
> + * one-shot behaviour for breakpoint exceptions in PPC64.
> + * The SIGTRAP signal is generated automatically for us in do_dabr().
> + * We don't have to do anything here
> + */
> + unregister_user_hw_breakpoint(current, bp);
> + kfree(bp);
This unregisters the breakpoint, but doesn't actually abort the
current breakpoint handling sequence it's invoked from. So, if your
breakpoint handler was invoked at all, which as previously mentioned,
I don't think it is, wouldn't this result in *two* SIGTRAPs from a
ptrace breakpoint: one issued before the trapping instruction from
do_dabr() and another afterwards from your step-over code.
> }
>
> int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> unsigned long data)
> {
> +#ifdef CONFIG_PPC64
> + struct thread_struct *thread = &(task->thread);
> + struct hw_breakpoint *bp;
> + int ret;
> +#endif
> /* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
> * For embedded processors we support one DAC and no IAC's at the
> * moment.
> @@ -791,6 +805,35 @@ int ptrace_set_debugreg(struct task_stru
> if (data && !(data & DABR_TRANSLATION))
> return -EIO;
>
> +#ifdef CONFIG_PPC64
> + bp = thread->hbp[0];
> + if (data == 0) {
> + if (bp) {
> + unregister_user_hw_breakpoint(task, bp);
> + kfree(bp);
> + }
> + return 0;
> + }
> +
> + if (bp) {
> + bp->info.type = data & HW_BREAKPOINT_RW;
> + task->thread.dabr = bp->info.address = data;
> + return modify_user_hw_breakpoint(task, bp);
> + }
> + bp = kzalloc(sizeof(struct hw_breakpoint), GFP_KERNEL);
> + if (!bp)
> + return -ENOMEM;
> +
> + /* Store the type of breakpoint */
> + bp->info.type = data & HW_BREAKPOINT_RW;
> + bp->triggered = ptrace_triggered;
> + task->thread.dabr = bp->info.address = data;
> +
> + ret = register_user_hw_breakpoint(task, bp);
> + if (ret)
> + return ret;
> +#endif /* CONFIG_PPC64 */
> +
> /* Move contents to the DABR register */
> task->thread.dabr = data;
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH 5/5] sound/aoa: Add kmalloc NULL tests
From: Takashi Iwai @ 2009-07-31 6:22 UTC (permalink / raw)
To: Julia Lawall
Cc: linuxppc-dev, Johannes Berg, kernel-janitors, alsa-devel,
linux-kernel
In-Reply-To: <Pine.LNX.4.64.0907301627260.8734@ask.diku.dk>
At Thu, 30 Jul 2009 16:29:54 +0200 (CEST),
Julia Lawall wrote:
>
> On Thu, 30 Jul 2009, Johannes Berg wrote:
>
> > On Thu, 2009-07-30 at 16:11 +0200, Julia Lawall wrote:
> > > From: Julia Lawall <julia@diku.dk>
> > >
> > > Check that the result of kzalloc is not NULL before a dereference.
> >
> > > irq_client = kzalloc(sizeof(struct pmf_irq_client),
> > > GFP_KERNEL);
> > > + if (!irq_client) {
> > > + err = -ENOMEM;
> > > + printk(KERN_ERR "snd-aoa: gpio layer failed to"
> > > + " register %s irq (%d)\n", name, err);
> > > + goto out_unlock;
> > > + }
> >
> > Looks good, thanks, but I'd really drop the printk if only to not have
> > the string there, that doesn't really seem interesting.
>
> The printk is based on similar error handling code a few lines later:
But another problem is that the same error message is reused although
the error condition is totally different. The kzalloc NULL isn't
about the registration error. So, it's rather confusing.
However, for this particular error path, I agree with Johannes; we can
skip the error message since the error code ENOMEM is obvious.
thanks,
Takashi
>
> if (err) {
> printk(KERN_ERR "snd-aoa: gpio layer failed to"
> " register %s irq (%d)\n", name,
> err);
> kfree(irq_client);
> goto out_unlock;
> }
>
> Should the printk be removed in this case as well? Or is it ok to fail
> silently in one case and not in the other?
>
> julia
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply
* Re: [PATCH 5/5] sound/aoa: Add kmalloc NULL tests
From: Julia Lawall @ 2009-07-31 6:31 UTC (permalink / raw)
To: Takashi Iwai
Cc: linuxppc-dev, Johannes Berg, kernel-janitors, alsa-devel,
linux-kernel
In-Reply-To: <s5hws5pid6u.wl%tiwai@suse.de>
On Fri, 31 Jul 2009, Takashi Iwai wrote:
> At Thu, 30 Jul 2009 16:29:54 +0200 (CEST),
> Julia Lawall wrote:
> >
> > On Thu, 30 Jul 2009, Johannes Berg wrote:
> >
> > > On Thu, 2009-07-30 at 16:11 +0200, Julia Lawall wrote:
> > > > From: Julia Lawall <julia@diku.dk>
> > > >
> > > > Check that the result of kzalloc is not NULL before a dereference.
> > >
> > > > irq_client = kzalloc(sizeof(struct pmf_irq_client),
> > > > GFP_KERNEL);
> > > > + if (!irq_client) {
> > > > + err = -ENOMEM;
> > > > + printk(KERN_ERR "snd-aoa: gpio layer failed to"
> > > > + " register %s irq (%d)\n", name, err);
> > > > + goto out_unlock;
> > > > + }
> > >
> > > Looks good, thanks, but I'd really drop the printk if only to not have
> > > the string there, that doesn't really seem interesting.
> >
> > The printk is based on similar error handling code a few lines later:
>
> But another problem is that the same error message is reused although
> the error condition is totally different. The kzalloc NULL isn't
> about the registration error. So, it's rather confusing.
>
> However, for this particular error path, I agree with Johannes; we can
> skip the error message since the error code ENOMEM is obvious.
OK, I will send a new patch.
julia
^ permalink raw reply
* Re: [PATCH 5/5] sound/aoa: Add kmalloc NULL tests
From: Julia Lawall @ 2009-07-31 6:32 UTC (permalink / raw)
To: Takashi Iwai
Cc: linuxppc-dev, Johannes Berg, kernel-janitors, alsa-devel,
linux-kernel
In-Reply-To: <s5hws5pid6u.wl%tiwai@suse.de>
From: Julia Lawall <julia@diku.dk>
Check that the result of kzalloc is not NULL before a dereference.
The semantic match that finds this problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)
// <smpl>
@@
expression *x;
identifier f;
constant char *C;
@@
x = \(kmalloc\|kcalloc\|kzalloc\)(...);
... when != x == NULL
when != x != NULL
when != (x || ...)
(
kfree(x)
|
f(...,C,...,x,...)
|
*f(...,x,...)
|
*x->f
)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
sound/aoa/core/gpio-pmf.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/var/linuxes/linux-next/sound/aoa/core/gpio-pmf.c b/var/julia/linuxcopy/sound/aoa/core/gpio-pmf.c
index 5ca2220..1dd0c28 100644
--- a/var/linuxes/linux-next/sound/aoa/core/gpio-pmf.c
+++ b/var/julia/linuxcopy/sound/aoa/core/gpio-pmf.c
@@ -182,6 +182,10 @@ static int pmf_set_notify(struct gpio_runtime *rt,
if (!old && notify) {
irq_client = kzalloc(sizeof(struct pmf_irq_client),
GFP_KERNEL);
+ if (!irq_client) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
irq_client->data = notif;
irq_client->handler = pmf_handle_notify_irq;
irq_client->owner = THIS_MODULE;
^ permalink raw reply related
* Re: can the kernel show user task stack backtrace ?
From: Norbert van Bolhuis @ 2009-07-31 8:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Mc Guire, Alessandro Rubini
In-Reply-To: <20090730165555.GA5476@opentech.at>
Thanks for the answers!
libSegFault.so seems to do what I want. I'll replace
sysctl -w kernel.print-fatal-signals=1
with
export LD_PRELOAD=/lib/libSegFault.so
since it better suits my needs.
^ permalink raw reply
* Re: [PATCH 5/5] sound/aoa: Add kmalloc NULL tests
From: Takashi Iwai @ 2009-07-31 8:16 UTC (permalink / raw)
To: Julia Lawall
Cc: linuxppc-dev, Johannes Berg, kernel-janitors, alsa-devel,
linux-kernel
In-Reply-To: <Pine.LNX.4.64.0907310831330.13462@ask.diku.dk>
At Fri, 31 Jul 2009 08:32:03 +0200 (CEST),
Julia Lawall wrote:
>
> From: Julia Lawall <julia@diku.dk>
>
> Check that the result of kzalloc is not NULL before a dereference.
>
> The semantic match that finds this problem is as follows:
> (http://www.emn.fr/x-info/coccinelle/)
>
> // <smpl>
> @@
> expression *x;
> identifier f;
> constant char *C;
> @@
>
> x = \(kmalloc\|kcalloc\|kzalloc\)(...);
> ... when != x == NULL
> when != x != NULL
> when != (x || ...)
> (
> kfree(x)
> |
> f(...,C,...,x,...)
> |
> *f(...,x,...)
> |
> *x->f
> )
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
Applied now.
But, please fix the path of the file correctly applicable to linux
kernel tree at the next time. It includes /var/x/y, and confuses git
am totally.
thanks,
Takashi
>
> ---
> sound/aoa/core/gpio-pmf.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/var/linuxes/linux-next/sound/aoa/core/gpio-pmf.c b/var/julia/linuxcopy/sound/aoa/core/gpio-pmf.c
> index 5ca2220..1dd0c28 100644
> --- a/var/linuxes/linux-next/sound/aoa/core/gpio-pmf.c
> +++ b/var/julia/linuxcopy/sound/aoa/core/gpio-pmf.c
> @@ -182,6 +182,10 @@ static int pmf_set_notify(struct gpio_runtime *rt,
> if (!old && notify) {
> irq_client = kzalloc(sizeof(struct pmf_irq_client),
> GFP_KERNEL);
> + if (!irq_client) {
> + err = -ENOMEM;
> + goto out_unlock;
> + }
> irq_client->data = notif;
> irq_client->handler = pmf_handle_notify_irq;
> irq_client->owner = THIS_MODULE;
>
^ permalink raw reply
* [PATCH] powerpc/fsl-booke: Read buffer overflow
From: Roel Kluin @ 2009-07-31 12:38 UTC (permalink / raw)
To: galak, linuxppc-dev, Andrew Morton
cam[tlbcam_index] is checked before tlbcam_index < ARRAY_SIZE(cam)
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
index bb3d659..dc93e95 100644
--- a/arch/powerpc/mm/fsl_booke_mmu.c
+++ b/arch/powerpc/mm/fsl_booke_mmu.c
@@ -161,7 +161,7 @@ unsigned long __init mmu_mapin_ram(void)
unsigned long virt = PAGE_OFFSET;
phys_addr_t phys = memstart_addr;
- while (cam[tlbcam_index] && tlbcam_index < ARRAY_SIZE(cam)) {
+ while (tlbcam_index < ARRAY_SIZE(cam) && cam[tlbcam_index]) {
settlbcam(tlbcam_index, virt, phys, cam[tlbcam_index], PAGE_KERNEL_X, 0);
virt += cam[tlbcam_index];
phys += cam[tlbcam_index];
^ permalink raw reply related
* Please pull 'next' branch of 4xx tree
From: Josh Boyer @ 2009-07-31 12:50 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
Hi Ben,
A few changes from Stefan for 460 and Kilauea boards.
Thanks,
josh
The following changes since commit 8984d7d529d50d11ebd840d69ce61ab0bf4ffcab:
Benjamin Herrenschmidt (1):
powerpc: Remaining 64-bit Book3E support
are available in the git repository at:
ssh://master.kernel.org/pub/scm/linux/kernel/git/jwboyer/powerpc-4xx.git next
Stefan Roese (5):
powerpc: Add AMCC 460EX/460GT Rev. B support to cputable.c
powerpc/44x: Add NAND support to Canyonlands dts
powerpc/40x: Update Kilauea dts to support NAND, RTC and HWMON
powerpc/44x: Update Canyonlands defconfig to support NOR, NAND and RTC
powerpc/40x: Update kilauea defconfig to support NAND, RTC and HWMON
arch/powerpc/boot/dts/canyonlands.dts | 49 +++-
arch/powerpc/boot/dts/kilauea.dts | 44 +++-
arch/powerpc/configs/40x/kilauea_defconfig | 298 +++++++++++++++++---
arch/powerpc/configs/44x/canyonlands_defconfig | 350 ++++++++++++++++++++----
arch/powerpc/kernel/cputable.c | 30 ++-
5 files changed, 658 insertions(+), 113 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/3] Support for PCI Express reset type
From: Andi Kleen @ 2009-07-31 16:19 UTC (permalink / raw)
To: Mike Mason
Cc: linuxppc-dev, Paul Mackerras, Richard Lary, linux-pci,
linasvepstas
In-Reply-To: <4A721FB1.4040903@us.ibm.com>
Mike Mason <mmlnx@us.ibm.com> writes:
>
> These patches supersede the previously submitted patch that
> implemented a fundamental reset bit field.
>
> Please review and let me know of any concerns.
Any plans to implement that for x86 too? Right now it seems to be a PPC
specific hack. And where is the driver that is using it?
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply
* Re: [PATCH 1/3] Support for PCI Express reset type
From: Andrew Vasquez @ 2009-07-31 16:47 UTC (permalink / raw)
To: Andi Kleen
Cc: linuxppc-dev@ozlabs.org, Paul Mackerras, Richard Lary,
linux-pci@vger.kernel.org, linasvepstas@gmail.com
In-Reply-To: <87hbws3jvx.fsf@basil.nowhere.org>
On Fri, 31 Jul 2009, Andi Kleen wrote:
> Mike Mason <mmlnx@us.ibm.com> writes:
> >
> > These patches supersede the previously submitted patch that
> > implemented a fundamental reset bit field.
> >
> > Please review and let me know of any concerns.
>
> Any plans to implement that for x86 too? Right now it seems to be a PPC
> specific hack.
Are there any non-PPC platforms which support EEH slot-reset?
> And where is the driver that is using it?
That would be the qla2xxx (FC/FCoE storage) and qlge (10gb NIC
) drivers. Changes to the drivers could look something like:
Index: b/drivers/net/qlge/qlge_main.c
===================================================================
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4289,6 +4289,9 @@ static int __devinit qlge_probe(struct p
return err;
}
+ /* Set EEH reset type to fundamental for this device */
+ pdev->needs_freset = 1;
+
qdev = netdev_priv(ndev);
SET_NETDEV_DEV(ndev, &pdev->dev);
ndev->features = (0
Index: b/drivers/scsi/qla2xxx/qla_os.c
===================================================================
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1773,6 +1773,10 @@ qla2x00_probe_one(struct pci_dev *pdev,
/* Set ISP-type information. */
qla2x00_set_isp_flags(ha);
+ /* Set EEH reset type to fundamental if required by hba */
+ if (IS_QLA24XX(ha) || IS_QLA25XX(ha) || IS_QLA81XX(ha))
+ pdev->needs_freset = 1;
+
/* Configure PCI I/O space */
ret = qla2x00_iospace_config(ha);
if (ret)
once the infrastructure changes were hashed out.
-- av
^ permalink raw reply
* [PATCH] Stop pci_set_dma_mask() from failing when RAM doesn't exceed the mask anyway
From: David Woodhouse @ 2009-07-31 19:41 UTC (permalink / raw)
To: linuxppc-dev
On an iMac G5, the b43 driver is failing to initialise because trying to
set the dma mask to 30-bit fails. Even though there's only 512MiB of RAM
in the machine anyway:
https://bugzilla.redhat.com/show_bug.cgi?id=514787
We should probably let it succeed if the available RAM in the system
doesn't exceed the requested limit.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index 20a60d6..1769a8e 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -90,11 +90,11 @@ static void dma_direct_unmap_sg(struct device *dev,
struct scatterlist *sg,
static int dma_direct_dma_supported(struct device *dev, u64 mask)
{
#ifdef CONFIG_PPC64
- /* Could be improved to check for memory though it better be
- * done via some global so platforms can set the limit in case
+ extern unsigned long highest_memmap_pfn;
+ /* Could be improved so platforms can set the limit in case
* they have limited DMA windows
*/
- return mask >= DMA_BIT_MASK(32);
+ return (mask >> PAGE_SHIFT) >= highest_memmap_pfn;
#else
return 1;
#endif
--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation
^ permalink raw reply related
* Re: [PATCH] mtd/maps: add mtd-ram support to physmap_of
From: Albrecht Dreß @ 2009-07-31 19:47 UTC (permalink / raw)
To: Wolfram Sang
Cc: devicetree-discuss, Vitaly Wool, Artem Bityutskiy, linuxppc-dev,
linux-mtd, Ken MacLeod, David Woodhouse
In-Reply-To: <1247834363-28198-1-git-send-email-w.sang@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 821 bytes --]
Am 17.07.09 14:39 schrieb(en) Wolfram Sang:
> Use physmap_of to access RAMs as mtd and add documenation for it.
> This approach is a lot less intrusive as adding an of-wrapper around
> plat-ram.c. As most extensions of plat-ram.c (e.g. custom
> map-functions) can't be mapped to the device tree anyhow, extending
> physmap_of seems to be the cleanest approach.
The patch works nicely on a custom (roughly Icecube) based 5200B board,
with a Renesas static ram, no partitions and a jffs2 file system on
it. I had to add some more tweaks to physmap_of.c as to work around
the 5200's limitations accessing chips in 16-bit mode through the local
bus, which is a lot easier and cleaner with this patch than with the
old one (of 05 June 09).
Acked-by: Albrecht Dreß <albrecht.dress@arcor.de>
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* question on PPC64 relocatable kernel
From: Ming Lei @ 2009-07-31 20:36 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org
Hi,
I am researching PPC64 code and try to come up a design for relocatable ker=
nel for ppc32. I noticed that the current ppc64 implementation only changes=
all the entries in RELA table to add the offset from compile load address =
to relocated address, but not for GOT table. Does GOT entry need the adjust=
ment as well?
The second question is: does PPC64 relocatable kernel still needs to copy t=
he whole kernel from running address to address 0?
Thanks
Ming
^ permalink raw reply
* Re: [PATCH] Stop pci_set_dma_mask() from failing when RAM doesn't exceed the mask anyway
From: Benjamin Herrenschmidt @ 2009-07-31 22:25 UTC (permalink / raw)
To: David Woodhouse; +Cc: linuxppc-dev
In-Reply-To: <1249069310.20192.220.camel@macbook.infradead.org>
On Fri, 2009-07-31 at 20:41 +0100, David Woodhouse wrote:
> On an iMac G5, the b43 driver is failing to initialise because trying to
> set the dma mask to 30-bit fails. Even though there's only 512MiB of RAM
> in the machine anyway:
> https://bugzilla.redhat.com/show_bug.cgi?id=514787
>
> We should probably let it succeed if the available RAM in the system
> doesn't exceed the requested limit.
>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Also, isn't our iommu code smart enough to clamp allocations to the DMA
mask nowadays ? In that case, we could probably just force iommu on
always...
Cheers,
Ben.
> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
> index 20a60d6..1769a8e 100644
> --- a/arch/powerpc/kernel/dma.c
> +++ b/arch/powerpc/kernel/dma.c
> @@ -90,11 +90,11 @@ static void dma_direct_unmap_sg(struct device *dev,
> struct scatterlist *sg,
> static int dma_direct_dma_supported(struct device *dev, u64 mask)
> {
> #ifdef CONFIG_PPC64
> - /* Could be improved to check for memory though it better be
> - * done via some global so platforms can set the limit in case
> + extern unsigned long highest_memmap_pfn;
> + /* Could be improved so platforms can set the limit in case
> * they have limited DMA windows
> */
> - return mask >= DMA_BIT_MASK(32);
> + return (mask >> PAGE_SHIFT) >= highest_memmap_pfn;
> #else
> return 1;
> #endif
>
^ permalink raw reply
* Re: [PATCH 3/20] powerpc/mm: Add HW threads support to no_hash TLB management
From: Benjamin Herrenschmidt @ 2009-07-31 22:29 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <6FD94305-B60D-4DAF-8296-88345D11187F@kernel.crashing.org>
On Thu, 2009-07-30 at 22:35 -0500, Kumar Gala wrote:
> > /* XXX This clear should ultimately be part of
> local_flush_tlb_mm */
> > - __clear_bit(id, stale_map[cpu]);
> > + for (cpu = cpu_first_thread_in_core(cpu);
> > + cpu <= cpu_last_thread_in_core(cpu); cpu++)
> > + __clear_bit(id, stale_map[cpu]);
> > }
>
> This looks a bit dodgy. using 'cpu' as both the loop variable and
> what you are computing to determine loop start/end..
>
Hrm... I would have thought that it was still correct... do you see any
reason why the above code is wrong ? because if not we may be hitting a
gcc issue...
IE. At loop init, cpu gets clamped down to the first thread in the core,
which should be fine. Then, we compare CPU to the last thread in core
for the current CPU which should always return the same value.
So I'm very interested to know what is actually wrong, ie, either I'm
just missing something obvious, or you are just pushing a bug under the
carpet which could come back and bit us later :-)
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] Stop pci_set_dma_mask() from failing when RAM doesn't exceed the mask anyway
From: David Woodhouse @ 2009-08-01 7:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1249079105.1509.95.camel@pasglop>
On Sat, 2009-08-01 at 08:25 +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2009-07-31 at 20:41 +0100, David Woodhouse wrote:
> > On an iMac G5, the b43 driver is failing to initialise because trying to
> > set the dma mask to 30-bit fails. Even though there's only 512MiB of RAM
> > in the machine anyway:
> > https://bugzilla.redhat.com/show_bug.cgi?id=514787
> >
> > We should probably let it succeed if the available RAM in the system
> > doesn't exceed the requested limit.
> >
> > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
>
> Also, isn't our iommu code smart enough to clamp allocations to the DMA
> mask nowadays ? In that case, we could probably just force iommu on
> always...
We're not using the IOMMU on this box:
PowerMac motherboard: iMac G5
DART: table not allocated, using direct DMA
--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation
^ permalink raw reply
* Re: [PATCH] Stop pci_set_dma_mask() from failing when RAM doesn't exceed the mask anyway
From: Benjamin Herrenschmidt @ 2009-08-01 8:00 UTC (permalink / raw)
To: David Woodhouse; +Cc: linuxppc-dev
In-Reply-To: <1249113285.20192.961.camel@macbook.infradead.org>
On Sat, 2009-08-01 at 08:54 +0100, David Woodhouse wrote:
> On Sat, 2009-08-01 at 08:25 +1000, Benjamin Herrenschmidt wrote:
> > On Fri, 2009-07-31 at 20:41 +0100, David Woodhouse wrote:
> > > On an iMac G5, the b43 driver is failing to initialise because trying to
> > > set the dma mask to 30-bit fails. Even though there's only 512MiB of RAM
> > > in the machine anyway:
> > > https://bugzilla.redhat.com/show_bug.cgi?id=514787
> > >
> > > We should probably let it succeed if the available RAM in the system
> > > doesn't exceed the requested limit.
> > >
> > > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> >
> > Also, isn't our iommu code smart enough to clamp allocations to the DMA
> > mask nowadays ? In that case, we could probably just force iommu on
> > always...
>
> We're not using the IOMMU on this box:
>
> PowerMac motherboard: iMac G5
> DART: table not allocated, using direct DMA
I know, I was suggesting we do :-)
Cheers,
Ben.
^ permalink raw reply
* [PATCH 1/10] arch/powerpc/sysdev/qe_lib: introduce missing kfree
From: Julia Lawall @ 2009-08-01 8:52 UTC (permalink / raw)
To: timur, linuxppc-dev, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku.dk>
Error handling code following a kzalloc should free the allocated data.
The semantic match that finds the problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)
// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@
x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
when != if (...) { <+...x...+> }
(
x->f1 = E
|
(x->f1 == NULL || ...)
|
f(...,x->f1,...)
)
...>
(
return \(0\|<+...x...+>\|ptr\);
|
return@p2 ...;
)
@script:python@
p1 << r.p1;
p2 << r.p2;
@@
print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
arch/powerpc/sysdev/qe_lib/qe_ic.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c b/arch/powerpc/sysdev/qe_lib/qe_ic.c
index 074905c..3faa42e 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
@@ -339,8 +339,10 @@ void __init qe_ic_init(struct device_node *node, unsigned int flags,
qe_ic->irqhost = irq_alloc_host(node, IRQ_HOST_MAP_LINEAR,
NR_QE_IC_INTS, &qe_ic_host_ops, 0);
- if (qe_ic->irqhost == NULL)
+ if (qe_ic->irqhost == NULL) {
+ kfree(qe_ic);
return;
+ }
qe_ic->regs = ioremap(res.start, res.end - res.start + 1);
@@ -352,6 +354,7 @@ void __init qe_ic_init(struct device_node *node, unsigned int flags,
if (qe_ic->virq_low == NO_IRQ) {
printk(KERN_ERR "Failed to map QE_IC low IRQ\n");
+ kfree(qe_ic);
return;
}
^ permalink raw reply related
* [PATCH 2/10] arch/powerpc: introduce missing kfree
From: Julia Lawall @ 2009-08-01 8:53 UTC (permalink / raw)
To: benh, paulus, linuxppc-dev, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku.dk>
Error handling code following a kzalloc should free the allocated data.
The semantic match that finds the problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)
// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@
x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
when != if (...) { <+...x...+> }
(
x->f1 = E
|
(x->f1 == NULL || ...)
|
f(...,x->f1,...)
)
...>
(
return \(0\|<+...x...+>\|ptr\);
|
return@p2 ...;
)
@script:python@
p1 << r.p1;
p2 << r.p2;
@@
print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
arch/powerpc/sysdev/ipic.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
index 69e2630..040144e 100644
--- a/arch/powerpc/sysdev/ipic.c
+++ b/arch/powerpc/sysdev/ipic.c
@@ -735,8 +735,10 @@ struct ipic * __init ipic_init(struct device_node *node, unsigned int flags)
ipic->irqhost = irq_alloc_host(node, IRQ_HOST_MAP_LINEAR,
NR_IPIC_INTS,
&ipic_host_ops, 0);
- if (ipic->irqhost == NULL)
+ if (ipic->irqhost == NULL) {
+ kfree(ipic);
return NULL;
+ }
ipic->regs = ioremap(res.start, res.end - res.start + 1);
^ permalink raw reply related
* Re: [PATCH] Stop pci_set_dma_mask() from failing when RAM doesn't exceed the mask anyway
From: David Woodhouse @ 2009-08-01 9:00 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1249113656.1509.125.camel@pasglop>
On Sat, 2009-08-01 at 18:00 +1000, Benjamin Herrenschmidt wrote:
> On Sat, 2009-08-01 at 08:54 +0100, David Woodhouse wrote:
> > On Sat, 2009-08-01 at 08:25 +1000, Benjamin Herrenschmidt wrote:
> > > On Fri, 2009-07-31 at 20:41 +0100, David Woodhouse wrote:
> > > > On an iMac G5, the b43 driver is failing to initialise because trying to
> > > > set the dma mask to 30-bit fails. Even though there's only 512MiB of RAM
> > > > in the machine anyway:
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=514787
> > > >
> > > > We should probably let it succeed if the available RAM in the system
> > > > doesn't exceed the requested limit.
> > > >
> > > > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> > >
> > > Also, isn't our iommu code smart enough to clamp allocations to the DMA
> > > mask nowadays ? In that case, we could probably just force iommu on
> > > always...
> >
> > We're not using the IOMMU on this box:
> >
> > PowerMac motherboard: iMac G5
> > DART: table not allocated, using direct DMA
>
> I know, I was suggesting we do :-)
I'm not sure. Losing 16MiB on a machine which only has 512MiB anyway
doesn't seem ideal, and we'll want to make the no-iommu code DTRT
_anyway_, surely?
So we might as well let the DART keep its existing logic (which is only
to bother if we have more than 1GiB of RAM; a limit chosen specifically
because of the Broadcom brokenness).
--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox