[PATCH v2 0/2] mark driver as non-removable

[PATCH v2 0/2] mark driver as non-removable

[Alexander Stein]

Changes in v2;
* Instead of adding a remove function which is unused otherwise, mark the driver
  as non-remoable

Using DEBUG_TEST_DRIVER_REMOVE every driver gets probed, removed and probed
again. This breaks drivers without removal function, e.g. arm perf
resulting in the following dump:
------------[ cut here ]------------
WARNING: CPU: 1 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x6c/0x7c
sysfs: cannot create duplicate filename '/devices/armv7_cortex_a7'
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.9.0+ #212
Hardware name: Freescale LS1021A
Backtrace:
[<8020c044>] (dump_backtrace) from [<8020c2f0>] (show_stack+0x18/0x1c)
 r7:00000009 r6:60000013 r5:80c1776c r4:00000000
[<8020c2d8>] (show_stack) from [<8046ead8>] (dump_stack+0x94/0xa8)
[<8046ea44>] (dump_stack) from [<8021ecd0>] (__warn+0xec/0x104)
 r7:00000009 r6:8092efc8 r5:00000000 r4:bf04bd80
[<8021ebe4>] (__warn) from [<8021ed28>] (warn_slowpath_fmt+0x40/0x48)
 r9:80a32848 r8:00000000 r7:00000000 r6:bf0ab780 r5:8091d590 r4:bf0df000
[<8021ecec>] (warn_slowpath_fmt) from [<8036d53c>] (sysfs_warn_dup+0x6c/0x7c)
 r3:bf0df000 r2:8092ef94
[<8036d4d0>] (sysfs_warn_dup) from [<8036d628>] (sysfs_create_dir_ns+0x8c/0x9c)
 r6:bf0ab780 r5:bf20ee08 r4:ffffffef
[<8036d59c>] (sysfs_create_dir_ns) from [<80471860>] (kobject_add_internal+0xa8/0x34c)
 r6:bf0aa198 r5:00000000 r4:bf20ee08
[<804717b8>] (kobject_add_internal) from [<80471b54>] (kobject_add+0x50/0x94)
 r7:00000000 r6:00000000 r5:00000000 r4:bf20ee08
[<80471b08>] (kobject_add) from [<80524590>] (device_add+0xe4/0x590)
 r3:00000000 r2:00000000
 r6:bf20ee00 r5:00000000 r4:bf20ee08
[<805244ac>] (device_add) from [<802a38a8>] (pmu_dev_alloc+0x88/0xd8)
 r10:00000091 r9:80a32848 r8:00000000 r7:80a32840 r6:80c0ed3c r5:00000000
 r4:bf1fe800
[<802a3820>] (pmu_dev_alloc) from [<80a0cd20>] (perf_event_sysfs_init+0x5c/0xb4)
 r7:80a32840 r6:00000000 r5:bf1fe800 r4:80c0ede0
[<80a0ccc4>] (perf_event_sysfs_init) from [<80201778>] (do_one_initcall+0x48/0x178)
 r6:80c45000 r5:80a0ccc4 r4:00000006
[<80201730>] (do_one_initcall) from [<80a00ecc>] (kernel_init_freeable+0x168/0x20c)
 r8:80a00610 r7:80a32840 r6:80c45000 r5:80c45000 r4:00000006
[<80a00d64>] (kernel_init_freeable) from [<806febcc>] (kernel_init+0x10/0x11c)
 r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:806febbc
 r4:00000000
[<806febbc>] (kernel_init) from [<80208388>] (ret_from_fork+0x14/0x2c)
 r5:806febbc r4:00000000
---[ end trace 9d251d389382804f ]---

This patchset marks the driver as explicitly non-remoavle preventing this error.
This disables bin/unbind for this driver as well.

Alexander Stein (2):
  drivers/perf: arm_pmu: Use devm_ allocators
  arm: perf: Mark as non-removable

 arch/arm/kernel/perf_event_v7.c |  1 +
 drivers/perf/arm_pmu.c          | 14 ++++----------
 2 files changed, 5 insertions(+), 10 deletions(-)

-- 
2.10.2

[PATCH v2 1/2] drivers/perf: arm_pmu: Use devm_ allocators

[Alexander Stein]

This eliminates several calls to kfree.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 drivers/perf/arm_pmu.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index b37b572..a9bbdbf 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -917,7 +917,8 @@ static int of_pmu_irq_cfg(struct arm_pmu *pmu)
 	bool using_spi = false;
 	struct platform_device *pdev = pmu->plat_device;
 
-	irqs = kcalloc(pdev->num_resources, sizeof(*irqs), GFP_KERNEL);
+	irqs = devm_kcalloc(&pdev->dev, pdev->num_resources, sizeof(*irqs),
+			    GFP_KERNEL);
 	if (!irqs)
 		return -ENOMEM;
 
@@ -939,7 +940,6 @@ static int of_pmu_irq_cfg(struct arm_pmu *pmu)
 				pr_err("PPI/SPI IRQ type mismatch for %s!\n",
 					dn->name);
 				of_node_put(dn);
-				kfree(irqs);
 				return -EINVAL;
 			}
 
@@ -988,10 +988,8 @@ static int of_pmu_irq_cfg(struct arm_pmu *pmu)
 			int ret;
 
 			ret = irq_get_percpu_devid_partition(irq, &pmu->supported_cpus);
-			if (ret) {
-				kfree(irqs);
+			if (ret)
 				return ret;
-			}
 		} else {
 			/* Otherwise default to all CPUs */
 			cpumask_setall(&pmu->supported_cpus);
@@ -1001,8 +999,6 @@ static int of_pmu_irq_cfg(struct arm_pmu *pmu)
 	/* If we matched up the IRQ affinities, use them to route the SPIs */
 	if (using_spi && i == pdev->num_resources)
 		pmu->irq_affinity = irqs;
-	else
-		kfree(irqs);
 
 	return 0;
 }
@@ -1017,7 +1013,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 	struct arm_pmu *pmu;
 	int ret = -ENODEV;
 
-	pmu = kzalloc(sizeof(struct arm_pmu), GFP_KERNEL);
+	pmu = devm_kzalloc(&pdev->dev, sizeof(struct arm_pmu), GFP_KERNEL);
 	if (!pmu) {
 		pr_info("failed to allocate PMU device!\n");
 		return -ENOMEM;
@@ -1074,8 +1070,6 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 out_free:
 	pr_info("%s: failed to register PMU devices!\n",
 		of_node_full_name(node));
-	kfree(pmu->irq_affinity);
-	kfree(pmu);
 	return ret;
 }
 
-- 
2.10.2

[PATCH v2 2/2] arm: perf: Mark as non-removable

[Alexander Stein]

