dle-develop — Dependable Linux Effort mailing list for developers

Re: [Dle-develop] efi_pstore: question about how to remove create_sysfs_entry() from a write callback.

[Seiji A. <sei...@hd...>]

Mike,

Previous pseudo code was not correct.
More precisely, it is as follows.
But, we still need to alloc memory dynamically in the part of <Save information about new entry>
because a workqueue can't know how many new entries in advance.

 spin_lock_irqsave(&efivars_lock);
 
 do {
 	get_next_variable();
 	if (found new entry)
  		<Save information about new entry>
 
 } while()
 
 spin_lock_irqrestore(&efivars_lock);

  efivars_create_sysfs_entries();

Seiji

> -----Original Message-----
> From: lin...@vg... [mailto:lin...@vg...] On Behalf Of Seiji Aguchi
> Sent: Tuesday, August 14, 2012 2:52 PM
> To: Mike Waychison
> Cc: lin...@vg...; Luck, Tony (ton...@in...); Matthew Garrett (mj...@re...); dz...@re...; dle-
> de...@li...; Satoru Moriya
> Subject: RE: efi_pstore: question about how to remove create_sysfs_entry() from a write callback.
> 
> > Can we not serialize this with &efivars->lock if it is updated to
> > disable interrupts?  A loop in the workqueue that locks, iterates
> > through
> > ->get_next_variable, and compares against searches in
> > efivars->list should work, no?
> 
> If my understanding is correct, your pseudo code is as follows.
> 
> spin_lock_irqsave(&efivars_lock);
> 
> do {
> 	get_next_variable();
> 	if (found new entry)
>  		efivars_create_sysfs_entries();
> 
> } while()
> 
> spin_lock_irqrestore(&efivars_lock);
> 
> But, memory is allocated dynamically with kzalloc() in efivars_create_sysfs_entries().
> I don't want to allocate memory  while holding spin_lock.
> 
> Please let me know if I'm missing something.
> 
> Seiji
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to maj...@vg... More
> majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [Dle-develop] efi_pstore: question about how to remove create_sysfs_entry() from a write callback.

[Seiji A. <sei...@hd...>]

> Can we not serialize this with &efivars->lock if it is updated to disable interrupts?  A loop in the workqueue that locks, iterates through
> ->get_next_variable, and compares against searches in
> efivars->list should work, no?

If my understanding is correct, your pseudo code is as follows.

spin_lock_irqsave(&efivars_lock);

do {
	get_next_variable();
	if (found new entry)
 		efivars_create_sysfs_entries();

} while()

spin_lock_irqrestore(&efivars_lock);

But, memory is allocated dynamically with kzalloc() in efivars_create_sysfs_entries().
I don't want to allocate memory  while holding spin_lock.

Please let me know if I'm missing something.

Seiji 

Re: [Dle-develop] efi_pstore: question about how to remove create_sysfs_entry() from a write callback.

[Mike W. <mi...@go...>]

On Tue, Aug 14, 2012 at 9:05 AM, Seiji Aguchi <sei...@hd...> wrote:
> Hi,
>
> I'm sending an email to discuss how to remove create_sysfs_entry() from a write callback.
>
> [Problem]
>
> Current efi_pstore creates sysfs entries ,which enable users to access to NVRAM, in a write callback.
> If a kernel panic happens in interrupt contexts, pstore may fail because it could sleep due to dynamic
> memory allocations during creating sysfs entries.
>
> To resolve the problem above, my goal here is removing create_sysfs_entry() from a write callback.
>
> [Ideas]
>
>  (1) Introduce a workqueue updating sysfs entries
>
>      To remove create_sysfs_entry() from a write callback,
>      It seems to be possible if efi_pstore updates its sysfs files
>      by scanning existing entries in NVRAM with a GetNextVariable()
>      in a workqueue.
>
>
>      I created a prototype patch based on an idea above but can't avoid a race
>      between SetVariable() in a write callback and GetNextVariable() in a workqueue.
>      It is not guaranteed by EFI specification.
>
>      EFI 2.3.1 specification, page 217.
>      <snip>
>      Calls to SetVariable() between calls to
>      GetNextVariableName() may produce unpredictable results.
>      <snip>

Can we not serialize this with &efivars->lock if it is updated to
disable interrupts?  A loop in the workqueue that locks, iterates
through ->get_next_variable, and compares against searches in
efivars->list should work, no?

>
>
>  (2) Don't support sysfs entries in efi_pstore.
>
>      Another idea is _not_ updating sysfs entries at all in efi_pstore.
>      This can avoid a race SetVariable() and GetNextVariable().
>
>      write callback
>        - simply write a new entry with SetVariable().
>          - This fits a discussion about holding multiple logs in a thread below.
>            http://marc.info/?l=linux-kernel&m=134316268011854&w=2
>
>      erase callback
>        - simply erase an existing entry with SetVariable().
>
>      read callback
>        - Scaning existing entries with GetNextVariable().
>          We can avoid a race between GetNextVariable() in a read callback
>          and SetVariable() in a write/erase callback by protecting them with efi_lock.
>
>  IMO, idea (2) is reasonable because we already have an interface, /dev/pstore, which users can access
>  to NVRAM and we don't need to support multiple user interfaces.
>
> Any comments are welcome.
>
> Seiji

[Dle-develop] efi_pstore: question about how to remove create_sysfs_entry() from a write callback.

[Seiji A. <sei...@hd...>]

Hi,

I'm sending an email to discuss how to remove create_sysfs_entry() from a write callback.

[Problem]

Current efi_pstore creates sysfs entries ,which enable users to access to NVRAM, in a write callback.
If a kernel panic happens in interrupt contexts, pstore may fail because it could sleep due to dynamic 
memory allocations during creating sysfs entries.

To resolve the problem above, my goal here is removing create_sysfs_entry() from a write callback.

[Ideas]

 (1) Introduce a workqueue updating sysfs entries

     To remove create_sysfs_entry() from a write callback,
     It seems to be possible if efi_pstore updates its sysfs files 
     by scanning existing entries in NVRAM with a GetNextVariable()
     in a workqueue.
     

     I created a prototype patch based on an idea above but can't avoid a race 
     between SetVariable() in a write callback and GetNextVariable() in a workqueue.
     It is not guaranteed by EFI specification.

     EFI 2.3.1 specification, page 217. 
     <snip>
     Calls to SetVariable() between calls to
     GetNextVariableName() may produce unpredictable results.
     <snip>


 (2) Don't support sysfs entries in efi_pstore.

     Another idea is _not_ updating sysfs entries at all in efi_pstore.
     This can avoid a race SetVariable() and GetNextVariable().

     write callback
       - simply write a new entry with SetVariable().
         - This fits a discussion about holding multiple logs in a thread below.
           http://marc.info/?l=linux-kernel&m=134316268011854&w=2 

     erase callback
       - simply erase an existing entry with SetVariable().

     read callback
       - Scaning existing entries with GetNextVariable().
         We can avoid a race between GetNextVariable() in a read callback 
         and SetVariable() in a write/erase callback by protecting them with efi_lock.

 IMO, idea (2) is reasonable because we already have an interface, /dev/pstore, which users can access
 to NVRAM and we don't need to support multiple user interfaces.

Any comments are welcome.

Seiji

Re: [Dle-develop] [PATCH V3] trace, x86: add x86 irq vector tracepoints

[Seiji A. <sei...@hd...>]

Thomas,

Could you please review my patch?

I updated it in accordance with your comment.

Seiji

