PATCH: EDAC - clean up atomic stuff

PATCH: EDAC - clean up atomic stuff

[Alan Cox]

Various proposals were made about the problem of u32 in atomic.h. I've
followed Andi Kleen's comments here - that atomic.h is about atomic_t
not atomic operations in general. I've moved the header bits to edac.h

Avi Kivity also observed the x86_64 one was wrong and I've fixed that
too

Removed the #if 0 unused code

Fixed some typos and coding style


Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.14-rc4-mm1/drivers/edac/edac_mc.c linux-2.6.14-rc4-mm1/drivers/edac/edac_mc.c
--- linux.vanilla-2.6.14-rc4-mm1/drivers/edac/edac_mc.c	2005-10-20 16:12:39.000000000 +0100
+++ linux-2.6.14-rc4-mm1/drivers/edac/edac_mc.c	2005-10-21 11:41:39.000000000 +0100
@@ -31,6 +31,7 @@
 
 #include <asm/uaccess.h>
 #include <asm/page.h>
+#include <asm/edac.h>
 
 #include "edac_mc.h"
 
@@ -67,106 +68,6 @@
 
 #ifdef CONFIG_SYSCTL
 
-#if 0
-static struct mem_ctl_info *find_mc_by_idx(int idx)
-{
-	struct list_head *item;
-	struct mem_ctl_info *mci;
-
-	list_for_each(item, &mc_devices) {
-		mci = list_entry(item, struct mem_ctl_info, link);
-
-		if (mci->mc_idx >= idx) {
-			if (mci->mc_idx == idx)
-				return mci;
-
-			break;
-		}
-	}
-
-	return NULL;
-}
-
-static void dimm_labels(char *buf, void *data)
-{
-	int mcidx, ridx, chidx;
-	char *mcstr, *rstr, *chstr, *lstr, *p;
-	struct mem_ctl_info *mci;
-
-	lstr = buf;
-
-	mcstr = strsep(&lstr, ".");
-	if (!lstr)
-		return;
-	mcidx = simple_strtol(mcstr, &p, 0);
-	if (*p)
-		return;
-	if ((mci = find_mc_by_idx(mcidx)) == NULL)
-		return;
-	rstr = strsep(&lstr, ".");
-	if (!lstr)
-		return;
-	ridx = simple_strtol(rstr, &p, 0);
-	if (*p)
-		return;
-	if ((ridx >= mci->nr_csrows) || !mci->csrows)
-		return;
-
-	chstr = strsep(&lstr, ":");
-	if (!lstr)
-		return;
-	chidx = simple_strtol(chstr, &p, 0);
-	if (*p)
-		return;
-	if ((chidx >= mci->csrows[ridx].nr_channels) ||
-	    !mci->csrows[ridx].channels)
-		return;
-
-	debugf1("%d:%d.%d:%s\n", mcidx, ridx, chidx, lstr);
-
-	strncpy(mci->csrows[ridx].channels[chidx].label, lstr,
-		EDAC_MC_LABEL_LEN + 1);
-	/*
-	 * no need to NULL terminate label since
-	 * get_user_tok() NULL terminates.
-	 */
-}
-
-static void counter_reset(char *buf, void *data)
-{
-	char *p = buf;
-	int mcidx, row, chan;
-	struct mem_ctl_info *mci;
-
-	pci_parity_count = 0;
-
-	mcidx = simple_strtol(buf, &p, 0);
-	if (*p)
-		return;
-
-	down(&mem_ctls_mutex);
-	mci = find_mc_by_idx(mcidx);
-
-	if (mci == NULL)
-		goto out;
-
-	mci->ue_noinfo_count = 0;
-	mci->ce_noinfo_count = 0;
-	mci->ue_count = 0;
-	mci->ce_count = 0;
-	for (row = 0; row < mci->nr_csrows; row++) {
-		struct csrow_info *ri = &mci->csrows[row];
-
-		ri->ue_count = 0;
-		ri->ce_count = 0;
-		for (chan = 0; chan < ri->nr_channels; chan++)
-			ri->channels[chan].ce_count = 0;
-	}
-	mci->start_time = jiffies;
-out:
-	up(&mem_ctls_mutex);
-}
-#endif
 
 static ctl_table mc_table[] = {
 	{-1, "panic_on_ue", &panic_on_ue,
@@ -729,14 +630,13 @@
 }
 
 
-/* FIXME - this should go in an arch dependant file */
 EXPORT_SYMBOL(edac_mc_scrub_block);
 
 void edac_mc_scrub_block(unsigned long page, unsigned long offset,
 			      u32 size)
 {
 	struct page *pg;
-	unsigned long *virt_addr;
+	void *virt_addr;
 
 	debugf3("MC: " __FILE__ ": %s()\n", __func__);
 
@@ -897,7 +797,7 @@
 	debugf3("MC%d: " __FILE__ ": %s()\n", mci->mc_idx, __func__);
 
 	/* FIXME - maybe make panic on INTERNAL ERROR an option */
-	if ((row >= mci->nr_csrows) || (row < 0)) {
+	if (row >= mci->nr_csrows || row < 0) {
 		/* something is wrong */
 		printk(KERN_ERR
 		       "MC%d: INTERNAL ERROR: row out of range (%d >= %d)\n",
@@ -980,7 +880,7 @@
 		if (status & (PCI_STATUS_SIG_SYSTEM_ERROR))
 			printk(KERN_CRIT
 			   	"PCI- "
-				"Signaled System Error on %s %s\n",
+				"Signalled System Error on %s %s\n",
 				dev->dev.bus_id,
 				pci_name(dev));
 
@@ -1029,7 +929,7 @@
 			if (status & (PCI_STATUS_SIG_SYSTEM_ERROR))
 				printk(KERN_CRIT
 					"PCI-Bridge- "
-					"Signaled System Error on %s %s\n",
+					"Signalled System Error on %s %s\n",
 					dev->dev.bus_id,
 					pci_name(dev));
 
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.14-rc4-mm1/include/asm-i386/atomic.h linux-2.6.14-rc4-mm1/include/asm-i386/atomic.h
--- linux.vanilla-2.6.14-rc4-mm1/include/asm-i386/atomic.h	2005-10-20 16:12:41.000000000 +0100
+++ linux-2.6.14-rc4-mm1/include/asm-i386/atomic.h	2005-10-21 11:36:54.000000000 +0100
@@ -237,15 +237,4 @@
 #define smp_mb__before_atomic_inc()	barrier()
 #define smp_mb__after_atomic_inc()	barrier()
 
-/* ECC atomic, DMA, SMP and interrupt safe scrub function */
-
-static __inline__ void atomic_scrub(unsigned long *virt_addr, u32 size)
-{
-	u32 i;
-	for (i = 0; i < size / 4; i++, virt_addr++)
-		/* Very carefully read and write to memory atomically
-		 * so we are interrupt, DMA and SMP safe.
-		 */
-		__asm__ __volatile__("lock; addl $0, %0"::"m"(*virt_addr));
-}
 #endif
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.14-rc4-mm1/include/asm-i386/edac.h linux-2.6.14-rc4-mm1/include/asm-i386/edac.h
--- linux.vanilla-2.6.14-rc4-mm1/include/asm-i386/edac.h	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.14-rc4-mm1/include/asm-i386/edac.h	2005-10-21 11:37:54.000000000 +0100
@@ -0,0 +1,18 @@
+#ifndef ASM_EDAC_H
+#define ASM_EDAC_H
+
+/* ECC atomic, DMA, SMP and interrupt safe scrub function */
+
+static __inline__ void atomic_scrub(void *va, u32 size)
+{
+	unsigned long *virt_addr = va;
+	u32 i;
+
+	for (i = 0; i < size / 4; i++, virt_addr++)
+		/* Very carefully read and write to memory atomically
+		 * so we are interrupt, DMA and SMP safe.
+		 */
+		__asm__ __volatile__("lock; addl $0, %0"::"m"(*virt_addr));
+}
+
+#endif
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.14-rc4-mm1/include/asm-x86_64/atomic.h linux-2.6.14-rc4-mm1/include/asm-x86_64/atomic.h
--- linux.vanilla-2.6.14-rc4-mm1/include/asm-x86_64/atomic.h	2005-10-20 16:12:41.000000000 +0100
+++ linux-2.6.14-rc4-mm1/include/asm-x86_64/atomic.h	2005-10-21 11:38:18.127825024 +0100
@@ -378,16 +378,4 @@
 #define smp_mb__before_atomic_inc()	barrier()
 #define smp_mb__after_atomic_inc()	barrier()
 
-/* ECC atomic, DMA, SMP and interrupt safe scrub function */
-
-static __inline__ void atomic_scrub(u32 *virt_addr, u32 size)
-{
-	u32 i;
-	for (i = 0; i < size / 4; i++, virt_addr++)
-		/* Very carefully read and write to memory atomically
-		 * so we are interrupt, DMA and SMP safe.
-		 */
-		__asm__ __volatile__("lock; addl $0, %0"::"m"(*virt_addr));
-}
-
 #endif
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.14-rc4-mm1/include/asm-x86_64/edac.h linux-2.6.14-rc4-mm1/include/asm-x86_64/edac.h
--- linux.vanilla-2.6.14-rc4-mm1/include/asm-x86_64/edac.h	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.14-rc4-mm1/include/asm-x86_64/edac.h	2005-10-21 11:38:34.000000000 +0100
@@ -0,0 +1,18 @@
+#ifndef ASM_EDAC_H
+#define ASM_EDAC_H
+
+/* ECC atomic, DMA, SMP and interrupt safe scrub function */
+
+static __inline__ void atomic_scrub(void *va, u32 size)
+{
+	unsigned int *virt_addr = va;
+	u32 i;
+
+	for (i = 0; i < size / 4; i++, virt_addr++)
+		/* Very carefully read and write to memory atomically
+		 * so we are interrupt, DMA and SMP safe.
+		 */
+		__asm__ __volatile__("lock; addl $0, %0"::"m"(*virt_addr));
+}
+
+#endif

Re: PATCH: EDAC - clean up atomic stuff

[Eric W. Biederman]

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

> Various proposals were made about the problem of u32 in atomic.h. I've
> followed Andi Kleen's comments here - that atomic.h is about atomic_t
> not atomic operations in general. I've moved the header bits to edac.h
>
> Avi Kivity also observed the x86_64 one was wrong and I've fixed that
> too

First thanks for getting this code merged.

I think I am the original author of this bit of scrub code and
I had thought it had disappeared long ago, because of maintenance
issues.  I at least had an identical implementation.

A couple of questions
- Why a u32 for length and not just unsigned?
- Why is the x86_64 version clearing 32bit words and not 64bit words,
  that should be noticeably faster if we ever need to use that
  code.
- Is KM_BOUNCE_READ a safe atomic_kmap entry to be using?
  I'm not certain, but my gut feel is that scrubbing probably
  wants it's own kmap type. 
  I remember doing some looking when I first wrote this and thinking
  that KM_BOUNCE_READ looked safe and was good enough until the code
  got merged into the kernel.

Eric


> diff -u --new-file --recursive --exclude-from /usr/src/exclude
> linux.vanilla-2.6.14-rc4-mm1/include/asm-i386/atomic.h
> linux-2.6.14-rc4-mm1/include/asm-i386/atomic.h
> --- linux.vanilla-2.6.14-rc4-mm1/include/asm-i386/atomic.h 2005-10-20
> 16:12:41.000000000 +0100
> +++ linux-2.6.14-rc4-mm1/include/asm-i386/atomic.h 2005-10-21 11:36:54.000000000
> +0100
> @@ -237,15 +237,4 @@
>  #define smp_mb__before_atomic_inc()	barrier()
>  #define smp_mb__after_atomic_inc()	barrier()
>  
> -/* ECC atomic, DMA, SMP and interrupt safe scrub function */
> -
> -static __inline__ void atomic_scrub(unsigned long *virt_addr, u32 size)
> -{
> -	u32 i;
> -	for (i = 0; i < size / 4; i++, virt_addr++)
> -		/* Very carefully read and write to memory atomically
> -		 * so we are interrupt, DMA and SMP safe.
> -		 */
> -		__asm__ __volatile__("lock; addl $0, %0"::"m"(*virt_addr));
> -}
>  #endif
> diff -u --new-file --recursive --exclude-from /usr/src/exclude
> linux.vanilla-2.6.14-rc4-mm1/include/asm-i386/edac.h
> linux-2.6.14-rc4-mm1/include/asm-i386/edac.h
> --- linux.vanilla-2.6.14-rc4-mm1/include/asm-i386/edac.h 1970-01-01
> 01:00:00.000000000 +0100
> +++ linux-2.6.14-rc4-mm1/include/asm-i386/edac.h 2005-10-21 11:37:54.000000000
> +0100
> @@ -0,0 +1,18 @@
> +#ifndef ASM_EDAC_H
> +#define ASM_EDAC_H
> +
> +/* ECC atomic, DMA, SMP and interrupt safe scrub function */
> +
> +static __inline__ void atomic_scrub(void *va, u32 size)
> +{
> +	unsigned long *virt_addr = va;
> +	u32 i;
> +
> +	for (i = 0; i < size / 4; i++, virt_addr++)
> +		/* Very carefully read and write to memory atomically
> +		 * so we are interrupt, DMA and SMP safe.
> +		 */
> +		__asm__ __volatile__("lock; addl $0, %0"::"m"(*virt_addr));
> +}
> +
> +#endif
> diff -u --new-file --recursive --exclude-from /usr/src/exclude
> linux.vanilla-2.6.14-rc4-mm1/include/asm-x86_64/edac.h
> linux-2.6.14-rc4-mm1/include/asm-x86_64/edac.h
> --- linux.vanilla-2.6.14-rc4-mm1/include/asm-x86_64/edac.h 1970-01-01
> 01:00:00.000000000 +0100
> +++ linux-2.6.14-rc4-mm1/include/asm-x86_64/edac.h 2005-10-21 11:38:34.000000000
> +0100
> @@ -0,0 +1,18 @@
> +#ifndef ASM_EDAC_H
> +#define ASM_EDAC_H
> +
> +/* ECC atomic, DMA, SMP and interrupt safe scrub function */
> +
> +static __inline__ void atomic_scrub(void *va, u32 size)
> +{
> +	unsigned int *virt_addr = va;
> +	u32 i;
> +
> +	for (i = 0; i < size / 4; i++, virt_addr++)
> +		/* Very carefully read and write to memory atomically
> +		 * so we are interrupt, DMA and SMP safe.
> +		 */
> +		__asm__ __volatile__("lock; addl $0, %0"::"m"(*virt_addr));
> +}
> +
> +#endif
>
> -
> 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/

Re: PATCH: EDAC - clean up atomic stuff

[Alan Cox]

On Gwe, 2005-10-28 at 10:33 -0600, Eric W. Biederman wrote:
> A couple of questions
> - Why a u32 for length and not just unsigned?

Because it was loading it into a 32bit counter so the input was 32bit.
Just habit really.

> - Why is the x86_64 version clearing 32bit words and not 64bit words,
>   that should be noticeably faster if we ever need to use that
>   code.

I doubt it makes much difference. I kept it 32bit to keep the split
simple. It can certainly be optimised if someone wants to. I'd hope
however ECC scrub is never a hot path!

> - Is KM_BOUNCE_READ a safe atomic_kmap entry to be using?
>   I'm not certain, but my gut feel is that scrubbing probably
>   wants it's own kmap type. 
>   I remember doing some looking when I first wrote this and thinking
>   that KM_BOUNCE_READ looked safe and was good enough until the code
>   got merged into the kernel.

I was looking at that. I think it is but I'm not 100% sure or an expert
on kmaps.

Re: PATCH: EDAC - clean up atomic stuff

[Eric W. Biederman]

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

> On Gwe, 2005-10-28 at 10:33 -0600, Eric W. Biederman wrote:
>> A couple of questions
>> - Why a u32 for length and not just unsigned?
>
> Because it was loading it into a 32bit counter so the input was 32bit.
> Just habit really.

Since it wasn't an ABI or poking in hardware a fixed size type felt wrong.
It is more of an API and there fixed sized types (not buried in
typedefs) always seem wrong to me.

>> - Why is the x86_64 version clearing 32bit words and not 64bit words,
>>   that should be noticeably faster if we ever need to use that
>>   code.
>
> I doubt it makes much difference. I kept it 32bit to keep the split
> simple. It can certainly be optimised if someone wants to. I'd hope
> however ECC scrub is never a hot path!

If we ever implement background software scrubbing it might become
important.  On older chipsets you only know the chip select that
had problems so you may need to scrub 128MB just to fix one bit.
There are also things like background scrubbing to ensure single bit
errors don't accumulate.  On x86_64 I don't think either is currently
an issues as the chipsets implement the scrubbing for you.

The basic point still remains that touching a lot of memory is something
that could be reasonably done.  And having the code run relatively
fast keeps it from sucking performance from the rest of the system.

But I agree that on x86_64 this isn't a fastpath, or likely to become
one as the hardware doesn't need software scrubbing.

As long as we atomically dirty the cache line without changing data we
should be good for now.

>> - Is KM_BOUNCE_READ a safe atomic_kmap entry to be using?
>>   I'm not certain, but my gut feel is that scrubbing probably
>>   wants it's own kmap type. 
>>   I remember doing some looking when I first wrote this and thinking
>>   that KM_BOUNCE_READ looked safe and was good enough until the code
>>   got merged into the kernel.
>
> I was looking at that. I think it is but I'm not 100% sure or an expert
> on kmaps.

Ok.  If I recall correctly atomic kmaps have the rule that they are
per cpu, and you can't be interrupted when the map is taken, by
something else that will use the map.  But they are safe to use from
interrupt context.  As I recall the code needs to ensure that an
interrupt handler doesn't use the same buffer and that it isn't
preempted where another thread will use the buffer.  The preemption
angle is new since that piece of code was written.

Anyway thanks for picking this up.

Eric

Re: PATCH: EDAC - clean up atomic stuff

[Andrew Morton]

ebiederm@xmission.com (Eric W. Biederman) wrote:
>
> Ok.  If I recall correctly atomic kmaps have the rule that they are
>  per cpu, and you can't be interrupted when the map is taken, by
>  something else that will use the map.  But they are safe to use from
>  interrupt context.  As I recall the code needs to ensure that an
>  interrupt handler doesn't use the same buffer and that it isn't
>  preempted where another thread will use the buffer.  The preemption
>  angle is new since that piece of code was written.

Yes, a particular atomic kmap slot is simply a static, per-cpu scalar. 
It's just like

int foo[NR_CPUS];

	...
	foo(smp_processor_id());

and all the same rules apply.

The use of KM_BOUNCE_READ does appear to be incorrect.  bounce_copy_vec()
will use KM_BOUNCE_READ from interrupt context, so if the EDAC code is
interrupted by the block layer while it holds that kmap, it will find that
it's suddenly diddling with a different physical page.

So to use KM_BOUNCE_READ, the EDAC code nees to disable local interrupts,
or to use a different (or new) slot.

In what contexts is edac_mc_scrub_block() called?  If process context, then
KM_USER0 would suit.

Ah, edac_mc_scrub_block() is passing the pageframe address to
kunmap_atomic() - that's a common bug.  It needs to pass in the virtual
address which kmap_atomic() returned.

Re: PATCH: EDAC - clean up atomic stuff

[Eric W. Biederman]

Andrew Morton <akpm@osdl.org> writes:

> ebiederm@xmission.com (Eric W. Biederman) wrote:
>>
>> Ok.  If I recall correctly atomic kmaps have the rule that they are
>>  per cpu, and you can't be interrupted when the map is taken, by
>>  something else that will use the map.  But they are safe to use from
>>  interrupt context.  As I recall the code needs to ensure that an
>>  interrupt handler doesn't use the same buffer and that it isn't
>>  preempted where another thread will use the buffer.  The preemption
>>  angle is new since that piece of code was written.
>
> Yes, a particular atomic kmap slot is simply a static, per-cpu scalar. 
> It's just like
>
> int foo[NR_CPUS];
>
> 	...
> 	foo(smp_processor_id());
>
> and all the same rules apply.
>
> The use of KM_BOUNCE_READ does appear to be incorrect.  bounce_copy_vec()
> will use KM_BOUNCE_READ from interrupt context, so if the EDAC code is
> interrupted by the block layer while it holds that kmap, it will find that
> it's suddenly diddling with a different physical page.
>
> So to use KM_BOUNCE_READ, the EDAC code nees to disable local interrupts,
> or to use a different (or new) slot.
>
> In what contexts is edac_mc_scrub_block() called?  If process context, then
> KM_USER0 would suit.

That function is very nice functionality but we could not implement it
properly outside of the kernel.  So the code has been disabled until
just recently. 

Hmm.  Looking at the patch it is most definitely being called from
process context.  Although I think the original was ok from interrupt
context as well.

> Ah, edac_mc_scrub_block() is passing the pageframe address to
> kunmap_atomic() - that's a common bug.  It needs to pass in the virtual
> address which kmap_atomic() returned.

Oops.  Although I am actually surprised kunmap_atomic even needs the address.
Although I can see the kmap type being equally redundant.

There is a much more serious bug there as well.  The code as it
exists is flatly impossible on x86_64 and some other architectures
as they do not support kmap.  It is also broken on x86 as grain can
easily be larger than page size, on old memory controllers where this
is most needed it is the frequently the size of a memory chip select
(aka the size of a single sided DIMM).

We need to do two things.
- Remove a factor from edac_mc_scrub_block (call it edac_mc_scrub_page)
  that simply scrubs a page or maybe a sub page.
- Place the edac_mc_scrub_page which does the kmap and a loop through
  the page contents in arch specific code.

Eric

Re: PATCH: EDAC - clean up atomic stuff

[Eric W. Biederman]

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

>> There is a much more serious bug there as well.  The code as it
>> exists is flatly impossible on x86_64 and some other architectures
>> as they do not support kmap.  It is also broken on x86 as grain can
>
> All platforms have kmap. On systems without "highmem" the kmap functions
> simply return the page address of the existing permanent physical
> mapping for the page. See include/linux/highmem.h

Duh, I just looked again.  I knew we had kmap, I had thought kmap_atomic
was special enough that it wasn't always there.  I'm wrong. 

> So it's all fine and larger than page sized scrubs can be added to the
> core code when they are needed.

The set of memory controllers where software scrubbing is interesting
and the set of memory controllers that need larger than page sized scrubs
intersect quite strongly.  Although I don't think any of those
memory controllers ever migrated over from the old ecc.c code base.
We should at least have a BUG_ON((offset+size) > PAGE_SIZE) so we
don't forget to fix it.

Eric

Re: PATCH: EDAC - clean up atomic stuff

[Alan Cox]

> There is a much more serious bug there as well.  The code as it
> exists is flatly impossible on x86_64 and some other architectures
> as they do not support kmap.  It is also broken on x86 as grain can

All platforms have kmap. On systems without "highmem" the kmap functions
simply return the page address of the existing permanent physical
mapping for the page. See include/linux/highmem.h

So it's all fine and larger than page sized scrubs can be added to the
core code when they are needed.

Alan

Re: PATCH: EDAC - clean up atomic stuff

[Andrew Morton]

ebiederm@xmission.com (Eric W. Biederman) wrote:
>
> Hmm.  Looking at the patch it is most definitely being called from
>  process context.

Well if we retain KM_BOUNCE_READ then we'll need local_irq_save().  If
edac_mc_scrub_block() will alwyas be called from process context then we
can use KM_USER0 and leave interrupts enabled.

Alan, can you confirm please?

Re: PATCH: EDAC - clean up atomic stuff

[Alan Cox]

On Mer, 2005-11-02 at 16:26 +1100, Andrew Morton wrote:
> Well if we retain KM_BOUNCE_READ then we'll need local_irq_save().  If
> edac_mc_scrub_block() will alwyas be called from process context then we
> can use KM_USER0 and leave interrupts enabled.
> 
> Alan, can you confirm please?

Probably best to local_irq_save. Especially considering the likely
direction of updates once it is merged

Alan