This driver can only built into the kernel. So disallow driver bind/unbind
and also prevent a kernel error in case DEBUG_TEST_DRIVER_REMOVE is
enabled.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 arch/arm/kernel/perf_event_v7.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
index b942349..795e373 100644
--- a/arch/arm/kernel/perf_event_v7.c
+++ b/arch/arm/kernel/perf_event_v7.c
@@ -2029,6 +2029,7 @@ static int armv7_pmu_device_probe(struct platform_device *pdev)
 static struct platform_driver armv7_pmu_driver = {
 	.driver		= {
 		.name	= "armv7-pmu",
+		.suppress_bind_attrs = true,
 		.of_match_table = armv7_pmu_of_device_ids,
 	},
 	.probe		= armv7_pmu_device_probe,
-- 
2.10.2

Re: [PATCH v2 2/2] arm: perf: Mark as non-removable

[Mark Rutland]

Hi,

On Wed, Dec 21, 2016 at 04:03:40PM +0100, Alexander Stein wrote:
> This driver can only built into the kernel. So disallow driver bind/unbind
> and also prevent a kernel error in case DEBUG_TEST_DRIVER_REMOVE is
> enabled.
> 
> Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> ---
>  arch/arm/kernel/perf_event_v7.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
> index b942349..795e373 100644
> --- a/arch/arm/kernel/perf_event_v7.c
> +++ b/arch/arm/kernel/perf_event_v7.c
> @@ -2029,6 +2029,7 @@ static int armv7_pmu_device_probe(struct platform_device *pdev)
>  static struct platform_driver armv7_pmu_driver = {
>  	.driver		= {
>  		.name	= "armv7-pmu",
> +		.suppress_bind_attrs = true,
>  		.of_match_table = armv7_pmu_of_device_ids,
>  	},

While this patch looks correct, the other perf_event_* drivers (e.g. those
under arch/arm/) will need similar treatment.

More generally, updating each and every driver in this manner seems like a
scattergun approach that is tiresome and error prone.

IMO, it would be vastly better for a higher layer to enforce that we don't
attempt to unbind drivers where the driver does not have a remove callback, as
is the case here (and I suspect most over cases where DEBUG_TEST_DRIVER_REMOVE
is blowing up).

Is there any reason that can't be enforced at the bus layer, say?

Thanks,
Mark.

Re: [PATCH v2 2/2] arm: perf: Mark as non-removable

[Alexander Stein]

Hi,

On Thursday 22 December 2016 22:48:32, Mark Rutland wrote:
> On Wed, Dec 21, 2016 at 04:03:40PM +0100, Alexander Stein wrote:
> > This driver can only built into the kernel. So disallow driver bind/unbind
> > and also prevent a kernel error in case DEBUG_TEST_DRIVER_REMOVE is
> > enabled.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > ---
> > 
> >  arch/arm/kernel/perf_event_v7.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/kernel/perf_event_v7.c
> > b/arch/arm/kernel/perf_event_v7.c index b942349..795e373 100644
> > --- a/arch/arm/kernel/perf_event_v7.c
> > +++ b/arch/arm/kernel/perf_event_v7.c
> > @@ -2029,6 +2029,7 @@ static int armv7_pmu_device_probe(struct
> > platform_device *pdev)> 
> >  static struct platform_driver armv7_pmu_driver = {
> >  
> >  	.driver		= {
> >  	
> >  		.name	= "armv7-pmu",
> > 
> > +		.suppress_bind_attrs = true,
> > 
> >  		.of_match_table = armv7_pmu_of_device_ids,
> >  	
> >  	},
> 
> While this patch looks correct, the other perf_event_* drivers (e.g. those
> under arch/arm/) will need similar treatment.

Yep, but this is the only one I can actually test.

> More generally, updating each and every driver in this manner seems like a
> scattergun approach that is tiresome and error prone.
> 
> IMO, it would be vastly better for a higher layer to enforce that we don't
> attempt to unbind drivers where the driver does not have a remove callback,
> as is the case here (and I suspect most over cases where
> DEBUG_TEST_DRIVER_REMOVE is blowing up).

You mean something like this?
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index 4eabfe2..3b6c1a2d 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -158,6 +158,9 @@ int driver_register(struct device_driver *drv)
> 
>                 printk(KERN_WARNING "Driver '%s' needs updating - please use
>                 "
>                 
>                         "bus_type methods\n", drv->name);
> 
> +       if (!drv->remove)
> +               drv->suppress_bind_attrs = true;
> +
> 
>         other = driver_find(drv->name, drv->bus);
>         if (other) {
>         
>                 printk(KERN_ERR "Error: Driver '%s' is already registered, "

> Is there any reason that can't be enforced at the bus layer, say?

I'm not sure if the change above works with remove functions set in struct 
bus_type too.
But on the other hand this would hide errors in drivers which are actually 
removable but do not cleanup properly which DEBUG_TEST_DRIVER_REMOVE tries to 
detect. By setting .suppress_bind_attrs = true explicitely you state "This 
driver cannot be removed!", so the remove callback is not missing by accident.

Best regards,
Alexander

Re: [PATCH v2 2/2] arm: perf: Mark as non-removable

[Mark Rutland]

On Wed, Jan 04, 2017 at 10:19:46AM +0100, Alexander Stein wrote:
> On Thursday 22 December 2016 22:48:32, Mark Rutland wrote:
> > On Wed, Dec 21, 2016 at 04:03:40PM +0100, Alexander Stein wrote:

> > More generally, updating each and every driver in this manner seems like a
> > scattergun approach that is tiresome and error prone.
> > 
> > IMO, it would be vastly better for a higher layer to enforce that we don't
> > attempt to unbind drivers where the driver does not have a remove callback,
> > as is the case here (and I suspect most over cases where
> > DEBUG_TEST_DRIVER_REMOVE is blowing up).
> 
> You mean something like this?
> > diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> > index 4eabfe2..3b6c1a2d 100644
> > --- a/drivers/base/driver.c
> > +++ b/drivers/base/driver.c
> > @@ -158,6 +158,9 @@ int driver_register(struct device_driver *drv)
> > 
> >                 printk(KERN_WARNING "Driver '%s' needs updating - please use
> >                 "
> >                 
> >                         "bus_type methods\n", drv->name);
> > 
> > +       if (!drv->remove)
> > +               drv->suppress_bind_attrs = true;
> > +
> > 
> >         other = driver_find(drv->name, drv->bus);
> >         if (other) {
> >         
> >                 printk(KERN_ERR "Error: Driver '%s' is already registered, "

Something of that sort, yes. Or have a bus-level callback so that the
bus can reject it dynamically (without having to alter the drv attrs).

> > Is there any reason that can't be enforced at the bus layer, say?
> 
> I'm not sure if the change above works with remove functions set in struct 
> bus_type too.
> But on the other hand this would hide errors in drivers which are actually 
> removable but do not cleanup properly which DEBUG_TEST_DRIVER_REMOVE tries to 
> detect.
> By setting .suppress_bind_attrs = true explicitely you state "This 
> driver cannot be removed!", so the remove callback is not missing by accident.

I'm not sure I follow. If the remove callback is accidentally missing,
the driver is not "actually removable" today -- there's either no remove
code, or it's not been wired up (the latter of which will likely result
in a compiler warning about an unused function).

Aborting the remove early in those cases is much safer than forcefully
removing a driver without a remove callback.

Thanks,
Mark.

Re: [PATCH v2 2/2] arm: perf: Mark as non-removable

[Russell King - ARM Linux]

On Wed, Jan 04, 2017 at 11:30:25AM +0000, Mark Rutland wrote:
> On Wed, Jan 04, 2017 at 10:19:46AM +0100, Alexander Stein wrote:
> > I'm not sure if the change above works with remove functions set in struct 
> > bus_type too.
> > But on the other hand this would hide errors in drivers which are actually 
> > removable but do not cleanup properly which DEBUG_TEST_DRIVER_REMOVE tries to 
> > detect.
> > By setting .suppress_bind_attrs = true explicitely you state "This 
> > driver cannot be removed!", so the remove callback is not missing by accident.
> 
> I'm not sure I follow. If the remove callback is accidentally missing,
> the driver is not "actually removable" today -- there's either no remove
> code, or it's not been wired up (the latter of which will likely result
> in a compiler warning about an unused function).
> 
> Aborting the remove early in those cases is much safer than forcefully
> removing a driver without a remove callback.

Drivers without a remove function may be removable - there's more layers
than just the driver - there's the bus layer as well, which may or may
not direct to a private-bus pointer.

There's no real way for the core driver model code to know whether the
lack of the ->remove in the struct device_driver is something that
prevents a driver being removed, or whether it's handled via some other
method.  Eg, platform drivers.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v2 2/2] arm: perf: Mark as non-removable

[Mark Rutland]

On Wed, Jan 04, 2017 at 11:40:56AM +0000, Russell King - ARM Linux wrote:
> On Wed, Jan 04, 2017 at 11:30:25AM +0000, Mark Rutland wrote:
> > On Wed, Jan 04, 2017 at 10:19:46AM +0100, Alexander Stein wrote:
> > > I'm not sure if the change above works with remove functions set in struct 
> > > bus_type too.
> > > But on the other hand this would hide errors in drivers which are actually 
> > > removable but do not cleanup properly which DEBUG_TEST_DRIVER_REMOVE tries to 
> > > detect.
> > > By setting .suppress_bind_attrs = true explicitely you state "This 
> > > driver cannot be removed!", so the remove callback is not missing by accident.
> > 
> > I'm not sure I follow. If the remove callback is accidentally missing,
> > the driver is not "actually removable" today -- there's either no remove
> > code, or it's not been wired up (the latter of which will likely result
> > in a compiler warning about an unused function).
> > 
> > Aborting the remove early in those cases is much safer than forcefully
> > removing a driver without a remove callback.
> 
> Drivers without a remove function may be removable - there's more layers
> than just the driver - there's the bus layer as well, which may or may
> not direct to a private-bus pointer.

Sure; which is why I initially suggested doing something at the bus
layer. That way, each layer could do any necessary check, and/or
delegate to a callback for the layer below.

> There's no real way for the core driver model code to know whether the
> lack of the ->remove in the struct device_driver is something that
> prevents a driver being removed, or whether it's handled via some other
> method.  Eg, platform drivers.

While true today, my suggestion is to add the infrastrucutre such that
it can. That seems nicer to me than each driver having to retain
(redundant) state.

Regardless, this patch itself is fine.

Thanks,
Mark.

Re: [PATCH v2 2/2] arm: perf: Mark as non-removable

[Will Deacon]

On Wed, Jan 04, 2017 at 11:46:13AM +0000, Mark Rutland wrote:
> On Wed, Jan 04, 2017 at 11:40:56AM +0000, Russell King - ARM Linux wrote:
> > On Wed, Jan 04, 2017 at 11:30:25AM +0000, Mark Rutland wrote:
> > > On Wed, Jan 04, 2017 at 10:19:46AM +0100, Alexander Stein wrote:
> > > > I'm not sure if the change above works with remove functions set in struct 
> > > > bus_type too.
> > > > But on the other hand this would hide errors in drivers which are actually 
> > > > removable but do not cleanup properly which DEBUG_TEST_DRIVER_REMOVE tries to 
> > > > detect.
> > > > By setting .suppress_bind_attrs = true explicitely you state "This 
> > > > driver cannot be removed!", so the remove callback is not missing by accident.
> > > 
> > > I'm not sure I follow. If the remove callback is accidentally missing,
> > > the driver is not "actually removable" today -- there's either no remove
> > > code, or it's not been wired up (the latter of which will likely result
> > > in a compiler warning about an unused function).
> > > 
> > > Aborting the remove early in those cases is much safer than forcefully
> > > removing a driver without a remove callback.
> > 
> > Drivers without a remove function may be removable - there's more layers
> > than just the driver - there's the bus layer as well, which may or may
> > not direct to a private-bus pointer.
> 
> Sure; which is why I initially suggested doing something at the bus
> layer. That way, each layer could do any necessary check, and/or
> delegate to a callback for the layer below.
> 
> > There's no real way for the core driver model code to know whether the
> > lack of the ->remove in the struct device_driver is something that
> > prevents a driver being removed, or whether it's handled via some other
> > method.  Eg, platform drivers.
> 
> While true today, my suggestion is to add the infrastrucutre such that
> it can. That seems nicer to me than each driver having to retain
> (redundant) state.
> 
> Regardless, this patch itself is fine.

Well, it's also largely incomplete as you point out, so I don't think
we gain an awful lot from merging it as-is.

Will

Re: [PATCH v2 0/2] mark driver as non-removable

[Russell King - ARM Linux]

On Wed, Dec 21, 2016 at 04:03:38PM +0100, Alexander Stein wrote:
> Changes in v2;
> * Instead of adding a remove function which is unused otherwise, mark the driver
>   as non-remoable
> 
> Using DEBUG_TEST_DRIVER_REMOVE every driver gets probed, removed and probed
> again. This breaks drivers without removal function, e.g. arm perf
> resulting in the following dump:
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x6c/0x7c
> sysfs: cannot create duplicate filename '/devices/armv7_cortex_a7'
> Modules linked in:
> CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.9.0+ #212
> Hardware name: Freescale LS1021A
> Backtrace:
> [<8020c044>] (dump_backtrace) from [<8020c2f0>] (show_stack+0x18/0x1c)
>  r7:00000009 r6:60000013 r5:80c1776c r4:00000000
> [<8020c2d8>] (show_stack) from [<8046ead8>] (dump_stack+0x94/0xa8)
> [<8046ea44>] (dump_stack) from [<8021ecd0>] (__warn+0xec/0x104)
>  r7:00000009 r6:8092efc8 r5:00000000 r4:bf04bd80
> [<8021ebe4>] (__warn) from [<8021ed28>] (warn_slowpath_fmt+0x40/0x48)
>  r9:80a32848 r8:00000000 r7:00000000 r6:bf0ab780 r5:8091d590 r4:bf0df000
> [<8021ecec>] (warn_slowpath_fmt) from [<8036d53c>] (sysfs_warn_dup+0x6c/0x7c)
>  r3:bf0df000 r2:8092ef94
> [<8036d4d0>] (sysfs_warn_dup) from [<8036d628>] (sysfs_create_dir_ns+0x8c/0x9c)
>  r6:bf0ab780 r5:bf20ee08 r4:ffffffef
> [<8036d59c>] (sysfs_create_dir_ns) from [<80471860>] (kobject_add_internal+0xa8/0x34c)
>  r6:bf0aa198 r5:00000000 r4:bf20ee08
> [<804717b8>] (kobject_add_internal) from [<80471b54>] (kobject_add+0x50/0x94)
>  r7:00000000 r6:00000000 r5:00000000 r4:bf20ee08
> [<80471b08>] (kobject_add) from [<80524590>] (device_add+0xe4/0x590)
>  r3:00000000 r2:00000000
>  r6:bf20ee00 r5:00000000 r4:bf20ee08
> [<805244ac>] (device_add) from [<802a38a8>] (pmu_dev_alloc+0x88/0xd8)
>  r10:00000091 r9:80a32848 r8:00000000 r7:80a32840 r6:80c0ed3c r5:00000000
>  r4:bf1fe800
> [<802a3820>] (pmu_dev_alloc) from [<80a0cd20>] (perf_event_sysfs_init+0x5c/0xb4)
>  r7:80a32840 r6:00000000 r5:bf1fe800 r4:80c0ede0
> [<80a0ccc4>] (perf_event_sysfs_init) from [<80201778>] (do_one_initcall+0x48/0x178)
>  r6:80c45000 r5:80a0ccc4 r4:00000006
> [<80201730>] (do_one_initcall) from [<80a00ecc>] (kernel_init_freeable+0x168/0x20c)
>  r8:80a00610 r7:80a32840 r6:80c45000 r5:80c45000 r4:00000006
> [<80a00d64>] (kernel_init_freeable) from [<806febcc>] (kernel_init+0x10/0x11c)
>  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:806febbc
>  r4:00000000
> [<806febbc>] (kernel_init) from [<80208388>] (ret_from_fork+0x14/0x2c)
>  r5:806febbc r4:00000000
> ---[ end trace 9d251d389382804f ]---

Please consider putting some of the above in patch 2's description, so
the reason for the patch doesn't get lost.  If you want to reduce the
commit message, you could consider cutting the registers from the
backtrace (which are only useful when doing in-depth debugging, not for
illustrating the callpath.)

I'd like to see acks on both of these before I take patch 2, but I'm not
expecting that to happen until the new year.

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.