> -----Original Message-----
> From: Seiji Aguchi
> Sent: Friday, August 03, 2012 3:50 PM
> To: lin...@vg...; Thomas Gleixner (tg...@li...); ro...@go...; 'mi...@el...' (mi...@el...);
> x8...@ke...
> Cc: dle...@li...; Satoru Moriya
> Subject: [PATCH V3] trace,x86: add x86 irq vector tracepoints
> 
> Change log
>  v2 -> v3
>  - Remove an invalidate_tlb_vector event because it was replaced by a call function vector
>    in a following commit.
>    http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=52aec3308db85f4e9f5c8b9f5dc4fbd0138c6fa4
> 
>  v1 -> v2
>  - Modify variable name from irq to vector.
>  - Merge arch-specific tracepoints below to an arch_irq_vector_entry/exit.
>    - error_apic_vector
>    - thermal_apic_vector
>    - threshold_apic_vector
>    - spurious_apic_vector
>    - x86_platform_ipi_vector
> 
> As Vaibhav explained in the thread below, tracepoints for irq vectors are useful.
> 
> http://www.spinics.net/lists/mm-commits/msg85707.html
> 
> <snip>
> The current interrupt traces from irq_handler_entry and irq_handler_exit provide when an interrupt is handled.  They provide good
> data about when the system has switched to kernel space and how it affects the currently running processes.
> 
> There are some IRQ vectors which trigger the system into kernel space, which are not handled in generic IRQ handlers.  Tracing such
> events gives us the information about IRQ interaction with other system events.
> 
> The trace also tells where the system is spending its time.  We want to know which cores are handling interrupts and how they are
> affecting other processes in the system.  Also, the trace provides information about when the cores are idle and which interrupts are
> changing that state.
> <snip>
> 
> On the other hand, my usecase is tracing just local timer event and getting a value of instruction pointer.
> 
>   I suggested to add an argument local timer event to get instruction pointer before.
>   But there is another way to get it with external module like systemtap.
>   So, I don't need to add any argument to irq vector tracepoints now.
> 
> Vaibhav's patch shared a trace point ,irq_vector_entry/irq_vector_exit, in all events.
> But there is an above use case to trace specific irq_vector rather than tracing all events.
> In this case, we are concerned about overhead due to unwanted events.
> 
> This patch modifies Vaibhav's one as follows.
>  - Separate generic, and across-architecture tracepoints to enable independently.
>    - nmi_vector
>    - local_timer_vector
>    - reschedule_vector
>    - call_function_vector
>    - call_function_single_vector
>    - irq_work_entry_vector
> 
>  - Rename architecture-specific tracepoints from irq_vector_entry/exit to
>    arch_irq_vector_entry/exit.
>    - error_apic_vector
>    - thermal_apic_vector
>    - threshold_apic_vector
>    - spurious_apic_vector
>    - x86_platform_ipi_vector
> 
>    Those x86 specific ones are not really frequently raised vectors, so
>    enabling them all won't affect performance and readability of the
>    traces too much.
> 
>  Signed-off-by: Seiji Aguchi <sei...@hd...>
> 
> ---
>  arch/x86/include/asm/irq_vectors.h       |    9 ++
>  arch/x86/kernel/apic/apic.c              |    7 +
>  arch/x86/kernel/cpu/mcheck/therm_throt.c |    3 +
>  arch/x86/kernel/cpu/mcheck/threshold.c   |    3 +
>  arch/x86/kernel/irq.c                    |    5 +
>  arch/x86/kernel/irq_work.c               |    3 +
>  arch/x86/kernel/nmi.c                    |    3 +
>  arch/x86/kernel/smp.c                    |    7 +
>  include/trace/events/irq_vectors.h       |  209 ++++++++++++++++++++++++++++++
>  9 files changed, 249 insertions(+), 0 deletions(-)  create mode 100644 include/trace/events/irq_vectors.h
> 
> diff --git a/arch/x86/include/asm/irq_vectors.h b/arch/x86/include/asm/irq_vectors.h
> index 1508e51..510ced5 100644
> --- a/arch/x86/include/asm/irq_vectors.h
> +++ b/arch/x86/include/asm/irq_vectors.h
> @@ -158,4 +158,13 @@ static inline int invalid_vm86_irq(int irq)
>  # define NR_IRQS			NR_IRQS_LEGACY
>  #endif
> 
> +#define irq_vector_name(vector) { vector, #vector }
> +
> +#define irq_vector_name_table						\
> +			irq_vector_name(ERROR_APIC_VECTOR),		\
> +			irq_vector_name(THERMAL_APIC_VECTOR),		\
> +			irq_vector_name(THRESHOLD_APIC_VECTOR),		\
> +			irq_vector_name(SPURIOUS_APIC_VECTOR),		\
> +			irq_vector_name(X86_PLATFORM_IPI_VECTOR)
> +
>  #endif /* _ASM_X86_IRQ_VECTORS_H */
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c index 24deb30..b9cdd8f 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -34,6 +34,7 @@
>  #include <linux/dmi.h>
>  #include <linux/smp.h>
>  #include <linux/mm.h>
> +#include <trace/events/irq_vectors.h>
> 
>  #include <asm/irq_remapping.h>
>  #include <asm/perf_event.h>
> @@ -895,7 +896,9 @@ void __irq_entry smp_apic_timer_interrupt(struct pt_regs *regs)
>  	 */
>  	irq_enter();
>  	exit_idle();
> +	trace_local_timer_entry(LOCAL_TIMER_VECTOR);
>  	local_apic_timer_interrupt();
> +	trace_local_timer_exit(LOCAL_TIMER_VECTOR);
>  	irq_exit();
> 
>  	set_irq_regs(old_regs);
> @@ -1881,6 +1884,7 @@ void smp_spurious_interrupt(struct pt_regs *regs)
> 
>  	irq_enter();
>  	exit_idle();
> +	trace_arch_irq_vector_entry(SPURIOUS_APIC_VECTOR);
>  	/*
>  	 * Check if this really is a spurious interrupt and ACK it
>  	 * if it is a vectored one.  Just in case...
> @@ -1895,6 +1899,7 @@ void smp_spurious_interrupt(struct pt_regs *regs)
>  	/* see sw-dev-man vol 3, chapter 7.4.13.5 */
>  	pr_info("spurious APIC interrupt on CPU#%d, "
>  		"should never happen.\n", smp_processor_id());
> +	trace_arch_irq_vector_exit(SPURIOUS_APIC_VECTOR);
>  	irq_exit();
>  }
> 
> @@ -1918,6 +1923,7 @@ void smp_error_interrupt(struct pt_regs *regs)
> 
>  	irq_enter();
>  	exit_idle();
> +	trace_arch_irq_vector_entry(ERROR_APIC_VECTOR);
>  	/* First tickle the hardware, only then report what went on. -- REW */
>  	v0 = apic_read(APIC_ESR);
>  	apic_write(APIC_ESR, 0);
> @@ -1938,6 +1944,7 @@ void smp_error_interrupt(struct pt_regs *regs)
> 
>  	apic_printk(APIC_DEBUG, KERN_CONT "\n");
> 
> +	trace_arch_irq_vector_exit(ERROR_APIC_VECTOR);
>  	irq_exit();
>  }
> 
> diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> index 47a1870..63c2cc8 100644
> --- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
> +++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> @@ -23,6 +23,7 @@
>  #include <linux/init.h>
>  #include <linux/smp.h>
>  #include <linux/cpu.h>
> +#include <trace/events/irq_vectors.h>
> 
>  #include <asm/processor.h>
>  #include <asm/apic.h>
> @@ -382,8 +383,10 @@ asmlinkage void smp_thermal_interrupt(struct pt_regs *regs)  {
>  	irq_enter();
>  	exit_idle();
> +	trace_arch_irq_vector_entry(THERMAL_APIC_VECTOR);
>  	inc_irq_stat(irq_thermal_count);
>  	smp_thermal_vector();
> +	trace_arch_irq_vector_exit(THERMAL_APIC_VECTOR);
>  	irq_exit();
>  	/* Ack only at the end to avoid potential reentry */
>  	ack_APIC_irq();
> diff --git a/arch/x86/kernel/cpu/mcheck/threshold.c b/arch/x86/kernel/cpu/mcheck/threshold.c
> index aa578ca..de74768 100644
> --- a/arch/x86/kernel/cpu/mcheck/threshold.c
> +++ b/arch/x86/kernel/cpu/mcheck/threshold.c
> @@ -3,6 +3,7 @@
>   */
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> +#include <trace/events/irq_vectors.h>
> 
>  #include <asm/irq_vectors.h>
>  #include <asm/apic.h>
> @@ -21,8 +22,10 @@ asmlinkage void smp_threshold_interrupt(void)  {
>  	irq_enter();
>  	exit_idle();
> +	trace_arch_irq_vector_entry(THRESHOLD_APIC_VECTOR);
>  	inc_irq_stat(irq_threshold_count);
>  	mce_threshold_vector();
> +	trace_arch_irq_vector_exit(THRESHOLD_APIC_VECTOR);
>  	irq_exit();
>  	/* Ack only at the end to avoid potential reentry */
>  	ack_APIC_irq();
> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c index 1f5f1d5..f4d7344 100644
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -18,6 +18,9 @@
>  #include <asm/mce.h>
>  #include <asm/hw_irq.h>
> 
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/irq_vectors.h>
> +
>  atomic_t irq_err_count;
> 
>  /* Function pointer for generic interrupt vector handling */ @@ -218,11 +221,13 @@ void smp_x86_platform_ipi(struct pt_regs *regs)
> 
>  	exit_idle();
> 
> +	trace_arch_irq_vector_entry(X86_PLATFORM_IPI_VECTOR);
>  	inc_irq_stat(x86_platform_ipis);
> 
>  	if (x86_platform_ipi_callback)
>  		x86_platform_ipi_callback();
> 
> +	trace_arch_irq_vector_exit(X86_PLATFORM_IPI_VECTOR);
>  	irq_exit();
> 
>  	set_irq_regs(old_regs);
> diff --git a/arch/x86/kernel/irq_work.c b/arch/x86/kernel/irq_work.c index ca8f703..2cf7505 100644
> --- a/arch/x86/kernel/irq_work.c
> +++ b/arch/x86/kernel/irq_work.c
> @@ -8,13 +8,16 @@
>  #include <linux/irq_work.h>
>  #include <linux/hardirq.h>
>  #include <asm/apic.h>
> +#include <trace/events/irq_vectors.h>
> 
>  void smp_irq_work_interrupt(struct pt_regs *regs)  {
>  	irq_enter();
>  	ack_APIC_irq();
> +	trace_irq_work_entry(IRQ_WORK_VECTOR);
>  	inc_irq_stat(apic_irq_work_irqs);
>  	irq_work_run();
> +	trace_irq_work_exit(IRQ_WORK_VECTOR);
>  	irq_exit();
>  }
> 
> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c index f84f5c5..cc57aba 100644
> --- a/arch/x86/kernel/nmi.c
> +++ b/arch/x86/kernel/nmi.c
> @@ -28,6 +28,7 @@
>  #include <asm/mach_traps.h>
>  #include <asm/nmi.h>
>  #include <asm/x86_init.h>
> +#include <trace/events/irq_vectors.h>
> 
>  struct nmi_desc {
>  	spinlock_t lock;
> @@ -482,12 +483,14 @@ do_nmi(struct pt_regs *regs, long error_code)
>  	nmi_nesting_preprocess(regs);
> 
>  	nmi_enter();
> +	trace_nmi_entry(NMI_VECTOR);
> 
>  	inc_irq_stat(__nmi_count);
> 
>  	if (!ignore_nmis)
>  		default_do_nmi(regs);
> 
> +	trace_nmi_exit(NMI_VECTOR);
>  	nmi_exit();
> 
>  	/* On i386, may loop back to preprocess */ diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c index
> 48d2b7d..5b2d6de 100644
> --- a/arch/x86/kernel/smp.c
> +++ b/arch/x86/kernel/smp.c
> @@ -23,6 +23,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/cpu.h>
>  #include <linux/gfp.h>
> +#include <trace/events/irq_vectors.h>
> 
>  #include <asm/mtrr.h>
>  #include <asm/tlbflush.h>
> @@ -252,8 +253,10 @@ finish:
>  void smp_reschedule_interrupt(struct pt_regs *regs)  {
>  	ack_APIC_irq();
> +	trace_reschedule_entry(RESCHEDULE_VECTOR);
>  	inc_irq_stat(irq_resched_count);
>  	scheduler_ipi();
> +	trace_reschedule_exit(RESCHEDULE_VECTOR);
>  	/*
>  	 * KVM uses this interrupt to force a cpu out of guest mode
>  	 */
> @@ -263,8 +266,10 @@ void smp_call_function_interrupt(struct pt_regs *regs)  {
>  	ack_APIC_irq();
>  	irq_enter();
> +	trace_call_function_entry(CALL_FUNCTION_VECTOR);
>  	generic_smp_call_function_interrupt();
>  	inc_irq_stat(irq_call_count);
> +	trace_call_function_exit(CALL_FUNCTION_VECTOR);
>  	irq_exit();
>  }
> 
> @@ -272,8 +277,10 @@ void smp_call_function_single_interrupt(struct pt_regs *regs)  {
>  	ack_APIC_irq();
>  	irq_enter();
> +	trace_call_function_single_entry(CALL_FUNCTION_SINGLE_VECTOR);
>  	generic_smp_call_function_single_interrupt();
>  	inc_irq_stat(irq_call_count);
> +	trace_call_function_single_exit(CALL_FUNCTION_SINGLE_VECTOR);
>  	irq_exit();
>  }
> 
> diff --git a/include/trace/events/irq_vectors.h b/include/trace/events/irq_vectors.h
> new file mode 100644
> index 0000000..fffe0c0
> --- /dev/null
> +++ b/include/trace/events/irq_vectors.h
> @@ -0,0 +1,209 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM irq_vectors
> +
> +#if !defined(_TRACE_IRQ_VECTORS_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_IRQ_VECTORS_H
> +
> +#include <linux/tracepoint.h>
> +#include <asm/irq.h>
> +
> +#ifndef irq_vector_name_table
> +#define irq_vector_name_table { -1, NULL } #endif
> +
> +
> +/*
> + * This class is used by generic ,cross-architecture tracepoints.
> + */
> +DECLARE_EVENT_CLASS(irq_vector,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector),
> +
> +	TP_STRUCT__entry(
> +		__field(	int,	vector	)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->vector = vector;
> +	),
> +
> +	TP_printk("vector=%d", __entry->vector) );
> +
> +/*
> + * nmi_entry - called before enterring a nmi vector handler  */
> +DEFINE_EVENT(irq_vector, nmi_entry,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * nmi_exit - called immediately after the interrupt vector
> + * handler returns
> + */
> +DEFINE_EVENT(irq_vector, nmi_exit,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * local_timer_entry - called before enterring a local timer interrupt
> + * vector handler
> + */
> +DEFINE_EVENT(irq_vector, local_timer_entry,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * local_timer_exit - called immediately after the interrupt vector
> + * handler returns
> + */
> +DEFINE_EVENT(irq_vector, local_timer_exit,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * reschedule_entry - called before enterring a reschedule vector
> +handler  */ DEFINE_EVENT(irq_vector, reschedule_entry,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * reschedule_exit - called immediately after the interrupt vector
> + * handler returns
> + */
> +DEFINE_EVENT(irq_vector, reschedule_exit,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * call_function_entry - called before enterring a call function
> + *			 vector handler
> + */
> +DEFINE_EVENT(irq_vector, call_function_entry,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * call_function_exit - called immediately after the interrupt vector
> + * handler returns
> + */
> +DEFINE_EVENT(irq_vector, call_function_exit,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * call_function_single_entry - called before enterring a call function
> + * single vector handler
> + */
> +DEFINE_EVENT(irq_vector, call_function_single_entry,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * call_function_single_exit - called immediately after the interrupt
> +vector
> + * handler returns
> + */
> +DEFINE_EVENT(irq_vector, call_function_single_exit,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * irq_work_entry - called before enterring an irq work vector handler
> +*/ DEFINE_EVENT(irq_vector, irq_work_entry,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * irq_work_exit - called immediately after the interrupt vector
> + * handler returns
> + */
> +DEFINE_EVENT(irq_vector, irq_work_exit,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * This class is used by arch-specific tracepoints.
> + */
> +DECLARE_EVENT_CLASS(arch_irq_vector,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector),
> +
> +	TP_STRUCT__entry(
> +		__field(	int,	vector	)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->vector = vector;
> +	),
> +
> +	TP_printk("vector=%d name=%s", __entry->vector,
> +		__print_symbolic(__entry->vector, irq_vector_name_table)) );
> +
> +/*
> + * arch_irq_vector_entry - called before enterring a interrupt vector
> +handler  */ DEFINE_EVENT(arch_irq_vector, arch_irq_vector_entry,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +/*
> + * arch_irq_vector_exit - called immediately after the interrupt vector
> + * handler returns
> + */
> +DEFINE_EVENT(arch_irq_vector, arch_irq_vector_exit,
> +
> +	TP_PROTO(int vector),
> +
> +	TP_ARGS(vector)
> +);
> +
> +#endif /*  _TRACE_IRQ_VECTORS_H */
> +
> +/* This part must be outside protection */ #include
> +<trace/define_trace.h>
> -- 1.7.1

[Dle-develop] [RESEND][PATCH]efi: initialize efi.runtime_version to make query_variable_info/update_capsule workable

[Seiji A. <sei...@hd...>]

A value of efi.runtime_version is checked before calling
update_capsule()/query_variable_info() as follows.
But it isn't initialized anywhere.
<snip>
static efi_status_t virt_efi_query_variable_info(u32 attr,
                                                 u64 *storage_space,
                                                  u64 *remaining_space,
                                                 u64 *max_variable_size) {
        if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
                return EFI_UNSUPPORTED; 
<snip>

This patch initializes a value of efi.runtime_version at boot time.

Acked-by: Matthew Garrett <mj...@re...>
Signed-off-by: Seiji Aguchi <sei...@hd...>
---
 arch/x86/platform/efi/efi.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 2dc29f5..4c82998 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -868,6 +868,7 @@ void __init efi_enter_virtual_mode(void)
 	 *
 	 * Call EFI services through wrapper functions.
 	 */
+	efi.runtime_version = efi_systab.fw_revision;
 	efi.get_time = virt_efi_get_time;
 	efi.set_time = virt_efi_set_time;
 	efi.get_wakeup_time = virt_efi_get_wakeup_time;
-- 
1.7.1

[Dle-develop] [PATCH V3] trace, x86: add x86 irq vector tracepoints

[Seiji A. <sei...@hd...>]

Change log 
 v2 -> v3
 - Remove an invalidate_tlb_vector event because it was replaced by a call function vector
   in a following commit.
   http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=52aec3308db85f4e9f5c8b9f5dc4fbd0138c6fa4

 v1 -> v2
 - Modify variable name from irq to vector.
 - Merge arch-specific tracepoints below to an arch_irq_vector_entry/exit.
   - error_apic_vector
   - thermal_apic_vector
   - threshold_apic_vector
   - spurious_apic_vector
   - x86_platform_ipi_vector

As Vaibhav explained in the thread below, tracepoints for irq vectors
are useful.

http://www.spinics.net/lists/mm-commits/msg85707.html

<snip>
The current interrupt traces from irq_handler_entry and irq_handler_exit
provide when an interrupt is handled.  They provide good data about when
the system has switched to kernel space and how it affects the currently
running processes.

There are some IRQ vectors which trigger the system into kernel space,
which are not handled in generic IRQ handlers.  Tracing such events gives
us the information about IRQ interaction with other system events.

The trace also tells where the system is spending its time.  We want to
know which cores are handling interrupts and how they are affecting other
processes in the system.  Also, the trace provides information about when
the cores are idle and which interrupts are changing that state.
<snip>

On the other hand, my usecase is tracing just local timer event and 
getting a value of instruction pointer.

  I suggested to add an argument local timer event to get instruction pointer before.
  But there is another way to get it with external module like systemtap.
  So, I don't need to add any argument to irq vector tracepoints now.

Vaibhav's patch shared a trace point ,irq_vector_entry/irq_vector_exit, in all events.
But there is an above use case to trace specific irq_vector rather than tracing all events.
In this case, we are concerned about overhead due to unwanted events.

This patch modifies Vaibhav's one as follows.
 - Separate generic, and across-architecture tracepoints to enable independently.
   - nmi_vector
   - local_timer_vector
   - reschedule_vector
   - call_function_vector
   - call_function_single_vector 
   - irq_work_entry_vector

 - Rename architecture-specific tracepoints from irq_vector_entry/exit to 
   arch_irq_vector_entry/exit.
   - error_apic_vector
   - thermal_apic_vector
   - threshold_apic_vector
   - spurious_apic_vector
   - x86_platform_ipi_vector

   Those x86 specific ones are not really frequently raised vectors, so
   enabling them all won't affect performance and readability of the
   traces too much.

 Signed-off-by: Seiji Aguchi <sei...@hd...>

---
 arch/x86/include/asm/irq_vectors.h       |    9 ++
 arch/x86/kernel/apic/apic.c              |    7 +
 arch/x86/kernel/cpu/mcheck/therm_throt.c |    3 +
 arch/x86/kernel/cpu/mcheck/threshold.c   |    3 +
 arch/x86/kernel/irq.c                    |    5 +
 arch/x86/kernel/irq_work.c               |    3 +
 arch/x86/kernel/nmi.c                    |    3 +
 arch/x86/kernel/smp.c                    |    7 +
 include/trace/events/irq_vectors.h       |  209 ++++++++++++++++++++++++++++++
 9 files changed, 249 insertions(+), 0 deletions(-)
 create mode 100644 include/trace/events/irq_vectors.h

diff --git a/arch/x86/include/asm/irq_vectors.h b/arch/x86/include/asm/irq_vectors.h
index 1508e51..510ced5 100644
--- a/arch/x86/include/asm/irq_vectors.h
+++ b/arch/x86/include/asm/irq_vectors.h
@@ -158,4 +158,13 @@ static inline int invalid_vm86_irq(int irq)
 # define NR_IRQS			NR_IRQS_LEGACY
 #endif
 
+#define irq_vector_name(vector) { vector, #vector }
+
+#define irq_vector_name_table						\
+			irq_vector_name(ERROR_APIC_VECTOR),		\
+			irq_vector_name(THERMAL_APIC_VECTOR),		\
+			irq_vector_name(THRESHOLD_APIC_VECTOR),		\
+			irq_vector_name(SPURIOUS_APIC_VECTOR),		\
+			irq_vector_name(X86_PLATFORM_IPI_VECTOR)
+
 #endif /* _ASM_X86_IRQ_VECTORS_H */
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 24deb30..b9cdd8f 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -34,6 +34,7 @@
 #include <linux/dmi.h>
 #include <linux/smp.h>
 #include <linux/mm.h>
+#include <trace/events/irq_vectors.h>
 
 #include <asm/irq_remapping.h>
 #include <asm/perf_event.h>
@@ -895,7 +896,9 @@ void __irq_entry smp_apic_timer_interrupt(struct pt_regs *regs)
 	 */
 	irq_enter();
 	exit_idle();
+	trace_local_timer_entry(LOCAL_TIMER_VECTOR);
 	local_apic_timer_interrupt();
+	trace_local_timer_exit(LOCAL_TIMER_VECTOR);
 	irq_exit();
 
 	set_irq_regs(old_regs);
@@ -1881,6 +1884,7 @@ void smp_spurious_interrupt(struct pt_regs *regs)
 
 	irq_enter();
 	exit_idle();
+	trace_arch_irq_vector_entry(SPURIOUS_APIC_VECTOR);
 	/*
 	 * Check if this really is a spurious interrupt and ACK it
 	 * if it is a vectored one.  Just in case...
@@ -1895,6 +1899,7 @@ void smp_spurious_interrupt(struct pt_regs *regs)
 	/* see sw-dev-man vol 3, chapter 7.4.13.5 */
 	pr_info("spurious APIC interrupt on CPU#%d, "
 		"should never happen.\n", smp_processor_id());
+	trace_arch_irq_vector_exit(SPURIOUS_APIC_VECTOR);
 	irq_exit();
 }
 
@@ -1918,6 +1923,7 @@ void smp_error_interrupt(struct pt_regs *regs)
 
 	irq_enter();
 	exit_idle();
+	trace_arch_irq_vector_entry(ERROR_APIC_VECTOR);
 	/* First tickle the hardware, only then report what went on. -- REW */
 	v0 = apic_read(APIC_ESR);
 	apic_write(APIC_ESR, 0);
@@ -1938,6 +1944,7 @@ void smp_error_interrupt(struct pt_regs *regs)
 
 	apic_printk(APIC_DEBUG, KERN_CONT "\n");
 
+	trace_arch_irq_vector_exit(ERROR_APIC_VECTOR);
 	irq_exit();
 }
 
diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c b/arch/x86/kernel/cpu/mcheck/therm_throt.c
index 47a1870..63c2cc8 100644
--- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
+++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
@@ -23,6 +23,7 @@
 #include <linux/init.h>
 #include <linux/smp.h>
 #include <linux/cpu.h>
+#include <trace/events/irq_vectors.h>
 
 #include <asm/processor.h>
 #include <asm/apic.h>
@@ -382,8 +383,10 @@ asmlinkage void smp_thermal_interrupt(struct pt_regs *regs)
 {
 	irq_enter();
 	exit_idle();
+	trace_arch_irq_vector_entry(THERMAL_APIC_VECTOR);
 	inc_irq_stat(irq_thermal_count);
 	smp_thermal_vector();
+	trace_arch_irq_vector_exit(THERMAL_APIC_VECTOR);
 	irq_exit();
 	/* Ack only at the end to avoid potential reentry */
 	ack_APIC_irq();
diff --git a/arch/x86/kernel/cpu/mcheck/threshold.c b/arch/x86/kernel/cpu/mcheck/threshold.c
index aa578ca..de74768 100644
--- a/arch/x86/kernel/cpu/mcheck/threshold.c
+++ b/arch/x86/kernel/cpu/mcheck/threshold.c
@@ -3,6 +3,7 @@
  */
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
+#include <trace/events/irq_vectors.h>
 
 #include <asm/irq_vectors.h>
 #include <asm/apic.h>
@@ -21,8 +22,10 @@ asmlinkage void smp_threshold_interrupt(void)
 {
 	irq_enter();
 	exit_idle();
+	trace_arch_irq_vector_entry(THRESHOLD_APIC_VECTOR);
 	inc_irq_stat(irq_threshold_count);
 	mce_threshold_vector();
+	trace_arch_irq_vector_exit(THRESHOLD_APIC_VECTOR);
 	irq_exit();
 	/* Ack only at the end to avoid potential reentry */
 	ack_APIC_irq();
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 1f5f1d5..f4d7344 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -18,6 +18,9 @@
 #include <asm/mce.h>
 #include <asm/hw_irq.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/irq_vectors.h>
+
 atomic_t irq_err_count;
 
 /* Function pointer for generic interrupt vector handling */
@@ -218,11 +221,13 @@ void smp_x86_platform_ipi(struct pt_regs *regs)
 
 	exit_idle();
 
+	trace_arch_irq_vector_entry(X86_PLATFORM_IPI_VECTOR);
 	inc_irq_stat(x86_platform_ipis);
 
 	if (x86_platform_ipi_callback)
 		x86_platform_ipi_callback();
 
+	trace_arch_irq_vector_exit(X86_PLATFORM_IPI_VECTOR);
 	irq_exit();
 
 	set_irq_regs(old_regs);
diff --git a/arch/x86/kernel/irq_work.c b/arch/x86/kernel/irq_work.c
index ca8f703..2cf7505 100644
--- a/arch/x86/kernel/irq_work.c
+++ b/arch/x86/kernel/irq_work.c
@@ -8,13 +8,16 @@
 #include <linux/irq_work.h>
 #include <linux/hardirq.h>
 #include <asm/apic.h>
+#include <trace/events/irq_vectors.h>
 
 void smp_irq_work_interrupt(struct pt_regs *regs)
 {
 	irq_enter();
 	ack_APIC_irq();
+	trace_irq_work_entry(IRQ_WORK_VECTOR);
 	inc_irq_stat(apic_irq_work_irqs);
 	irq_work_run();
+	trace_irq_work_exit(IRQ_WORK_VECTOR);
 	irq_exit();
 }
 
diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index f84f5c5..cc57aba 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -28,6 +28,7 @@
 #include <asm/mach_traps.h>
 #include <asm/nmi.h>
 #include <asm/x86_init.h>
+#include <trace/events/irq_vectors.h>
 
 struct nmi_desc {
 	spinlock_t lock;
@@ -482,12 +483,14 @@ do_nmi(struct pt_regs *regs, long error_code)
 	nmi_nesting_preprocess(regs);
 
 	nmi_enter();
+	trace_nmi_entry(NMI_VECTOR);
 
 	inc_irq_stat(__nmi_count);
 
 	if (!ignore_nmis)
 		default_do_nmi(regs);
 
+	trace_nmi_exit(NMI_VECTOR);
 	nmi_exit();
 
 	/* On i386, may loop back to preprocess */
diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
index 48d2b7d..5b2d6de 100644
--- a/arch/x86/kernel/smp.c
+++ b/arch/x86/kernel/smp.c
@@ -23,6 +23,7 @@
 #include <linux/interrupt.h>
 #include <linux/cpu.h>
 #include <linux/gfp.h>
+#include <trace/events/irq_vectors.h>
 
 #include <asm/mtrr.h>
 #include <asm/tlbflush.h>
@@ -252,8 +253,10 @@ finish:
 void smp_reschedule_interrupt(struct pt_regs *regs)
 {
 	ack_APIC_irq();
+	trace_reschedule_entry(RESCHEDULE_VECTOR);
 	inc_irq_stat(irq_resched_count);
 	scheduler_ipi();
+	trace_reschedule_exit(RESCHEDULE_VECTOR);
 	/*
 	 * KVM uses this interrupt to force a cpu out of guest mode
 	 */
@@ -263,8 +266,10 @@ void smp_call_function_interrupt(struct pt_regs *regs)
 {
 	ack_APIC_irq();
 	irq_enter();
+	trace_call_function_entry(CALL_FUNCTION_VECTOR);
 	generic_smp_call_function_interrupt();
 	inc_irq_stat(irq_call_count);
+	trace_call_function_exit(CALL_FUNCTION_VECTOR);
 	irq_exit();
 }
 
@@ -272,8 +277,10 @@ void smp_call_function_single_interrupt(struct pt_regs *regs)
 {
 	ack_APIC_irq();
 	irq_enter();
+	trace_call_function_single_entry(CALL_FUNCTION_SINGLE_VECTOR);
 	generic_smp_call_function_single_interrupt();
 	inc_irq_stat(irq_call_count);
+	trace_call_function_single_exit(CALL_FUNCTION_SINGLE_VECTOR);
 	irq_exit();
 }
 
diff --git a/include/trace/events/irq_vectors.h b/include/trace/events/irq_vectors.h
new file mode 100644
index 0000000..fffe0c0
--- /dev/null
+++ b/include/trace/events/irq_vectors.h
@@ -0,0 +1,209 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM irq_vectors
+
+#if !defined(_TRACE_IRQ_VECTORS_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_IRQ_VECTORS_H
+
+#include <linux/tracepoint.h>
+#include <asm/irq.h>
+
+#ifndef irq_vector_name_table
+#define irq_vector_name_table { -1, NULL }
+#endif
+
+
+/*
+ * This class is used by generic ,cross-architecture tracepoints.
+ */
+DECLARE_EVENT_CLASS(irq_vector,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector),
+
+	TP_STRUCT__entry(
+		__field(	int,	vector	)
+	),
+
+	TP_fast_assign(
+		__entry->vector = vector;
+	),
+
+	TP_printk("vector=%d", __entry->vector)
+);
+
+/*
+ * nmi_entry - called before enterring a nmi vector handler
+ */
+DEFINE_EVENT(irq_vector, nmi_entry,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * nmi_exit - called immediately after the interrupt vector
+ * handler returns
+ */
+DEFINE_EVENT(irq_vector, nmi_exit,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * local_timer_entry - called before enterring a local timer interrupt
+ * vector handler
+ */
+DEFINE_EVENT(irq_vector, local_timer_entry,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * local_timer_exit - called immediately after the interrupt vector
+ * handler returns
+ */
+DEFINE_EVENT(irq_vector, local_timer_exit,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * reschedule_entry - called before enterring a reschedule vector handler
+ */
+DEFINE_EVENT(irq_vector, reschedule_entry,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * reschedule_exit - called immediately after the interrupt vector
+ * handler returns
+ */
+DEFINE_EVENT(irq_vector, reschedule_exit,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * call_function_entry - called before enterring a call function
+ *			 vector handler
+ */
+DEFINE_EVENT(irq_vector, call_function_entry,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * call_function_exit - called immediately after the interrupt vector
+ * handler returns
+ */
+DEFINE_EVENT(irq_vector, call_function_exit,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * call_function_single_entry - called before enterring a call function
+ * single vector handler
+ */
+DEFINE_EVENT(irq_vector, call_function_single_entry,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * call_function_single_exit - called immediately after the interrupt vector
+ * handler returns
+ */
+DEFINE_EVENT(irq_vector, call_function_single_exit,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * irq_work_entry - called before enterring an irq work vector handler
+ */
+DEFINE_EVENT(irq_vector, irq_work_entry,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * irq_work_exit - called immediately after the interrupt vector
+ * handler returns
+ */
+DEFINE_EVENT(irq_vector, irq_work_exit,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * This class is used by arch-specific tracepoints.
+ */
+DECLARE_EVENT_CLASS(arch_irq_vector,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector),
+
+	TP_STRUCT__entry(
+		__field(	int,	vector	)
+	),
+
+	TP_fast_assign(
+		__entry->vector = vector;
+	),
+
+	TP_printk("vector=%d name=%s", __entry->vector,
+		__print_symbolic(__entry->vector, irq_vector_name_table))
+);
+
+/*
+ * arch_irq_vector_entry - called before enterring a interrupt vector handler
+ */
+DEFINE_EVENT(arch_irq_vector, arch_irq_vector_entry,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+/*
+ * arch_irq_vector_exit - called immediately after the interrupt vector
+ * handler returns
+ */
+DEFINE_EVENT(arch_irq_vector, arch_irq_vector_exit,
+
+	TP_PROTO(int vector),
+
+	TP_ARGS(vector)
+);
+
+#endif /*  _TRACE_IRQ_VECTORS_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 1.7.1

[Dle-develop] [RFC][PATCH V3]efi_pstore: hold multiple logs to avoid losing critical message

[Seiji A. <sei...@hd...>]

This patchset avoids losing a critical message like panic in NVRAM.

Change v2 -> v3
 - Merged previous 1/3 and 2/3 patches.
 - Dropped previous a 3/3 patch.
 - Remove a kernel parameter ,efi_pstore_log_num.
 - Check if there is enough space to log with QueryVariableInfo().
 - Pass oopscount to arguments of write/erase/read callbacks
   to handle multiple events.

Change v1 -> v2

1/3
  - Freshly created to avoid overwriting entries.

2/3
  - Freshly created to handle multiple logs.
  - Add an additional change passing ctime to arguments of erase_callback.

3/3
  - This is based on previous 2/2 patch
  - Add comments to kernel/printk.h in preparation for future change 
    without considering this patch.
  - Remove infinite loop to avoid potential hang up.
  - Add CFLAGS, -Wswitch-enum and remove default case from switch sentence 
    in preparation for future change without considering this patch.
  - Change a return value to -EEXIST when an erasable entry is not found. 
  - Remove KMSG_DUMP_UNDEF from kmsg_dump_reason because no one uses it.


[Problem]
    Currently, efi_pstore driver simply overwrites existing panic messages in NVRAM.
    So, in the following scenario, we will lose 1st panic messages.

    1. kernel panics.
    2. efi_pstore is kicked and write panic messages to NVRAM.
    3. system reboots.
    4. kernel panics again before a user checks the 1st panic messages in NVRAM.

[Solution]
   To avoid losing a critical message, this patch takes an approach holding multiple
   logs.
   Also, to avoid handling out of space situation, it checks if there are enough spaces
   to write logs with QueryVariableInfo().


[Patch Descriptions]

  This patch makes efi_pstore hold multiple logs.
  Once a critical message is logged, users can see it via /dev/pstore 
  without being influenced by subsequent events.

  Specific changes are as follows.

  - Check if there are enough spaces to write logs with QueryVariableInfo().
  - Remove a logic erasing existing entries from write callback.
  - Add the logic above to erase callback.
  - Pass oopscount to arguments of write/erase/read callbacks to handle multiple events.
  - Pass ctime to arguments of erase_callback to avoid invisible entries via /dev/pstore.

 Signed-off-by: Seiji Aguchi <sei...@hd...>
 Suggested-by: Matthew Garrett <mj...@re...>

---
 drivers/acpi/apei/erst.c   |   16 ++++----
 drivers/firmware/efivars.c |   93 ++++++++++++++++++++++++++++----------------
 fs/pstore/inode.c          |    7 ++-
 fs/pstore/internal.h       |    2 +-
 fs/pstore/platform.c       |   12 +++---
 fs/pstore/ram.c            |   11 ++---
 include/linux/efi.h        |    1 +
 include/linux/pstore.h     |    6 ++-
 8 files changed, 89 insertions(+), 59 deletions(-)

diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c
index e4d9d24..833a9f4 100644
--- a/drivers/acpi/apei/erst.c
+++ b/drivers/acpi/apei/erst.c
@@ -931,14 +931,14 @@ static int erst_check_table(struct acpi_table_erst *erst_tab)
 
 static int erst_open_pstore(struct pstore_info *psi);
 static int erst_close_pstore(struct pstore_info *psi);
-static ssize_t erst_reader(u64 *id, enum pstore_type_id *type,
+static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, u64 *count,
 			   struct timespec *time, char **buf,
 			   struct pstore_info *psi);
 static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason,
-		       u64 *id, unsigned int part,
+		       u64 *id, unsigned int part, u64 count,
 		       size_t size, struct pstore_info *psi);
-static int erst_clearer(enum pstore_type_id type, u64 id,
-			struct pstore_info *psi);
+static int erst_clearer(enum pstore_type_id type, u64 id, u64 count,
+			struct timespec time, struct pstore_info *psi);
 
 static struct pstore_info erst_info = {
 	.owner		= THIS_MODULE,
@@ -987,7 +987,7 @@ static int erst_close_pstore(struct pstore_info *psi)
 	return 0;
 }
 
-static ssize_t erst_reader(u64 *id, enum pstore_type_id *type,
+static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, u64 *count,
 			   struct timespec *time, char **buf,
 			   struct pstore_info *psi)
 {
@@ -1055,7 +1055,7 @@ out:
 }
 
 static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason,
-		       u64 *id, unsigned int part,
+		       u64 *id, unsigned int part, u64 count,
 		       size_t size, struct pstore_info *psi)
 {
 	struct cper_pstore_record *rcd = (struct cper_pstore_record *)
@@ -1101,8 +1101,8 @@ static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason,
 	return ret;
 }
 
-static int erst_clearer(enum pstore_type_id type, u64 id,
-			struct pstore_info *psi)
+static int erst_clearer(enum pstore_type_id type, u64 id, u64 count,
+			struct timespec time, struct pstore_info *psi)
 {
 	return erst_clear(id);
 }
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 47408e8..9966fe3 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -647,7 +647,7 @@ static int efi_pstore_close(struct pstore_info *psi)
 }
 
 static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
-			       struct timespec *timespec,
+			       u64 *count, struct timespec *timespec,
 			       char **buf, struct pstore_info *psi)
 {
 	efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
@@ -655,6 +655,7 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
 	char name[DUMP_NAME_LEN];
 	int i;
 	unsigned int part, size;
+	u64 cnt;
 	unsigned long time;
 
 	while (&efivars->walk_entry->list != &efivars->list) {
@@ -663,8 +664,10 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
 			for (i = 0; i < DUMP_NAME_LEN; i++) {
 				name[i] = efivars->walk_entry->var.VariableName[i];
 			}
-			if (sscanf(name, "dump-type%u-%u-%lu", type, &part, &time) == 3) {
+			if (sscanf(name, "dump-type%u-%u-%llu-%lu",
+			    type, &part, &cnt, &time) == 4) {
 				*id = part;
+				*count = cnt;
 				timespec->tv_sec = time;
 				timespec->tv_nsec = 0;
 				get_var_data_locked(efivars, &efivars->walk_entry->var);
@@ -687,18 +690,62 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
 
 static int efi_pstore_write(enum pstore_type_id type,
 		enum kmsg_dump_reason reason, u64 *id,
-		unsigned int part, size_t size, struct pstore_info *psi)
+		unsigned int part, u64 count, size_t size,
+		struct pstore_info *psi)
 {
 	char name[DUMP_NAME_LEN];
+	efi_char16_t efi_name[DUMP_NAME_LEN];
+	efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
+	struct efivars *efivars = psi->data;
+	int i, ret = 0;
+	u64 storage_space, remaining_space, max_variable_size;
+	efi_status_t status = EFI_NOT_FOUND;
+
+	spin_lock(&efivars->lock);
+
+	status = efivars->ops->query_variable_info(PSTORE_EFI_ATTRIBUTES,
+						   &storage_space,
+						   &remaining_space,
+						   &max_variable_size);
+	if (status || remaining_space < max_variable_size) {
+		spin_unlock(&efivars->lock);
+		*id = part;
+		return -EEXIST;
+	}
+
+	sprintf(name, "dump-type%u-%u-%llu-%lu", type, part, count,
+		get_seconds());
+
+	for (i = 0; i < DUMP_NAME_LEN; i++)
+		efi_name[i] = name[i];
+
+	efivars->ops->set_variable(efi_name, &vendor, PSTORE_EFI_ATTRIBUTES,
+				   size, psi->buf);
+
+	spin_unlock(&efivars->lock);
+
+	if (size)
+		ret = efivar_create_sysfs_entry(efivars,
+					  utf16_strsize(efi_name,
+							DUMP_NAME_LEN * 2),
+					  efi_name, &vendor);
+
+	*id = part;
+	return ret;
+};
+
+static int efi_pstore_erase(enum pstore_type_id type, u64 id, u64 count,
+			    struct timespec time, struct pstore_info *psi)
+{
 	char stub_name[DUMP_NAME_LEN];
 	efi_char16_t efi_name[DUMP_NAME_LEN];
 	efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
 	struct efivars *efivars = psi->data;
 	struct efivar_entry *entry, *found = NULL;
-	int i, ret = 0;
+	int i;
 
-	sprintf(stub_name, "dump-type%u-%u-", type, part);
-	sprintf(name, "%s%lu", stub_name, get_seconds());
+	sprintf(stub_name, "dump-type%u-%llu-%llu-%lu", type, id, count,
+		time.tv_sec);
 
 	spin_lock(&efivars->lock);
 
@@ -717,9 +764,6 @@ static int efi_pstore_write(enum pstore_type_id type,
 		if (utf16_strncmp(entry->var.VariableName, efi_name,
 				  utf16_strlen(efi_name)))
 			continue;
-		/* Needs to be a prefix */
-		if (entry->var.VariableName[utf16_strlen(efi_name)] == 0)
-			continue;
 
 		/* found */
 		found = entry;
@@ -732,32 +776,11 @@ static int efi_pstore_write(enum pstore_type_id type,
 	if (found)
 		list_del(&found->list);
 
-	for (i = 0; i < DUMP_NAME_LEN; i++)
-		efi_name[i] = name[i];
-
-	efivars->ops->set_variable(efi_name, &vendor, PSTORE_EFI_ATTRIBUTES,
-				   size, psi->buf);
-
 	spin_unlock(&efivars->lock);
 
 	if (found)
 		efivar_unregister(found);
 
-	if (size)
-		ret = efivar_create_sysfs_entry(efivars,
-					  utf16_strsize(efi_name,
-							DUMP_NAME_LEN * 2),
-					  efi_name, &vendor);
-
-	*id = part;
-	return ret;
-};
-
-static int efi_pstore_erase(enum pstore_type_id type, u64 id,
-			    struct pstore_info *psi)
-{
-	efi_pstore_write(type, 0, &id, (unsigned int)id, 0, psi);
-
 	return 0;
 }
 #else
@@ -771,7 +794,7 @@ static int efi_pstore_close(struct pstore_info *psi)
 	return 0;
 }
 
-static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
+static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, u64 *count,
 			       struct timespec *timespec,
 			       char **buf, struct pstore_info *psi)
 {
@@ -780,13 +803,14 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
 
 static int efi_pstore_write(enum pstore_type_id type,
 		enum kmsg_dump_reason reason, u64 *id,
-		unsigned int part, size_t size, struct pstore_info *psi)
+		unsigned int part, u64 count, size_t size,
+		struct pstore_info *psi)
 {
 	return 0;
 }
 
-static int efi_pstore_erase(enum pstore_type_id type, u64 id,
-			    struct pstore_info *psi)
+static int efi_pstore_erase(enum pstore_type_id type, u64 id, u64 count,
+			    struct timespec time, struct pstore_info *psi)
 {
 	return 0;
 }
@@ -1226,6 +1250,7 @@ efivars_init(void)
 	ops.get_variable = efi.get_variable;
 	ops.set_variable = efi.set_variable;
 	ops.get_next_variable = efi.get_next_variable;
+	ops.query_variable_info = efi.query_variable_info;
 	error = register_efivars(&__efivars, &ops, efi_kobj);
 	if (error)
 		goto err_put;
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 11a2aa2..ca1b6c9 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -48,6 +48,7 @@ struct pstore_private {
 	struct pstore_info *psi;
 	enum pstore_type_id type;
 	u64	id;
+	u64	count;
 	ssize_t	size;
 	char	data[];
 };
@@ -75,7 +76,8 @@ static int pstore_unlink(struct inode *dir, struct dentry *dentry)
 	struct pstore_private *p = dentry->d_inode->i_private;
 
 	if (p->psi->erase)
-		p->psi->erase(p->type, p->id, p->psi);
+		p->psi->erase(p->type, p->id, p->count,
+			      dentry->d_inode->i_ctime, p->psi);
 
 	return simple_unlink(dir, dentry);
 }
@@ -170,7 +172,7 @@ int pstore_is_mounted(void)
  * Load it up with "size" bytes of data from "buf".
  * Set the mtime & ctime to the date that this record was originally stored.
  */
-int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id,
+int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, u64 count,
 		  char *data, size_t size, struct timespec time,
 		  struct pstore_info *psi)
 {
@@ -206,6 +208,7 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id,
 		goto fail_alloc;
 	private->type = type;
 	private->id = id;
+	private->count = count;
 	private->psi = psi;
 
 	switch (type) {
diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h
index 3bde461..d93e20e 100644
--- a/fs/pstore/internal.h
+++ b/fs/pstore/internal.h
@@ -1,6 +1,6 @@
 extern void	pstore_set_kmsg_bytes(int);
 extern void	pstore_get_records(int);
 extern int	pstore_mkfile(enum pstore_type_id, char *psname, u64 id,
-			      char *data, size_t size,
+			      u64 count, char *data, size_t size,
 			      struct timespec time, struct pstore_info *psi);
 extern int	pstore_is_mounted(void);
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 03ce7a9..371a184 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -66,7 +66,7 @@ void pstore_set_kmsg_bytes(int bytes)
 }
 
 /* Tag each group of saved records with a sequence number */
-static int	oopscount;
+unsigned int	oopscount;
 
 static const char *get_reason_str(enum kmsg_dump_reason reason)
 {
@@ -128,7 +128,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
 			break;
 
 		ret = psinfo->write(PSTORE_TYPE_DMESG, reason, &id, part,
-				    hsize + len, psinfo);
+				    oopscount, hsize + len, psinfo);
 		if (ret == 0 && reason == KMSG_DUMP_OOPS && pstore_is_mounted())
 			pstore_new_entry = 1;
 
@@ -202,7 +202,7 @@ void pstore_get_records(int quiet)
 	struct pstore_info *psi = psinfo;
 	char			*buf = NULL;
 	ssize_t			size;
-	u64			id;
+	u64			id, count;
 	enum pstore_type_id	type;
 	struct timespec		time;
 	int			failed = 0, rc;
@@ -214,9 +214,9 @@ void pstore_get_records(int quiet)
 	if (psi->open && psi->open(psi))
 		goto out;
 
-	while ((size = psi->read(&id, &type, &time, &buf, psi)) > 0) {
-		rc = pstore_mkfile(type, psi->name, id, buf, (size_t)size,
-				  time, psi);
+	while ((size = psi->read(&id, &type, &count, &time, &buf, psi)) > 0) {
+		rc = pstore_mkfile(type, psi->name, id, count, buf,
+				   (size_t)size, time, psi);
 		kfree(buf);
 		buf = NULL;
 		if (rc && (rc != -EEXIST || !quiet))
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 453030f..2bdd4f2 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -86,9 +86,8 @@ static int ramoops_pstore_open(struct pstore_info *psi)
 }
 
 static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
-				   struct timespec *time,
-				   char **buf,
-				   struct pstore_info *psi)
+				   u64 *count, struct timespec *time,
+				   char **buf, struct pstore_info *psi)
 {
 	ssize_t size;
 	struct ramoops_context *cxt = psi->data;
@@ -137,7 +136,7 @@ static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz)
 static int ramoops_pstore_write(enum pstore_type_id type,
 				enum kmsg_dump_reason reason,
 				u64 *id,
-				unsigned int part,
+				unsigned int part, u64 count,
 				size_t size, struct pstore_info *psi)
 {
 	struct ramoops_context *cxt = psi->data;
@@ -177,8 +176,8 @@ static int ramoops_pstore_write(enum pstore_type_id type,
 	return 0;
 }
 
-static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
-				struct pstore_info *psi)
+static int ramoops_pstore_erase(enum pstore_type_id type, u64 id, u64 count,
+				timespec time, struct pstore_info *psi)
 {
 	struct ramoops_context *cxt = psi->data;
 
diff --git a/include/linux/efi.h b/include/linux/efi.h
index ec45ccd..b19bef5 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -635,6 +635,7 @@ struct efivar_operations {
 	efi_get_variable_t *get_variable;
 	efi_get_next_variable_t *get_next_variable;
 	efi_set_variable_t *set_variable;
+	efi_query_variable_info_t *query_variable_info;
 };
 
 struct efivars {
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index e1461e1..9fd1fc8 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -42,12 +42,14 @@ struct pstore_info {
 	int		(*open)(struct pstore_info *psi);
 	int		(*close)(struct pstore_info *psi);
 	ssize_t		(*read)(u64 *id, enum pstore_type_id *type,
-			struct timespec *time, char **buf,
+			u64 *count, struct timespec *time, char **buf,
 			struct pstore_info *psi);
 	int		(*write)(enum pstore_type_id type,
 			enum kmsg_dump_reason reason, u64 *id,
-			unsigned int part, size_t size, struct pstore_info *psi);
+			unsigned int part, u64 count, size_t size,
+			struct pstore_info *psi);
 	int		(*erase)(enum pstore_type_id type, u64 id,
+			u64 count, struct timespec time,
 			struct pstore_info *psi);
 	void		*data;
 };
-- 1.7.1

[Dle-develop] Gracias por tu colaboración en la primera aportación a las ONG's y estas son nuestras propuestas para hoy

[GlobalBono <of...@em...>]

Cabestan. Layout Doble
   Asegúrate de no perderte ninguna oferta, añade of...@gl... a tu lista de contactos.
Si no ves correctamente las imágenes, pulsa [ http://email.globalbono.com/E25072012155322.cfm?WL=719&WS=132430_6215484&WA=253 ] aquí
[ http://email.globalbono.com/Go/index.cfm?WL=564&WS=132430_6215484&WA=253 ]     Tus Ofertas de hoy    
    Síguenos en:    
[ http://email.globalbono.com/Go/index.cfm?WL=50&WS=132430_6215484&WA=253 ] [ http://email.globalbono.com/Go/index.cfm?WL=51&WS=132430_6215484&WA=253 ] 







[ http://email.globalbono.com/Go/index.cfm?WL=704&WS=132430_6215484&WA=253 ] 



[ http://email.globalbono.com/Go/index.cfm?WL=236&WS=132430_6215484&WA=253 ] 

40% Dto. Lote de 24 tuppers 
Lote de 24 tuppers con su propio dispensador giratorio para que los tengas siempre bien ...



29,19¤


[ http://email.globalbono.com/Go/index.cfm?WL=236&WS=132430_6215484&WA=253 ] 

Antes

49,00¤Dto: 40%
Ahorro: 19,81¤











[ http://email.globalbono.com/Go/index.cfm?WL=123&WS=132430_6215484&WA=253 ] 

49% Dto. Decantador de Vino
Decantador de vino instantáneo con el que podrás oxigenar la bebida de Baco sin tener qu...



35,33¤


[ http://email.globalbono.com/Go/index.cfm?WL=123&WS=132430_6215484&WA=253 ] 

Antes

69,95¤Dto: 49%
Ahorro: 34,62¤














[ http://email.globalbono.com/Go/index.cfm?WL=705&WS=132430_6215484&WA=253 ] 

52% Dto. Escapada con Sabor Castellano en Hotel Tres Estrellas
Vente a Burgos y conoce todo lo castellano y burgalés, incluso su insuperable gastronomí...



29,00¤


[ http://email.globalbono.com/Go/index.cfm?WL=705&WS=132430_6215484&WA=253 ] 

Antes

60,00¤Dto: 52%
Ahorro: 31,00¤











[ http://email.globalbono.com/Go/index.cfm?WL=694&WS=132430_6215484&WA=253 ] 

57% Dto. Cultura de Día en Oviedo, Amor para la Noche en Hotel Tres Estrellas 
Acercaros a conocer Oviedo y su ruta de las esculturas y Santa María del Naranco, para d...



26,00¤


[ http://email.globalbono.com/Go/index.cfm?WL=694&WS=132430_6215484&WA=253 ] 

Antes

60,00¤Dto: 57%
Ahorro: 34,00¤














[ http://email.globalbono.com/Go/index.cfm?WL=593&WS=132430_6215484&WA=253 ] 

69% Dto. Pack de Gel para Ventilador
Paquetes congelables para tu ventilador con los que tendrás, siempre que quieras, un ven...



9,26¤


[ http://email.globalbono.com/Go/index.cfm?WL=593&WS=132430_6215484&WA=253 ] 

Antes

30,00¤Dto: 69%
Ahorro: 20,74¤











[ http://email.globalbono.com/Go/index.cfm?WL=634&WS=132430_6215484&WA=253 ] 

45% Dto. Corta Pelo para Perros
No dejes que tu mascota pase calor innecesario y menos ahora en verano. Córtale el pelo ...



21,64¤


[ http://email.globalbono.com/Go/index.cfm?WL=634&WS=132430_6215484&WA=253 ] 

Antes

39,00¤Dto: 45%
Ahorro: 17,36¤













[ http://email.globalbono.com/Go/index.cfm?WL=706&WS=132430_6215484&WA=253 ] 



De conformidad con lo que establece la Ley Orgánica 15/1999 de Protección de Datos de Carácter Personal, le informamos que sus datos personales serán incorporados a un fichero bajo la responsabilidad de GlobalBono, con la finalidad de poder atender los compromisos derivados de la relación que mantenemos con usted. Puede ejercer sus derechos de acceso, cancelación, rectificación y oposición mediante un escrito a la dirección email in...@gl.... Si en el plazo de 30 días no nos comunica lo contrario, entenderemos que los datos no han sido modificados, que se compromete a notificarnos cualquier variación y que tenemos el consentimiento para utilizarlos a fin de poder fidelizar la relación entre las partes.
   ¿Quieres dejar de recibir este email? Haz click  [ http://email.globalbono.com/baja/form_baja_GB.cfm?WL=182&WS=132430_6215484&WA=253 ] aquí.   

[Dle-develop] Entre un Blanqueador Dental, un Termómetro Botellero y una escapada a Oviedo, se encuentran nuestras propuestas de hoy.

[GlobalBono <of...@em...>]

Cabestan. Layout Doble
   Asegúrate de no perderte ninguna oferta, añade of...@gl... a tu lista de contactos.
Si no ves correctamente las imágenes, pulsa [ http://email.globalbono.com/E24072012160706.cfm?WL=701&WS=132430_6215484&WA=241 ] aquí
[ http://email.globalbono.com/Go/index.cfm?WL=564&WS=132430_6215484&WA=241 ]     Tus Ofertas de hoy    
    Síguenos en:    
[ http://email.globalbono.com/Go/index.cfm?WL=50&WS=132430_6215484&WA=241 ] [ http://email.globalbono.com/Go/index.cfm?WL=51&WS=132430_6215484&WA=241 ] 








[ http://email.globalbono.com/Go/index.cfm?WL=91&WS=132430_6215484&WA=241 ] 

47% Dto. Termómetro Para Botellas
Termómetro para botellas, sirve el vino siempre a la temperatura perfecta y recomendada,...



15,51¤


[ http://email.globalbono.com/Go/index.cfm?WL=91&WS=132430_6215484&WA=241 ] 

Antes

29,00¤Dto: 47%
Ahorro: 13,49¤











[ http://email.globalbono.com/Go/index.cfm?WL=92&WS=132430_6215484&WA=241 ] 

47% Dto. Pack de 3 Beauty Bra 
Pack de 3 Beauty Bra edición verano, para que puedas seguir luciendo tu ropa sin que se ...



24,00¤


[ http://email.globalbono.com/Go/index.cfm?WL=92&WS=132430_6215484&WA=241 ] 

Antes

45,00¤Dto: 47%
Ahorro: 21,00¤














[ http://email.globalbono.com/Go/index.cfm?WL=687&WS=132430_6215484&WA=241 ] 

60% Dto. Cuenca en Verano, en Hotel Cuatro Estrellas
Conoce Cuenca, ciudad Patrimonio de la Humanidad, sus casas colgantes y su ciudad encant...



28,00¤


[ http://email.globalbono.com/Go/index.cfm?WL=687&WS=132430_6215484&WA=241 ] 

Antes

70,00¤Dto: 60%
Ahorro: 42,00¤











[ http://email.globalbono.com/Go/index.cfm?WL=694&WS=132430_6215484&WA=241 ] 

57% Dto. Cultura de Día en Oviedo, Amor para la Noche en Hotel Tres Estrellas 
Acercaros a conocer Oviedo y su ruta de las esculturas y Santa María del Naranco, para d...



26,00¤


[ http://email.globalbono.com/Go/index.cfm?WL=694&WS=132430_6215484&WA=241 ] 

Antes

60,00¤Dto: 57%
Ahorro: 34,00¤














[ http://email.globalbono.com/Go/index.cfm?WL=594&WS=132430_6215484&WA=241 ] 

59% Dto. Blanqueador Dental
Elimina las manchas de café y tabaco de tus dientes con éste blanqueador dental. Recuper...



15,92¤


[ http://email.globalbono.com/Go/index.cfm?WL=594&WS=132430_6215484&WA=241 ] 

Antes

39,00¤Dto: 59%
Ahorro: 23,08¤











[ http://email.globalbono.com/Go/index.cfm?WL=686&WS=132430_6215484&WA=241 ] 

38% Dto. Tendedero Eléctrico
Quizá pienses que en verano no te hace falta, porque todo se seca muy rápido con el calo...



48,90¤


[ http://email.globalbono.com/Go/index.cfm?WL=686&WS=132430_6215484&WA=241 ] 

Antes

79,00¤Dto: 38%
Ahorro: 30,10¤














De conformidad con lo que establece la Ley Orgánica 15/1999 de Protección de Datos de Carácter Personal, le informamos que sus datos personales serán incorporados a un fichero bajo la responsabilidad de GlobalBono, con la finalidad de poder atender los compromisos derivados de la relación que mantenemos con usted. Puede ejercer sus derechos de acceso, cancelación, rectificación y oposición mediante un escrito a la dirección email in...@gl.... Si en el plazo de 30 días no nos comunica lo contrario, entenderemos que los datos no han sido modificados, que se compromete a notificarnos cualquier variación y que tenemos el consentimiento para utilizarlos a fin de poder fidelizar la relación entre las partes.
   ¿Quieres dejar de recibir este email? Haz click  [ http://email.globalbono.com/baja/form_baja_GB.cfm?WL=182&WS=132430_6215484&WA=241 ] aquí.   

Re: [Dle-develop] [RFC][PATCH v2 2/3] Hold multiple logs

[Matthew G. <mj...@re...>]

On Tue, Jul 24, 2012 at 09:12:25PM +0000, Luck, Tony wrote:
> > I think we inevitably lose in that scenario. I'd need to verify, but my 
> > recollection is that overwriting existing variables may be equivalent to 
> > a delete/create cycle.
> 
> This would mean that EFI really wants the OS to treat EFI variables as pretty much
> exclusively read-only.  Any activity which periodically updates a variable would
> eventually run into problems in an EFI implementation that loses the old space
> until a reset.

Sure. I'll test with a few implementations and see what I can figure 
out. We may just want to reserve some space for pstore, then have delete 
in pstore simply map to hiding the entries rather than deleting them.

-- 
Matthew Garrett | mj...@sr...

Re: [Dle-develop] [RFC][PATCH v2 2/3] Hold multiple logs

[Luck, T. <ton...@in...>]

> I think we inevitably lose in that scenario. I'd need to verify, but my 
> recollection is that overwriting existing variables may be equivalent to 
> a delete/create cycle.

This would mean that EFI really wants the OS to treat EFI variables as pretty much
exclusively read-only.  Any activity which periodically updates a variable would
eventually run into problems in an EFI implementation that loses the old space
until a reset.

Sad.

-Tony

Re: [Dle-develop] [RFC][PATCH v2 2/3] Hold multiple logs

[Matthew G. <mj...@re...>]

On Tue, Jul 24, 2012 at 08:54:59PM +0000, Luck, Tony wrote:

> Now we panic. Pstore asks EFI "Do you have any space?" EFI replies "No". Pstore can't even overwrite
> one of the old OOPs records ... because it thinks they have all been erased. So we lose the panic
> log.

I think we inevitably lose in that scenario. I'd need to verify, but my 
recollection is that overwriting existing variables may be equivalent to 
a delete/create cycle.

-- 
Matthew Garrett | mj...@sr...

Re: [Dle-develop] [RFC][PATCH v2 2/3] Hold multiple logs

[Luck, T. <ton...@in...>]

> So, we don't need to introduce a overwriting policy.
> I will make a patch using QueryVariableInfo and just writing multiple logs.

I don't think that's what Matthew said.

Here's the bad scenario he envisions:

System is running. It has an OOPs, which gets logged by pstore, and the system carries on running.

The new daemon that we said we needed earlier in this e-mail thread finds the entry in pstore and
copies it to some place in /var/log and removes the pstore entry - causing pstore to ask EFI (or ERST)
backend to erase the record. Firmware does the erase, but doesn't put the space back into the
free pool for use.

Repeat with more OOPs until all the EFI (or ERST) space has been allocated and then lost into
firmware limbo waiting for a reset.

Now we panic. Pstore asks EFI "Do you have any space?" EFI replies "No". Pstore can't even overwrite
one of the old OOPs records ... because it thinks they have all been erased. So we lose the panic
log.

-Tony

Re: [Dle-develop] [RFC][PATCH v2 2/3] Hold multiple logs

[Seiji A. <sei...@hd...>]

> > One thing that's worth noting - UEFI systems will typically only
> > recover deleted space on reset. create->delete->create->delete will
> > reduce available space until the platform is rebooted, at which point
> > the deleted portion will become available again.
> 
> Some ACPI/ERST systems do this too :-(

So, we don't need to introduce a overwriting policy.
I will make a patch using QueryVariableInfo and just writing multiple logs.

Seiji

Re: [Dle-develop] [RFC][PATCH v2 2/3] Hold multiple logs

[Luck, T. <ton...@in...>]

> One thing that's worth noting - UEFI systems will typically only recover 
> deleted space on reset. create->delete->create->delete will reduce 
> available space until the platform is rebooted, at which point the 
> deleted portion will become available again.

Some ACPI/ERST systems do this too :-(

-Tony

Re: [Dle-develop] [RFC][PATCH v2 2/3] Hold multiple logs

[Matthew G. <mj...@re...>]

On Tue, Jul 24, 2012 at 05:52:25PM +0000, Luck, Tony wrote:
> 	if (QueryVariableInfo says enough space)
> 		pstore saves log as new record
> 	else
> 		we consider over-write options to re-use an existing record, or just drop this one

I'd lean towards saying drop, and rely on userspace to do something 
useful. Personal experience is that if two oopses are unrelated then 
there's enough time for userspace to do something and remove the 
existing record, and if they're related it's the first one that tells 
you where the problem actually is.

One thing that's worth noting - UEFI systems will typically only recover 
deleted space on reset. create->delete->create->delete will reduce 
available space until the platform is rebooted, at which point the 
deleted portion will become available again.

-- 
Matthew Garrett | mj...@sr...

Re: [Dle-develop] [RFC][PATCH v2 2/3] Hold multiple logs

[Luck, T. <ton...@in...>]

> I talked with Matthew a bit privately and he suggested to use QueryVariableInfo service which is supported in EFI 2.0 or later.
> If we can use it, we know the remaining NVRAM space before calling SetVariable.

So we can have (pseudo)-code like this:


	if (QueryVariableInfo says enough space)
		pstore saves log as new record
	else
		we consider over-write options to re-use an existing record, or just drop this one

That looks like a good solution - platforms that provide enough non-volatile memory for
multiple records make use of it for better diagnosis. Those that don't have available memory
don't get to test their "what do we do when we run out of space" code.

-Tony

Re: [Dle-develop] [RFC][PATCH v2 2/3] Hold multiple logs

[Seiji A. <sei...@hd...>]

Tony,

I think all guys agree to hold multiple logs.
On the other hand, we have different opinions on overwriting policy.

So, I would like to find a way to fix this issue ,losing critical message, without overwriting policy at first.

I talked with Matthew a bit privately and he suggested to use QueryVariableInfo service which is supported in EFI 2.0 or later.
If we can use it, we know the remaining NVRAM space before calling SetVariable.

This mean that we can avoid the situation which efi_pstore have to handle out of space conditions.
Also, we don't need to introduce a new kernel parameter by just holding multiple logs.

What do you think?

Seiji

> -----Original Message-----
> From: Matthew Garrett [mailto:mj...@re...]
> Sent: Monday, July 23, 2012 10:17 AM
> To: Don Zickus
> Cc: Seiji Aguchi; Luck, Tony; lin...@vg...; lin...@vg...; mi...@go...; dle-
> de...@li...; Satoru Moriya
> Subject: Re: [RFC][PATCH v2 2/3] Hold multiple logs
> 
> On Thu, Jul 19, 2012 at 11:03:28PM -0400, Don Zickus wrote:
> 
> > What is the harm of not using this and just letting the number be
> > infinite (or until EFI runs out of space)?  Is it a big deal if extra
> > failures are logged?
> 
> Running out of space in EFI isn't a well-tested scenario, and I wouldn't expect all firmware to handle it gracefully. This is made worse by
> EFI 1 not providing any information about available storage. I'd be fine with changing the default number of entries on systems where
> we can obtain the appropriate information to make that decision, but otherwise I think it should be limited to 1.
> 
> --
> Matthew Garrett | mj...@sr...

Re: [Dle-develop] [PATCH]efi: initialize efi.runtime_version to make query_variable_info/update_capsule workable

[Matthew G. <mj...@sr...>]

On Tue, Jul 24, 2012 at 01:27:23PM +0000, Seiji Aguchi wrote:
> A value of efi.runtime_version is checked before calling 
> update_capsule()/query_variable_info() as follows.
> But it isn't initialized anywhere.
> 
> <snip>
> static efi_status_t virt_efi_query_variable_info(u32 attr,
>                                                  u64 *storage_space,
>                                                  u64 *remaining_space,
>                                                  u64 *max_variable_size)
> {
>         if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
>                 return EFI_UNSUPPORTED;
> <snip>
> 
> This patch initializes a value of efi.runtime_version at boot time.
> 
>  Signed-off-by: Seiji Aguchi <sei...@hd...>

I could have sworn that something equivalent had been posted before, 
but:

Acked-by: Matthew Garrett <mj...@re...>

-- 
Matthew Garrett | mj...@sr...

[Dle-develop] [PATCH]efi: initialize efi.runtime_version to make query_variable_info/update_capsule workable

[Seiji A. <sei...@hd...>]

A value of efi.runtime_version is checked before calling 
update_capsule()/query_variable_info() as follows.
But it isn't initialized anywhere.

<snip>
static efi_status_t virt_efi_query_variable_info(u32 attr,
                                                 u64 *storage_space,
                                                 u64 *remaining_space,
                                                 u64 *max_variable_size)
{
        if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
                return EFI_UNSUPPORTED;
<snip>

This patch initializes a value of efi.runtime_version at boot time.

 Signed-off-by: Seiji Aguchi <sei...@hd...>

---
 arch/x86/platform/efi/efi.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 92660ed..f55a4ce 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -890,6 +890,7 @@ void __init efi_enter_virtual_mode(void)
 	 *
 	 * Call EFI services through wrapper functions.
 	 */
+	efi.runtime_version = efi_systab.fw_revision;
 	efi.get_time = virt_efi_get_time;
 	efi.set_time = virt_efi_set_time;
 	efi.get_wakeup_time = virt_efi_get_wakeup_time;
-- 1.7.1

[Dle-develop] Termómetro Digital por 5,87¤ / Silla Plegable de bolsillo por sólo 9,31¤ / Conoce Granada y tómate un as tapas por 24¤

[GlobalBono <of...@em...>]

Cabestan. Layout Doble
   Asegúrate de no perderte ninguna oferta, añade of...@gl... a tu lista de contactos.
Si no ves correctamente las imágenes, pulsa [ http://email.globalbono.com/E23072012172503.cfm?WL=683&WS=132430_6215484&WA=235 ] aquí
[ http://email.globalbono.com/Go/index.cfm?WL=564&WS=132430_6215484&WA=235 ]     Tus Ofertas de hoy    
    Síguenos en:    
[ http://email.globalbono.com/Go/index.cfm?WL=50&WS=132430_6215484&WA=235 ] [ http://email.globalbono.com/Go/index.cfm?WL=51&WS=132430_6215484&WA=235 ] 








[ http://email.globalbono.com/Go/index.cfm?WL=190&WS=132430_6215484&WA=235 ] 

57% Dto. Rodillo Anti Estrías
Rodillo con mini agujas para que aumentes la circulación en tu piel antes de aplicar cua...



29,90¤


[ http://email.globalbono.com/Go/index.cfm?WL=190&WS=132430_6215484&WA=235 ] 

Antes

70,00¤Dto: 57%
Ahorro: 40,10¤











[ http://email.globalbono.com/Go/index.cfm?WL=122&WS=132430_6215484&WA=235 ] 

60% Dto. Gafas Relax
Gafas Relax para que puedas dar descanso a tus ojos mientras escuchas tu música favorita...



29,67¤


[ http://email.globalbono.com/Go/index.cfm?WL=122&WS=132430_6215484&WA=235 ] 

Antes

75,00¤Dto: 60%
Ahorro: 45,33¤














[ http://email.globalbono.com/Go/index.cfm?WL=636&WS=132430_6215484&WA=235 ] 

41% Dto. Escapada y Noche de Cine, en el Hotel Sercotel Ciutat de Montcada, 4*
Regálate una escapada de cine en Montcada i Reixac, a las afueras de Barcelona. La entra...



41,00¤


[ http://email.globalbono.com/Go/index.cfm?WL=636&WS=132430_6215484&WA=235 ] 

Antes

70,00¤Dto: 41%
Ahorro: 29,00¤











[ http://email.globalbono.com/Go/index.cfm?WL=617&WS=132430_6215484&WA=235 ] 

52% Dto. Escapada de Tapas en Granada, en Hotel Tres Estrellas
Vente a Granada, disfruta de la belleza de esta ciudad, de tres tapas típicas, y pasa la...



24,00¤


[ http://email.globalbono.com/Go/index.cfm?WL=617&WS=132430_6215484&WA=235 ] 

Antes

50,00¤Dto: 52%
Ahorro: 26,00¤














[ http://email.globalbono.com/Go/index.cfm?WL=674&WS=132430_6215484&WA=235 ] 

63% Dto. Silla Super Plegable 
Llévate donde quieras esta silla más que plegable. Aguanta hasta 100 kilos de peso por s...



9,31¤


[ http://email.globalbono.com/Go/index.cfm?WL=674&WS=132430_6215484&WA=235 ] 

Antes

25,00¤Dto: 63%
Ahorro: 15,69¤











[ http://email.globalbono.com/Go/index.cfm?WL=670&WS=132430_6215484&WA=235 ] 

69% Dto. Termómetro Digital Infantil
Ten siempre clara la temperatura de tus hijos con éste termómetro digital. Su pantalla L...



5,87¤


[ http://email.globalbono.com/Go/index.cfm?WL=670&WS=132430_6215484&WA=235 ] 

Antes

19,00¤Dto: 69%
Ahorro: 13,13¤














De conformidad con lo que establece la Ley Orgánica 15/1999 de Protección de Datos de Carácter Personal, le informamos que sus datos personales serán incorporados a un fichero bajo la responsabilidad de GlobalBono, con la finalidad de poder atender los compromisos derivados de la relación que mantenemos con usted. Puede ejercer sus derechos de acceso, cancelación, rectificación y oposición mediante un escrito a la dirección email in...@gl.... Si en el plazo de 30 días no nos comunica lo contrario, entenderemos que los datos no han sido modificados, que se compromete a notificarnos cualquier variación y que tenemos el consentimiento para utilizarlos a fin de poder fidelizar la relación entre las partes.
   ¿Quieres dejar de recibir este email? Haz click  [ http://email.globalbono.com/baja/form_baja_GB.cfm?WL=182&WS=132430_6215484&WA=235 ] aquí.   

[Dle-develop] Las propuestas de hoy son: Organizador de bolsos por solo 10,31¤ / Ventilador Solac por 34,10¤ / Escapada y noche de cine en Montcada por 41¤ / Lote de 24 Tupers por 29,19¤

[GlobalBono <of...@em...>]

Cabestan. Layout Simple
   Asegúrate de no perderte ninguna oferta, añade of...@gl... a tu lista de contactos.
Si no ves correctamente las imágenes, pulsa [ http://email.globalbono.com/E21072012100453.cfm?WL=667&WS=132430_6215484&WA=226 ] aquí
[ http://email.globalbono.com/Go/index.cfm?WL=564&WS=132430_6215484&WA=226 ]     Tus Ofertas de hoy    
    Síguenos en:    
[ http://email.globalbono.com/Go/index.cfm?WL=50&WS=132430_6215484&WA=226 ] [ http://email.globalbono.com/Go/index.cfm?WL=51&WS=132430_6215484&WA=226 ] 








[ http://email.globalbono.com/Go/index.cfm?WL=236&WS=132430_6215484&WA=226 ] 40% Dto. Lote de 24 tuppers 
Lote de 24 tuppers con su propio dispensador giratorio para que los tengas siempre bien ordenados por t...

29,19¤


[ http://email.globalbono.com/Go/index.cfm?WL=236&WS=132430_6215484&WA=226 ] 


Antes

49,00¤Dto: 40%
Ahorro: 19,81¤
















[ http://email.globalbono.com/Go/index.cfm?WL=123&WS=132430_6215484&WA=226 ] 49% Dto. Decantador de Vino
Decantador de vino instantáneo con el que podrás oxigenar la bebida de Baco sin tener que esperar largo...

35,33¤


[ http://email.globalbono.com/Go/index.cfm?WL=123&WS=132430_6215484&WA=226 ] 


Antes

69,95¤Dto: 49%
Ahorro: 34,62¤
















[ http://email.globalbono.com/Go/index.cfm?WL=617&WS=132430_6215484&WA=226 ] 52% Dto. Escapada de Tapas en Granada, en Hotel Tres Estrellas
Vente a Granada, disfruta de la belleza de esta ciudad, de tres tapas típicas, y pasa la noche en el Ho...

24,00¤


[ http://email.globalbono.com/Go/index.cfm?WL=617&WS=132430_6215484&WA=226 ] 


Antes

50,00¤Dto: 52%
Ahorro: 26,00¤
















[ http://email.globalbono.com/Go/index.cfm?WL=636&WS=132430_6215484&WA=226 ] 41% Dto. Escapada y Noche de Cine, en el Hotel Sercotel Ciutat de Montcada, 4*
Regálate una escapada de cine en Montcada i Reixac, a las afueras de Barcelona. La entrada, las palomit...

41,00¤


[ http://email.globalbono.com/Go/index.cfm?WL=636&WS=132430_6215484&WA=226 ] 


Antes

70,00¤Dto: 41%
Ahorro: 29,00¤
















[ http://email.globalbono.com/Go/index.cfm?WL=656&WS=132430_6215484&WA=226 ] 42% Dto. Ventilador Solac
No dejes de refrescarte porque no tengas un enchufe cerca, éste ventilador con batería, y 30 cm. de diá...

34,10¤


[ http://email.globalbono.com/Go/index.cfm?WL=656&WS=132430_6215484&WA=226 ] 


Antes

59,00¤Dto: 42%
Ahorro: 24,90¤
















[ http://email.globalbono.com/Go/index.cfm?WL=592&WS=132430_6215484&WA=226 ] 64% Dto. Organizador de Bolsos
Ten siempre claro dónde has dejado las llaves, el móvil, el monedero, el mp3, y... todo, gracias a éste...

10,31¤


[ http://email.globalbono.com/Go/index.cfm?WL=592&WS=132430_6215484&WA=226 ] 


Antes

29,00¤Dto: 64%
Ahorro: 18,69¤
















De conformidad con lo que establece la Ley Orgánica 15/1999 de Protección de Datos de Carácter Personal, le informamos que sus datos personales serán incorporados a un fichero bajo la responsabilidad de GlobalBono, con la finalidad de poder atender los compromisos derivados de la relación que mantenemos con usted. Puede ejercer sus derechos de acceso, cancelación, rectificación y oposición mediante un escrito a la dirección email in...@gl.... Si en el plazo de 30 días no nos comunica lo contrario, entenderemos que los datos no han sido modificados, que se compromete a notificarnos cualquier variación y que tenemos el consentimiento para utilizarlos a fin de poder fidelizar la relación entre las partes.
   ¿Quieres dejar de recibir este email? Haz click  [ http://email.globalbono.com/baja/form_baja_GB.cfm?WL=182&WS=132430_6215484&WA=226 ] aquí.     

Re: [Dle-develop] [RFC][PATCH v2 2/3] Hold multiple logs

[Matthew G. <mj...@re...>]

On Thu, Jul 19, 2012 at 11:03:28PM -0400, Don Zickus wrote:

> What is the harm of not using this and just letting the number be infinite
> (or until EFI runs out of space)?  Is it a big deal if extra failures are
> logged?

Running out of space in EFI isn't a well-tested scenario, and I wouldn't 
expect all firmware to handle it gracefully. This is made worse by EFI 1 
not providing any information about available storage. I'd be fine with 
changing the default number of entries on systems where we can obtain 
the appropriate information to make that decision, but otherwise I think 
it should be limited to 1.

-- 
Matthew Garrett | mj...@sr...

Re: [Dle-develop] [RFC][PATCH v2 0/3] efi_pstore: avoid losing critical message

[Matthew G. <mj...@re...>]

On Thu, Jul 19, 2012 at 09:11:44PM +0000, Seiji Aguchi wrote:

> [Patch Descriptions]
>  Patch 1/3 and 2/3 take fist approach, _not_ overwriting entries.
> 
>  Patch 3/3 takes second approach, adding some logic overwriting entries.
> 
>  1/3: Avoid overwriting existing entry
>   This patch just avoid overwriting entries to save the 1st critical message 
>   without being influenced by subsequent events.

I think there's a good argument in favour of this approach. Userspace 
crash report tools should be taught to copy the crash data and then 
remove the existing entries. It avoids any of the complexity associated 
with other approaches and ensures that behaviour is always consistent.

-- 
Matthew Garrett | mj...@sr...