Re: [PATCH] powerpc/nvram: Add spinlock to oops_to_nvram to prevent oops in compression code.

2011-12-01 Thread Jim Keniston 
On Thu, 2011-12-01 at 12:46 +1100, Anton Blanchard wrote:
 When issuing a system reset we almost always oops in the oops_to_nvram
 code because multiple CPUs are using the deflate work area. Add a
 spinlock to protect it.
 
 To play it safe I'm using trylock to avoid locking up if the NVRAM
 code oopses. This means we might miss multiple CPUs oopsing at exactly
 the same time but I think it's best to play it safe for now. Once we
 are happy with the reliability we can change it to a full spinlock.
 
 Signed-off-by: Anton Blanchard an...@samba.org

Acked-by: Jim Keniston jkeni...@us.ibm.com

 ---
 
 Index: linux-build/arch/powerpc/platforms/pseries/nvram.c
 ===
 --- linux-build.orig/arch/powerpc/platforms/pseries/nvram.c   2011-12-01 
 09:44:27.205568463 +1100
 +++ linux-build/arch/powerpc/platforms/pseries/nvram.c2011-12-01 
 12:36:49.334478156 +1100
 @@ -634,6 +634,8 @@ static void oops_to_nvram(struct kmsg_du
  {
   static unsigned int oops_count = 0;
   static bool panicking = false;
 + static DEFINE_SPINLOCK(lock);
 + unsigned long flags;
   size_t text_len;
   unsigned int err_type = ERR_TYPE_KERNEL_PANIC_GZ;
   int rc = -1;
 @@ -664,6 +666,9 @@ static void oops_to_nvram(struct kmsg_du
   if (clobbering_unread_rtas_event())
   return;
 
 + if (!spin_trylock_irqsave(lock, flags))
 + return;
 +
   if (big_oops_buf) {
   text_len = capture_last_msgs(old_msgs, old_len,
   new_msgs, new_len, big_oops_buf, big_oops_buf_sz);
 @@ -679,4 +684,6 @@ static void oops_to_nvram(struct kmsg_du
 
   (void) nvram_write_os_partition(oops_log_partition, oops_buf,
   (int) (sizeof(*oops_len) + *oops_len), err_type, ++oops_count);
 +
 + spin_unlock_irqrestore(lock, flags);
  }
 


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc/nvram: Add spinlock to oops_to_nvram to prevent oops in compression code.

2011-11-30 Thread Anton Blanchard 

When issuing a system reset we almost always oops in the oops_to_nvram
code because multiple CPUs are using the deflate work area. Add a
spinlock to protect it.

To play it safe I'm using trylock to avoid locking up if the NVRAM
code oopses. This means we might miss multiple CPUs oopsing at exactly
the same time but I think it's best to play it safe for now. Once we
are happy with the reliability we can change it to a full spinlock.

Signed-off-by: Anton Blanchard an...@samba.org
---

Index: linux-build/arch/powerpc/platforms/pseries/nvram.c
===
--- linux-build.orig/arch/powerpc/platforms/pseries/nvram.c 2011-12-01 
09:44:27.205568463 +1100
+++ linux-build/arch/powerpc/platforms/pseries/nvram.c  2011-12-01 
12:36:49.334478156 +1100
@@ -634,6 +634,8 @@ static void oops_to_nvram(struct kmsg_du
 {
static unsigned int oops_count = 0;
static bool panicking = false;
+   static DEFINE_SPINLOCK(lock);
+   unsigned long flags;
size_t text_len;
unsigned int err_type = ERR_TYPE_KERNEL_PANIC_GZ;
int rc = -1;
@@ -664,6 +666,9 @@ static void oops_to_nvram(struct kmsg_du
if (clobbering_unread_rtas_event())
return;
 
+   if (!spin_trylock_irqsave(lock, flags))
+   return;
+
if (big_oops_buf) {
text_len = capture_last_msgs(old_msgs, old_len,
new_msgs, new_len, big_oops_buf, big_oops_buf_sz);
@@ -679,4 +684,6 @@ static void oops_to_nvram(struct kmsg_du
 
(void) nvram_write_os_partition(oops_log_partition, oops_buf,
(int) (sizeof(*oops_len) + *oops_len), err_type, ++oops_count);
+
+   spin_unlock_irqrestore(lock, flags);
 }
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/nvram: Add spinlock to oops_to_nvram to prevent oops in compression code.

2011-11-30 Thread Benjamin Herrenschmidt 
On Thu, 2011-12-01 at 12:46 +1100, Anton Blanchard wrote:
 When issuing a system reset we almost always oops in the oops_to_nvram
 code because multiple CPUs are using the deflate work area. Add a
 spinlock to protect it.
 
 To play it safe I'm using trylock to avoid locking up if the NVRAM
 code oopses. This means we might miss multiple CPUs oopsing at exactly
 the same time but I think it's best to play it safe for now. Once we
 are happy with the reliability we can change it to a full spinlock.

How would we miss ?

trylock does loop on stwcx. failure, it doesn't loop if the lock is
-taken-, so if the lock is only used for actually dealing with the oops
the only miss is because somebody already got it... or am I missing
something ?

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/nvram: Add spinlock to oops_to_nvram to prevent oops in compression code.

2011-11-30 Thread Anton Blanchard 
Hi Ben,

 How would we miss ?
 
 trylock does loop on stwcx. failure, it doesn't loop if the lock is
 -taken-, so if the lock is only used for actually dealing with the
 oops the only miss is because somebody already got it... or am I
 missing something ?

I'm thinking of two CPUs that enter at exactly the same time either
through a system reset or an ugly bug (writing junk at 0x900 so the
decrementer exception gets an oops). Probably unlikely enough that we
don't care.

Anton
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev