Embedded Linux development
 help / color / mirror / Atom feed
* Re: [Proposal] [PATCH] generic clock framework
From: Francesco VIRLINZI @ 2009-11-11  7:54 UTC (permalink / raw)
  To: Tim Bird; +Cc: Linux-sh, linux-embedded, linux-arm-kernel
In-Reply-To: <4AF9AC3B.2070004@am.sony.com>

Hi Tim
Thanks for your feedback.
ARM mail list was already involved.

Regards
  Francesco
On 11/10/2009 07:08 PM, Tim Bird wrote:
> Francesco VIRLINZI wrote:
>    
>> Hi all
>>
>> I'm Francesco and I work in STMicroelectronics
>>
>> In the last ELC-E_2009 I spoke on a generic clock framework I'm working on
>>   (see
>> http://tree.celinuxforum.org/CelfPubWiki/ELCEurope2009Presentations?action=AttachFile&do=view&target=ELC_E_2009_Generic_Clock_Framework.pdf).
>>
>>
>> I wrote the gcf to manage both clocks the platform_devices during a
>> clock operation.
>>      
> This looks good to me, in principle, but I'm not a clock or PM
> expert.  I would recommend sending this to the linux-kernel and
> linux-pm lists as well.  I think you'll get a wider audience for
> feedback.
>   -- Tim
>
> =============================
> Tim Bird
> Architecture Group Chair, CE Linux Forum
> Senior Staff Engineer, Sony Corporation of America
> =============================
>
>
>    

^ permalink raw reply

* [PATCH, RFC] panic-note: Annotation from user space for panics
From: David VomLehn @ 2009-11-12  2:13 UTC (permalink / raw)
  To: linux-embedded; +Cc: akpm, dwm2, linux-kernel, mpm, paul.gortmaker

Allows annotation of panics to include platform information. It's no big
deal to collect information, but way helpful when you are collecting
failure reports from a eventual base of millions of systems deployed in
other people's homes.

One of the biggest reasons this is an RFC is that I'm uncomfortable with
putting the pseudo-file that holds the annotation information in /proc.
Different layers of the software stack may drop dynamic information, such
as DHCP-supplied IP addresses, in here as they come up. This means it's
necessary to be able to append to the end of the annotation, so this looks
much more like a real file than a sysctl file.  It also has multiple lines,
which doesn't look a sysctl file. Annotation can be viewed as a debug thing,
so maybe it belongs in debugfs, but people seem to be doing somewhat different
things with that filesystem.

So, suggestions on this issue, and any others are most welcome. If there a
better way to do this, I'll be happy to use it.

Signed-off-by: David VomLehn <dvomlehn@cisco.com>
---
 fs/proc/Makefile       |    1 +
 fs/proc/panic-note.c   |  293 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/kernel.h |    7 +
 kernel/panic.c         |    1 +
 lib/Kconfig.debug      |    8 ++
 5 files changed, 310 insertions(+), 0 deletions(-)

diff --git a/fs/proc/Makefile b/fs/proc/Makefile
index 11a7b5c..486d273 100644
--- a/fs/proc/Makefile
+++ b/fs/proc/Makefile
@@ -26,3 +26,4 @@ proc-$(CONFIG_PROC_VMCORE)	+= vmcore.o
 proc-$(CONFIG_PROC_DEVICETREE)	+= proc_devtree.o
 proc-$(CONFIG_PRINTK)	+= kmsg.o
 proc-$(CONFIG_PROC_PAGE_MONITOR)	+= page.o
+proc-$(CONFIG_PANIC_NOTE)	+= panic-note.o
diff --git a/fs/proc/panic-note.c b/fs/proc/panic-note.c
new file mode 100644
index 0000000..449c5ef
--- /dev/null
+++ b/fs/proc/panic-note.c
@@ -0,0 +1,293 @@
+/*
+ *				panic-note.c
+ *
+ * Allow a blob to be registered with the kernel that will be printed if
+ * the kernel panics.
+ *
+ * Copyright (C) 2009  Cisco Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+/* Open issues:
+ * Where should the panic_note file be created? It's almost like a sysctl,
+ * but doesn't follow the same rules. When you write to a sysctl file, the
+ * previous data is replaced. When you write to the panic_note file, you
+ * can append to the end of the existing data.
+ */
+
+#include <linux/semaphore.h>
+#include <linux/fs.h>
+#include <linux/proc_fs.h>
+#include <linux/module.h>
+#include <linux/uaccess.h>
+
+/* Maximum size, in bytes, allowed for the blob. Having this limit prevents
+ * an inadvertant denial of service attack that might happen if someone with
+ * root privileges was automatically generating this note and the generator
+ * had an infinite loop. Perhaps this is more of a a denial of service
+ * suicide. */
+#define PANIC_NOTE_SIZE		(PAGE_SIZE * 4)
+
+/*
+ * struct panic_note_data - Information about the panic note
+ * @n:		Number of bytes in the note
+ * @p:		Pointer to the data in the note
+ * @sem:	Semaphore controlling access to data in the note
+ */
+struct panic_note_state {
+	size_t			n;
+	void			*p;
+	struct rw_semaphore	sem;
+};
+
+static struct panic_note_state panic_note_state = {
+	0, NULL, __RWSEM_INITIALIZER(panic_note_state.sem)
+};
+static const struct file_operations panic_note_fops;
+static struct inode_operations panic_note_iops;
+static struct proc_dir_entry *panic_note_entry;
+
+/*
+ * panic_note_print - display the panic note
+ * @priority:	Printk priority to use, e.g. KERN_EMERG
+ */
+void panic_note_print()
+{
+	int i;
+	int linelen;
+
+	/* We skip the semaphore stuff because we're in a panic situation and
+	 * the scheduler isn't available in case the semaphore is already owned
+	 * by someone else */
+	for (i = 0; i < panic_note_state.n; i += linelen) {
+		const char *p;
+		int remaining;
+		const char *nl;
+
+		p = panic_note_state.p + i;
+		remaining = panic_note_state.n - i;
+
+		nl = memchr(p, '\n', remaining);
+
+		if (nl == NULL) {
+			linelen = remaining;
+			pr_emerg("%.*s\n", linelen, p);
+		} else {
+			linelen = nl - p + 1;
+			pr_emerg("%.*s", linelen, p);
+		}
+	}
+}
+
+/*
+ * read_write_size - calculate the limited copy_to_user/copy_from_user count
+ * @nbytes:	The number of bytes requested
+ * @pos:	Offset, in bytes, into the file
+ * @size:	Maximum I/O offset, in bytes. For a read, this is the actual
+ *		number of bytes in the file, since you can't read past
+ *		the end. Writes can be done after the number of bytes in the
+ *		file, so this is the maximum possible file size, minus one.
+ *
+ * Returns the number of bytes to copy.
+ */
+static ssize_t read_write_size(size_t nbytes, loff_t pos, size_t size)
+{
+	ssize_t retval;
+
+	if (pos >= size)
+		retval = 0;
+
+	else {
+		retval = size - pos;
+		if (retval > nbytes)
+			retval = nbytes;
+	}
+
+	return retval;
+}
+
+/*
+ * panic_note_read - return data from the panic note
+ * @filp:	Pointer to information on the file
+ * @buf:	Pointer, in user space, to the buffer in which to return the
+ * 		data
+ * @nbytes:	Number of bytes requested
+ * @ppos:	Pointer to file position
+ *
+ * Returns the number of bytes actually transferred, or a negative errno
+ * value if none could be transferred.
+ */
+static ssize_t panic_note_read(struct file *filp, char __user *buf,
+	size_t nbytes, loff_t *ppos)
+{
+	ssize_t retval;
+	ssize_t result;
+
+	down_read(&panic_note_state.sem);
+	panic_note_entry->size = panic_note_state.n;
+	retval = read_write_size(nbytes, *ppos, panic_note_state.n);
+
+	if (retval > 0) {
+		result = copy_to_user(buf, panic_note_state.p + *ppos, retval);
+
+		if (result != 0)
+			retval = -EFAULT;
+		else
+			*ppos += retval;
+	}
+	up_read(&panic_note_state.sem);
+
+	return retval;
+}
+
+/*
+ * panic_note_write - store data in the panic note
+ * @filp:	Pointer to information on the file
+ * @buf:	Pointer, in user space, to the buffer from which to retrieve the
+ * 		data
+ * @nbytes:	Number of bytes requested
+ * @ppos:	Pointer to file position
+ *
+ * Returns the number of bytes actually transferred, or a negative errno
+ * value if none could be transferred.
+ */
+static ssize_t panic_note_write(struct file *filp, const char __user *buf,
+	size_t nbytes, loff_t *ppos)
+{
+	ssize_t retval;
+	ssize_t result;
+	loff_t pos;
+
+	down_write(&panic_note_state.sem);
+
+	/* If the O_APPEND flag is set, ignore the current position and
+	 * add to the end. */
+	pos = ((filp->f_flags & O_APPEND) == 0) ? *ppos : panic_note_state.n;
+
+	retval = read_write_size(nbytes, pos, PANIC_NOTE_SIZE);
+
+	if (retval == 0)
+		retval = -ENOSPC;
+	else {
+		/* If we have a hole, fill it with zeros */
+		if (pos > panic_note_state.n)
+			memset(panic_note_state.p + panic_note_state.n,
+				0, pos - panic_note_state.n);
+
+		/* Fetch what was written from user space */
+		result = copy_from_user(panic_note_state.p + pos, buf,
+			retval);
+
+		if (result != 0)
+			retval = -EFAULT;
+		else {
+
+			/* If we now have more bytes than we did, grow the
+			 * size */
+			if (pos + retval > panic_note_state.n) {
+				struct inode *inode;
+				inode = filp->f_path.dentry->d_inode;
+				panic_note_state.n = pos + retval;
+				panic_note_entry->size = panic_note_state.n;
+			}
+
+			*ppos = pos + retval;
+		}
+	}
+	up_write(&panic_note_state.sem);
+
+	return retval;
+}
+
+static int panic_note_open(struct inode *inode, struct file *filp)
+{
+	filp->f_op = &panic_note_fops;
+	inode->i_op = &panic_note_iops;
+	panic_note_entry->size = panic_note_state.n;
+
+	return 0;
+}
+
+static const struct file_operations panic_note_fops = {
+	.owner = THIS_MODULE,
+	.open = panic_note_open,
+	.read = panic_note_read,
+	.write = panic_note_write,
+};
+
+static void panic_note_truncate(struct inode *inode)
+{
+	down_write(&panic_note_state.sem);
+	panic_note_state.n = 0;
+	panic_note_entry->size = panic_note_state.n;
+	up_write(&panic_note_state.sem);
+}
+
+static struct inode_operations panic_note_iops = {
+	.truncate = panic_note_truncate,
+};
+
+static int __init panic_note_init(void)
+{
+	int retval;
+
+	/* This can allocate kernel memory, so we let only the root use
+	 * it. */
+	panic_note_entry = create_proc_entry("panic_note", 0600, NULL);
+
+	if (panic_note_entry == NULL) {
+		retval = -ENOMEM;
+		goto error_exit;
+	}
+
+	/* Set up the basic proc file fields */
+	panic_note_entry->proc_fops = &panic_note_fops;
+	panic_note_entry->proc_iops = &panic_note_iops;
+
+	/* Allocate a buffer. Doing so now avoids the possibility that
+	 * we won't be able to get when when the kernel runs out of
+	 * memory. */
+	panic_note_state.p = kmalloc(PANIC_NOTE_SIZE, GFP_KERNEL);
+
+	if (panic_note_state.p == NULL) {
+		retval = -ENOMEM;
+		goto kmalloc_buf_error;
+	}
+
+	return 0;
+
+kmalloc_buf_error:
+	kfree(panic_note_state.p);
+	panic_note_state.p = NULL;
+
+	remove_proc_entry("panic_note", NULL);
+
+error_exit:
+	return retval;
+}
+
+static int __exit panic_note_cleanup(void)
+{
+	if (panic_note_state.p != NULL)
+		kfree(panic_note_state.p);
+
+	remove_proc_entry("panic_note", NULL);
+
+	return 0;
+}
+
+late_initcall(panic_note_init);
+late_initcall(panic_note_cleanup);
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index f4e3184..86ca4d7 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -312,6 +312,13 @@ extern void add_taint(unsigned flag);
 extern int test_taint(unsigned flag);
 extern unsigned long get_taint(void);
 extern int root_mountflags;
+#ifdef CONFIG_PANIC_NOTE
+extern void panic_note_print(void);
+#else
+static inline void panic_note_print(void)
+{
+}
+#endif
 
 /* Values used for system_state */
 extern enum system_states {
diff --git a/kernel/panic.c b/kernel/panic.c
index 96b45d0..513deae 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -70,6 +70,7 @@ NORET_TYPE void panic(const char * fmt, ...)
 	vsnprintf(buf, sizeof(buf), fmt, args);
 	va_end(args);
 	printk(KERN_EMERG "Kernel panic - not syncing: %s\n",buf);
+	panic_note_print();
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 	dump_stack();
 #endif
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 30df586..bade7a1 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1045,6 +1045,14 @@ config DMA_API_DEBUG
 	  This option causes a performance degredation.  Use only if you want
 	  to debug device drivers. If unsure, say N.
 
+config PANIC_NOTE
+	bool "Create file for user space data to be reported at panic time"
+	default n
+	help
+	  This creates a pseudo-file, named /proc/panic_note, into which
+	  user space data can be written. If a panic occurs, the contents
+	  of the file will be included in the failure report.
+
 source "samples/Kconfig"
 
 source "lib/Kconfig.kgdb"

^ permalink raw reply related

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Marco Stornelli @ 2009-11-12 18:00 UTC (permalink / raw)
  To: David VomLehn
  Cc: linux-embedded, akpm, dwm2, linux-kernel, mpm, paul.gortmaker
In-Reply-To: <20091112021322.GA6166@dvomlehn-lnx2.corp.sa.net>

Sincerely, I don't understand why we should involve the kernel to gather
this kind of information when we can use other (user-space) tools, only
to have "all" in a single report maybe? I think it's a bit weak reason
to include this additional behavior in the kernel.

David VomLehn ha scritto:
> Allows annotation of panics to include platform information. It's no big
> deal to collect information, but way helpful when you are collecting
> failure reports from a eventual base of millions of systems deployed in
> other people's homes.
> 

Marco

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Matt Mackall @ 2009-11-12 18:06 UTC (permalink / raw)
  To: David VomLehn; +Cc: linux-embedded, akpm, dwm2, linux-kernel, paul.gortmaker
In-Reply-To: <20091112021322.GA6166@dvomlehn-lnx2.corp.sa.net>

On Wed, 2009-11-11 at 21:13 -0500, David VomLehn wrote:
> Allows annotation of panics to include platform information. It's no big
> deal to collect information, but way helpful when you are collecting
> failure reports from a eventual base of millions of systems deployed in
> other people's homes.

I'd like to hear a bit more use case motivation on this feature. Also,
why do you want more than a page?

-- 
http://selenic.com : development and support for Mercurial and Linux


^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Paul Gortmaker @ 2009-11-12 19:50 UTC (permalink / raw)
  To: David VomLehn; +Cc: linux-embedded, akpm, dwm2, linux-kernel, mpm
In-Reply-To: <20091112021322.GA6166@dvomlehn-lnx2.corp.sa.net>

David VomLehn wrote:
> Allows annotation of panics to include platform information. It's no big
> deal to collect information, but way helpful when you are collecting
> failure reports from a eventual base of millions of systems deployed in
> other people's homes.
> 
> One of the biggest reasons this is an RFC is that I'm uncomfortable with
> putting the pseudo-file that holds the annotation information in /proc.
> Different layers of the software stack may drop dynamic information, such
> as DHCP-supplied IP addresses, in here as they come up. This means it's
> necessary to be able to append to the end of the annotation, so this looks
> much more like a real file than a sysctl file.  It also has multiple lines,
> which doesn't look a sysctl file. Annotation can be viewed as a debug thing,
> so maybe it belongs in debugfs, but people seem to be doing somewhat different
> things with that filesystem.
> 
> So, suggestions on this issue, and any others are most welcome. If there a
> better way to do this, I'll be happy to use it.
> 
> Signed-off-by: David VomLehn <dvomlehn@cisco.com>
> ---

> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -70,6 +70,7 @@ NORET_TYPE void panic(const char * fmt, ...)
>  	vsnprintf(buf, sizeof(buf), fmt, args);
>  	va_end(args);
>  	printk(KERN_EMERG "Kernel panic - not syncing: %s\n",buf);
> +	panic_note_print();
>  #ifdef CONFIG_DEBUG_BUGVERBOSE
>  	dump_stack();
>  #endif


Why hook into panic() directly like this, vs. using the panic
notifier list? If you use that, and then put the data handling
magic that you need into your own kernel module that knows how
to interface with the reporting apps that you have, you can
do the whole thing without having to alter existing code, I think.

Paul.

> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 30df586..bade7a1 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1045,6 +1045,14 @@ config DMA_API_DEBUG
>  	  This option causes a performance degredation.  Use only if you want
>  	  to debug device drivers. If unsure, say N.
>  
> +config PANIC_NOTE
> +	bool "Create file for user space data to be reported at panic time"
> +	default n
> +	help
> +	  This creates a pseudo-file, named /proc/panic_note, into which
> +	  user space data can be written. If a panic occurs, the contents
> +	  of the file will be included in the failure report.
> +
>  source "samples/Kconfig"
>  
>  source "lib/Kconfig.kgdb"

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: David VomLehn @ 2009-11-12 21:56 UTC (permalink / raw)
  To: Marco Stornelli
  Cc: linux-embedded, akpm, dwm2, linux-kernel, mpm, paul.gortmaker
In-Reply-To: <4AFC4D31.2000101@gmail.com>

(Switched from top posting)

On Thu, Nov 12, 2009 at 01:00:17PM -0500, Marco Stornelli wrote:
> David VomLehn ha scritto:
> > Allows annotation of panics to include platform information. It's no big
> > deal to collect information, but way helpful when you are collecting
> > failure reports from a eventual base of millions of systems deployed in
> > other people's homes.
> > 
> Sincerely, I don't understand why we should involve the kernel to gather
> this kind of information when we can use other (user-space) tools, only
> to have "all" in a single report maybe? I think it's a bit weak reason
> to include this additional behavior in the kernel.

Good question. Some more detail on our application might help. In some
situations, we may have no disk and only enough flash for the bootloader.
The kernel is downloaded over the network. When we get to user space, we
initialize a number of things dynamically. For example, we dynamically
compute some MAC address, and most of the IP addresses are obtained with
DHCP. This are very useful to have for panic analysis.

Since there is neither flash nor disk, user space has no place to store
this information, should the kernel panic. When we come back up, we will get
different MAC and IP addresses. Storing them in memory is our only hope.

Fortunately, there is a section of RAM that the bootloader promises not
to overwrite. On a panic, we capture the messages written on the console
and store them in the protected area. If the information from the
/proc file is written as part of the panic, we will capture it, too.

There is a later email suggesting this be done in a panic notifier, and I
think that's a better approach. Then, instead of having this be a /proc file,
we could have a pseudo-device in /dev.

> Marco

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: David VomLehn @ 2009-11-12 21:58 UTC (permalink / raw)
  To: Matt Mackall; +Cc: linux-embedded, akpm, dwm2, linux-kernel, paul.gortmaker
In-Reply-To: <1258049211.10621.40.camel@calx>

On Thu, Nov 12, 2009 at 01:06:51PM -0500, Matt Mackall wrote:
> On Wed, 2009-11-11 at 21:13 -0500, David VomLehn wrote:
> > Allows annotation of panics to include platform information. It's no big
> > deal to collect information, but way helpful when you are collecting
> > failure reports from a eventual base of millions of systems deployed in
> > other people's homes.
> 
> I'd like to hear a bit more use case motivation on this feature. Also,
> why do you want more than a page?

Hopefully, I have addressed the first question in my previous email. As
for the second, I doubt there is a need for more than a page. I just picked
a value to start developing with. This is still a work in progress...

> http://selenic.com : development and support for Mercurial and Linux

David VL

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: David VomLehn @ 2009-11-12 22:09 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: linux-embedded, akpm, dwm2, linux-kernel, mpm
In-Reply-To: <4AFC6711.1090405@windriver.com>

On Thu, Nov 12, 2009 at 02:50:41PM -0500, Paul Gortmaker wrote:
> David VomLehn wrote:
>> Allows annotation of panics to include platform information. It's no big
>> deal to collect information, but way helpful when you are collecting
>> failure reports from a eventual base of millions of systems deployed in
>> other people's homes.
...
> Why hook into panic() directly like this, vs. using the panic
> notifier list? If you use that, and then put the data handling
> magic that you need into your own kernel module that knows how
> to interface with the reporting apps that you have, you can
> do the whole thing without having to alter existing code, I think.

I agree--a panic notifier list is probably a better approach.

David VL

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Simon Kagstrom @ 2009-11-13  8:10 UTC (permalink / raw)
  To: David VomLehn
  Cc: Marco Stornelli, linux-embedded, akpm, dwm2, linux-kernel, mpm,
	paul.gortmaker
In-Reply-To: <20091112215649.GA28349@dvomlehn-lnx2.corp.sa.net>

On Thu, 12 Nov 2009 16:56:49 -0500
David VomLehn <dvomlehn@cisco.com> wrote:

> Good question. Some more detail on our application might help. In some
> situations, we may have no disk and only enough flash for the bootloader.
> The kernel is downloaded over the network. When we get to user space, we
> initialize a number of things dynamically. For example, we dynamically
> compute some MAC address, and most of the IP addresses are obtained with
> DHCP. This are very useful to have for panic analysis.
> 
> Since there is neither flash nor disk, user space has no place to store
> this information, should the kernel panic. When we come back up, we will get
> different MAC and IP addresses. Storing them in memory is our only hope.
> 
> Fortunately, there is a section of RAM that the bootloader promises not
> to overwrite. On a panic, we capture the messages written on the console
> and store them in the protected area. If the information from the
> /proc file is written as part of the panic, we will capture it, too.

Can't you solve this completely from userspace using phram and mtdoops
instead? I.e., setup two phram areas

	modprobe phram 4K@start-of-your-area,4K@start-of-your-area+4K    # Can't remember the exact syntax!

you'll then get /dev/mtdX and /dev/mtdX+1 for these two. You can then do

	modprobe mtdoops mtddev=/dev/mtdX+1 dump_oops=0

to load mtdoops to catch the panic in the second area, and just write
your userspace messages to /dev/mtdX.


One thing probably have to be fixed though: I don't think phram has a
panic_write, which will be needed by mtdoops to catch the panic - this
should be trivial to add though since it's plain RAM.

// Simon

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Artem Bityutskiy @ 2009-11-13 11:26 UTC (permalink / raw)
  To: David VomLehn
  Cc: linux-embedded, akpm, dwm2, linux-kernel, mpm, paul.gortmaker
In-Reply-To: <20091112021322.GA6166@dvomlehn-lnx2.corp.sa.net>

On Wed, 2009-11-11 at 21:13 -0500, David VomLehn wrote:
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 96b45d0..513deae 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -70,6 +70,7 @@ NORET_TYPE void panic(const char * fmt, ...)
>         vsnprintf(buf, sizeof(buf), fmt, args);
>         va_end(args);
>         printk(KERN_EMERG "Kernel panic - not syncing: %s\n",buf);
> +       panic_note_print();
>  #ifdef CONFIG_DEBUG_BUGVERBOSE
>         dump_stack();
>  #endif
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug

You should use panic notifiers instead. See the panic_notifier_list.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Artem Bityutskiy @ 2009-11-13 11:35 UTC (permalink / raw)
  To: Matt Mackall
  Cc: David VomLehn, linux-embedded, akpm, dwm2, linux-kernel,
	paul.gortmaker
In-Reply-To: <1258049211.10621.40.camel@calx>

On Thu, 2009-11-12 at 12:06 -0600, Matt Mackall wrote:
> On Wed, 2009-11-11 at 21:13 -0500, David VomLehn wrote:
> > Allows annotation of panics to include platform information. It's no big
> > deal to collect information, but way helpful when you are collecting
> > failure reports from a eventual base of millions of systems deployed in
> > other people's homes.
> 
> I'd like to hear a bit more use case motivation on this feature. Also,
> why do you want more than a page?

We also need this kind of functionality. The use case is very simple.
Every time the kernel oopeses, we save the oops information on the flash
using mtdoops module. There is even core support, which should be merged
to 2.6.33, see this patch:

http://git.infradead.org/users/dedekind/l2-mtd-2.6.git/commit/832c3d00e82f267316a2b53634631a1821eebae8

(and there was a corresponding discussion on lkml).

And what we want is to dump information about the user-space environment
at the same time to the oops. Specifically, we want to dump information
about what was the SW build number.

And we want this information to be printed at the same time, because we
cannot run any user-space at the panic time. This information is later
read from the flash and sent via the network to the central place. And
by the time it is sent, the user may have already re-flashed his device
with something else.

So I very much appreciate this patch, although I think it should use the
panic notifiers instead of calling a function directly from the panic.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Artem Bityutskiy @ 2009-11-13 11:45 UTC (permalink / raw)
  To: Simon Kagstrom
  Cc: David VomLehn, Marco Stornelli, linux-embedded, akpm, dwm2,
	linux-kernel, mpm, paul.gortmaker
In-Reply-To: <20091113091031.3f6d4bba@marrow.netinsight.se>

On Fri, 2009-11-13 at 09:10 +0100, Simon Kagstrom wrote:
> On Thu, 12 Nov 2009 16:56:49 -0500
> David VomLehn <dvomlehn@cisco.com> wrote:
> 
> > Good question. Some more detail on our application might help. In some
> > situations, we may have no disk and only enough flash for the bootloader.
> > The kernel is downloaded over the network. When we get to user space, we
> > initialize a number of things dynamically. For example, we dynamically
> > compute some MAC address, and most of the IP addresses are obtained with
> > DHCP. This are very useful to have for panic analysis.
> > 
> > Since there is neither flash nor disk, user space has no place to store
> > this information, should the kernel panic. When we come back up, we will get
> > different MAC and IP addresses. Storing them in memory is our only hope.
> > 
> > Fortunately, there is a section of RAM that the bootloader promises not
> > to overwrite. On a panic, we capture the messages written on the console
> > and store them in the protected area. If the information from the
> > /proc file is written as part of the panic, we will capture it, too.
> 
> Can't you solve this completely from userspace using phram and mtdoops
> instead? I.e., setup two phram areas
> 
> 	modprobe phram 4K@start-of-your-area,4K@start-of-your-area+4K    # Can't remember the exact syntax!
> 
> you'll then get /dev/mtdX and /dev/mtdX+1 for these two. You can then do
> 
> 	modprobe mtdoops mtddev=/dev/mtdX+1 dump_oops=0
> 
> to load mtdoops to catch the panic in the second area, and just write
> your userspace messages to /dev/mtdX.

This might work for them, not sure, but not for us. We store panics on
flash, and later they are automatically sent to the panic collection
system via the network. And the complications are:

1. There may be many panics before the device has network access and has
a chance to send the panics.
2. User can re-flash the device with different SW inbetween.

So we really need to print some user-space supplied information during
the panic, and then we store it on flash with mtdoops, and the later,
when the device has network access we send whole bunch of oopses via the
network.

> One thing probably have to be fixed though: I don't think phram has a
> panic_write, which will be needed by mtdoops to catch the panic - this
> should be trivial to add though since it's plain RAM.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Shargorodsky Atal (EXT-Teleca/Helsinki) @ 2009-11-13 11:50 UTC (permalink / raw)
  To: ext David VomLehn
  Cc: Paul Gortmaker, linux-embedded@vger.kernel.org,
	akpm@linux-foundation.org, dwm2@infradead.org,
	linux-kernel@vger.kernel.org, mpm@selenic.com
In-Reply-To: <20091112220925.GC28349@dvomlehn-lnx2.corp.sa.net>

On Thu, 2009-11-12 at 23:09 +0100, ext David VomLehn wrote:
> On Thu, Nov 12, 2009 at 02:50:41PM -0500, Paul Gortmaker wrote:
> > David VomLehn wrote:
> >> Allows annotation of panics to include platform information. It's no big
> >> deal to collect information, but way helpful when you are collecting
> >> failure reports from a eventual base of millions of systems deployed in
> >> other people's homes.
> ...
> > Why hook into panic() directly like this, vs. using the panic
> > notifier list? If you use that, and then put the data handling
> > magic that you need into your own kernel module that knows how
> > to interface with the reporting apps that you have, you can
> > do the whole thing without having to alter existing code, I think.
> 
> I agree--a panic notifier list is probably a better approach.
> 

That's what we currently use:

http://marc.info/?l=linux-kernel&m=125655380512117&w=2

Very simple and does the job.
Requires to rearrange the panic code a bit, though.
Namely, to move notifiers invocation before the call to kmsg_dump().
Dumpers were introduced by Simon Kagstrom here:

http://marc.info/?l=linux-kernel&m=125569530109871&w=2


Regards,
Atal

> David VL
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Simon Kagstrom @ 2009-11-13 11:59 UTC (permalink / raw)
  To: dedekind1
  Cc: David VomLehn, Marco Stornelli, linux-embedded, akpm, dwmw2,
	linux-kernel, mpm, paul.gortmaker,
	Shargorodsky Atal (EXT-Teleca/Helsinki)
In-Reply-To: <1258112748.21596.1227.camel@localhost>

(Also fix David Woodhouses address and add Atal)

On Fri, 13 Nov 2009 13:45:48 +0200
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> So we really need to print some user-space supplied information during
> the panic, and then we store it on flash with mtdoops, and the later,
> when the device has network access we send whole bunch of oopses via the
> network.

Yes, I see that your case would have to be handled differently. A
complication (which I believe was discussed before) is that kmsg_dump()
is done before the panic notifiers are called.

The reason I put it there is to have it before crash_kexec(), so I
guess we'll have to take up the discussion on what to do with it. For
me it now seems like it would be OK to move kmsg_dump() down below the
panic notifiers.

If you have a kdump kernel to load, then you will most likely not need
the kmsg dumped data anyway.

// Simon

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Artem Bityutskiy @ 2009-11-13 14:16 UTC (permalink / raw)
  To: Simon Kagstrom
  Cc: David VomLehn, Marco Stornelli, linux-embedded, akpm, dwmw2,
	linux-kernel, mpm, paul.gortmaker,
	Shargorodsky Atal (EXT-Teleca/Helsinki)
In-Reply-To: <20091113125946.5353164c@marrow.netinsight.se>

On Fri, 2009-11-13 at 12:59 +0100, Simon Kagstrom wrote:
> (Also fix David Woodhouses address and add Atal)
> 
> On Fri, 13 Nov 2009 13:45:48 +0200
> Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 
> > So we really need to print some user-space supplied information during
> > the panic, and then we store it on flash with mtdoops, and the later,
> > when the device has network access we send whole bunch of oopses via the
> > network.
> 
> Yes, I see that your case would have to be handled differently. A
> complication (which I believe was discussed before) is that kmsg_dump()
> is done before the panic notifiers are called.
> 
> The reason I put it there is to have it before crash_kexec(), so I
> guess we'll have to take up the discussion on what to do with it. For
> me it now seems like it would be OK to move kmsg_dump() down below the
> panic notifiers.
> 
> If you have a kdump kernel to load, then you will most likely not need
> the kmsg dumped data anyway.

Yeah, I think this is a separate issue which can be fixed separately.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

^ permalink raw reply

* Re: [PATCH 0/6] Generic PWM API implementation
From: Grant Likely @ 2009-11-13 19:08 UTC (permalink / raw)
  To: Bill Gatliff; +Cc: linux-embedded, Mike Frysinger, David Brownell
In-Reply-To: <cover.1224093510.git.bgat@billgatliff.com>

Hi Bill

On Wed, Oct 15, 2008 at 11:14 AM, Bill Gatliff <bgat@billgatliff.com> wrote:
> This series implements a Generic PWM Device API, including reference
> implementations for the Atmel PWMC device, an LED device, and an LED
> trigger.  It is based on linux-2.6.27.
[...]
> The implementation of the Generic PWM Device API is structurally
> similar to the generic GPIO API, except that the PWM code uses
> platform bus_id strings instead of integers to identify target
> deviices.  A configuration structure is also provided, both to
> facilitate atomic hardware state changes and so that the API can be
> extended in a source-code-compatible way to accomodate devices with
> features not anticipated by the current code.

Hey Bill,

I'm concerned about the approach taken here.  As I understand it, the
PWM signals are very similar to GPIOs in that each PWM device controls
an external signal line, just like GPIO lines.  The difference being
that PWMs cannot do input, and has additional capabilities (can be
programmed with a signal; not just on/off/tristate).  Actually, many
GPIOs have these properties too.  I've got a part with output-only
gpios, and gpio devices that also have a PWM.

What is the reason for bringing in an entirely new framework instead
of extending the GPIO API or gpiolib?  I'm not too excited about
having two entirely different frameworks for what basically boils down
to "numbered signal pins".

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 0/6] Generic PWM API implementation
From: Mike Frysinger @ 2009-11-14  4:22 UTC (permalink / raw)
  To: Grant Likely; +Cc: Bill Gatliff, linux-embedded, David Brownell
In-Reply-To: <fa686aa40911131108o301efa1ew5c0ab31184aa7f75@mail.gmail.com>

On Fri, Nov 13, 2009 at 14:08, Grant Likely wrote:
> On Wed, Oct 15, 2008 at 11:14 AM, Bill Gatliff wrote:
>> This series implements a Generic PWM Device API, including reference
>> implementations for the Atmel PWMC device, an LED device, and an LED
>> trigger.  It is based on linux-2.6.27.
> [...]
>> The implementation of the Generic PWM Device API is structurally
>> similar to the generic GPIO API, except that the PWM code uses
>> platform bus_id strings instead of integers to identify target
>> deviices.  A configuration structure is also provided, both to
>> facilitate atomic hardware state changes and so that the API can be
>> extended in a source-code-compatible way to accomodate devices with
>> features not anticipated by the current code.
>
> I'm concerned about the approach taken here.  As I understand it, the
> PWM signals are very similar to GPIOs in that each PWM device controls
> an external signal line, just like GPIO lines.  The difference being
> that PWMs cannot do input, and has additional capabilities (can be
> programmed with a signal; not just on/off/tristate).  Actually, many
> GPIOs have these properties too.  I've got a part with output-only
> gpios, and gpio devices that also have a PWM.
>
> What is the reason for bringing in an entirely new framework instead
> of extending the GPIO API or gpiolib?  I'm not too excited about
> having two entirely different frameworks for what basically boils down
> to "numbered signal pins".

unifying resource management obviously makes sense so as to avoid
conflicts, but i dont think the fact that one pin can be multi purpose
means it should be entirely forced into the GPIO framework, nor do i
see any real gain for doing so.
-mike

^ permalink raw reply

* Re: [PATCH 0/6] Generic PWM API implementation
From: Grant Likely @ 2009-11-14  7:55 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: Bill Gatliff, linux-embedded, David Brownell
In-Reply-To: <8bd0f97a0911132022pf743558lfef1402125d4b60b@mail.gmail.com>

On Fri, Nov 13, 2009 at 9:22 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> On Fri, Nov 13, 2009 at 14:08, Grant Likely wrote:
>> On Wed, Oct 15, 2008 at 11:14 AM, Bill Gatliff wrote:
>>> This series implements a Generic PWM Device API, including reference
>>> implementations for the Atmel PWMC device, an LED device, and an LED
>>> trigger.  It is based on linux-2.6.27.
>> [...]
>>> The implementation of the Generic PWM Device API is structurally
>>> similar to the generic GPIO API, except that the PWM code uses
>>> platform bus_id strings instead of integers to identify target
>>> deviices.  A configuration structure is also provided, both to
>>> facilitate atomic hardware state changes and so that the API can be
>>> extended in a source-code-compatible way to accomodate devices with
>>> features not anticipated by the current code.
>>
>> I'm concerned about the approach taken here.  As I understand it, the
>> PWM signals are very similar to GPIOs in that each PWM device controls
>> an external signal line, just like GPIO lines.  The difference being
>> that PWMs cannot do input, and has additional capabilities (can be
>> programmed with a signal; not just on/off/tristate).  Actually, many
>> GPIOs have these properties too.  I've got a part with output-only
>> gpios, and gpio devices that also have a PWM.
>>
>> What is the reason for bringing in an entirely new framework instead
>> of extending the GPIO API or gpiolib?  I'm not too excited about
>> having two entirely different frameworks for what basically boils down
>> to "numbered signal pins".
>
> unifying resource management obviously makes sense so as to avoid
> conflicts, but i dont think the fact that one pin can be multi purpose
> means it should be entirely forced into the GPIO framework, nor do i
> see any real gain for doing so.

Common code is a big gain in and of itself.  It means less to develop,
less to maintain, and fewer APIs in the kernel.  Right now, I don't
see a fundamental difference is between GPIO and PWM pin management.
It is essentially the same problem, and in many cases PWM pins can
also be used as GPIOs.  I think the question should be flipped around;
rather than asking for a compelling reason for them to be merged; I
want to know the compelling reason to keep them separate.  What is the
fundamental difference that keeps them apart?

However, since it was mentioned, I do see some real gains for using
the same infrastructure:
- Devices using GPIO pins can easily be extended to take advantage of
PWM modes.  i could see GPIO LEDs taking advantage of this for example
- (as I already mentioned) PWM pins that can also behave as GPIOs
don't need to register 2 interfaces.
- All the existing support code for hooking up GPIO pins to other
devices can be reused as is.
- Individual platforms have the option of implementing the GPIO+PWM
API directly (fast, but static), or they can hook in via GPIOLIB
(dynamic; slower but pluggable)
- All the OF device tree bindings for GPIOs also work with PWMs.

What I would like to see is the PWM functions added to the GPIO API.
GPIO drivers can then either implement them or not.  If a GPIO driver
supports the PWM function, great.  If not, then it returns -EINVAL.
Heck, I'll even got a driver right now that I'd use it with.  I'm more
than happy to help code it up even.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Marco Stornelli @ 2009-11-14  8:28 UTC (permalink / raw)
  To: dedekind1
  Cc: Simon Kagstrom, David VomLehn, linux-embedded, akpm, dwm2,
	linux-kernel, mpm, paul.gortmaker
In-Reply-To: <1258112748.21596.1227.camel@localhost>

I think in general the procedure should be: at startup or event (for
example acquired IP address from DHCP) user applications write in flash
(better in persistent ram) a log with a tag or a timestamp or something
like this, when there is a kernel panic, it is captured in a file stored
together the log and when possible the system should send all via
network for example. Are there problems that I can't see to follow this
approach? When David says "...so this looks much more like a real file
than a sysctl file" I quite agree, it seems a normal application/system
log indeed.

Marco

Artem Bityutskiy wrote:
> On Fri, 2009-11-13 at 09:10 +0100, Simon Kagstrom wrote:
>> On Thu, 12 Nov 2009 16:56:49 -0500
>> David VomLehn <dvomlehn@cisco.com> wrote:
>>
>>> Good question. Some more detail on our application might help. In some
>>> situations, we may have no disk and only enough flash for the bootloader.
>>> The kernel is downloaded over the network. When we get to user space, we
>>> initialize a number of things dynamically. For example, we dynamically
>>> compute some MAC address, and most of the IP addresses are obtained with
>>> DHCP. This are very useful to have for panic analysis.
>>>
>>> Since there is neither flash nor disk, user space has no place to store
>>> this information, should the kernel panic. When we come back up, we will get
>>> different MAC and IP addresses. Storing them in memory is our only hope.
>>>
>>> Fortunately, there is a section of RAM that the bootloader promises not
>>> to overwrite. On a panic, we capture the messages written on the console
>>> and store them in the protected area. If the information from the
>>> /proc file is written as part of the panic, we will capture it, too.
>> Can't you solve this completely from userspace using phram and mtdoops
>> instead? I.e., setup two phram areas
>>
>> 	modprobe phram 4K@start-of-your-area,4K@start-of-your-area+4K    # Can't remember the exact syntax!
>>
>> you'll then get /dev/mtdX and /dev/mtdX+1 for these two. You can then do
>>
>> 	modprobe mtdoops mtddev=/dev/mtdX+1 dump_oops=0
>>
>> to load mtdoops to catch the panic in the second area, and just write
>> your userspace messages to /dev/mtdX.
> 
> This might work for them, not sure, but not for us. We store panics on
> flash, and later they are automatically sent to the panic collection
> system via the network. And the complications are:
> 
> 1. There may be many panics before the device has network access and has
> a chance to send the panics.
> 2. User can re-flash the device with different SW inbetween.
> 
> So we really need to print some user-space supplied information during
> the panic, and then we store it on flash with mtdoops, and the later,
> when the device has network access we send whole bunch of oopses via the
> network.
> 
>> One thing probably have to be fixed though: I don't think phram has a
>> panic_write, which will be needed by mtdoops to catch the panic - this
>> should be trivial to add though since it's plain RAM.
> 

^ permalink raw reply

* [Proposal] [PATCH] [RESEND] generic clock framework
From: Francesco VIRLINZI @ 2009-11-16 13:47 UTC (permalink / raw)
  To: Linux-sh, linux-embedded, linux-arm-kernel, linux-pm,
	linux-kernel, linuxppc-de

Hi all

I'm Francesco and I work in STMicroelectronics

In the last ELC-E_2009 I spoke on a generic clock framework I'm working on
  (see 
http://tree.celinuxforum.org/CelfPubWiki/ELCEurope2009Presentations?action=AttachFile&do=view&target=ELC_E_2009_Generic_Clock_Framework.pdf). 


I wrote the gcf to manage both clocks the platform_devices during a 
clock operation.

The main features are:
  - it's integrated in the LDM
  - it tracks the clock-to-clock relationship
  - it tracks the clock-to-device relationship

  - it has sysfs interface
  - - the user can navigate the clock tree under /sys/clocks/...

  - it uses the linux API (<linux/clk.h>) with some extra functions (to 
register/unregister a clock
    and other utility functions as clk_for_each())

  - it involves the platform_device and the platform_driver in the clock 
propagation.
  - - basically each clock operation is managed as a transaction which 
evolves step by step.
  - - all the clock rates are evaluated (before the clk operation is 
actually done)
  - - each platform_device can check (before the clk operation is 
actually done) the clk environment
      it will have at the end of clock operation and if required it can 
reject the operation.
  - - each clock operation is actually executed only if all the 
platform_devices accept the operation it-self


Moreover a common clock framework could be used to avoid a lot of 
duplicated and/or similar code
  just a grep of 'EXPORT_SYMBOL\(clk_enable' under arch/arm finds 22 
entries.

The patch is based on a 2.6.30 kernel also if it has a preliminary 
integration with the PM_RUNTIME
  support.

It works on our st40 (an sh4 cpu based system) no test/porting was done 
on other platforms.

It would be mainly a starting point for a discussion and I'm available 
to extend/fix/share it.

Regards
  Francesco

^ permalink raw reply

* Re: [PATCH 0/6] Generic PWM API implementation
From: David Brownell @ 2009-11-17  7:47 UTC (permalink / raw)
  To: Grant Likely; +Cc: Mike Frysinger, Bill Gatliff, linux-embedded
In-Reply-To: <fa686aa40911132355t3b79fe46m90005ecba453dcc1@mail.gmail.com>

On Friday 13 November 2009, Grant Likely wrote:
> 		  Right now, I don't
> see a fundamental difference is between GPIO and PWM pin management.
> It is essentially the same problem, and in many cases PWM pins can
> also be used as GPIOs. 

Pin management for a given SoC is going to be relevant to setting
every signal, no matter what peripheral it's associated with.  The
same argument applies to an MDIO bus, I2C, 1-wire, and more.

And I don't buy it in those cases either.


> I think the question should be flipped around; 
> rather than asking for a compelling reason for them to be merged; I
> want to know the compelling reason to keep them separate.  What is the
> fundamental difference that keeps them apart?

PWM is about periodic signal generation without CPU intervention.

GPIO is about explicit CPU management/interrogation of single
signals.


> What I would like to see is the PWM functions added to the GPIO API.

No.  If you want a pin mux interface, come up with one of them.

It shouldn't be a PWM interface, a GPIO interface, an I2C interface,
a SPI interface, an MDIO interface, a 1-wire interface ... or any of
dozens of other things.  It'd be purely for pinmux.


^ permalink raw reply

* Re: [PATCH 0/6] Generic PWM API implementation
From: David Brownell @ 2009-11-17  8:27 UTC (permalink / raw)
  To: Grant Likely; +Cc: Bill Gatliff, linux-embedded, Mike Frysinger
In-Reply-To: <fa686aa40911131108o301efa1ew5c0ab31184aa7f75@mail.gmail.com>

On Friday 13 November 2009, Grant Likely wrote:
> I'm concerned about the approach taken here.  As I understand it, the
> PWM signals are very similar to GPIOs in that each PWM device controls
> an external signal line, just like GPIO lines.

PWM is not GPIO, and doesn't fit into a GPIO framework.

Since *everything* boils down to one or more signal lines,
your argument leads directly to Linux having no native
hardware interface except GPIOs.  Not ... practical. ;)



> The difference being 
> that PWMs cannot do input, and has additional capabilities (can be
> programmed with a signal; not just on/off/tristate)

If you want to combine PWM with something else ... timers would
be a better target.  They're both fundamentally about periodic
phenomena.  And quite a lot of timers support PWM output modes...

(A generic interface to hardware timers is lacking, too.)


> What is the reason for bringing in an entirely new framework instead
> of extending the GPIO API or gpiolib?  I'm not too excited about
> having two entirely different frameworks for what basically boils down
> to "numbered signal pins".

You seem to mis-understand what PWM is all about, then.
The whole point of a PWM is to set up a periodic activity
that will run without CPU intervention.

GPIOs, on the other hand, are packaged for manual bit
twiddling.  While it's possible to create low-speed
implementations of serial protocols using GPIOs (like
2-wire/I2C, one-wire, and various SPI variants), those
are explicitly the high-overhead (and low performance)
substitutes, to be used only when native hardware isn't
available (or is broken etc).

For example you won't often get 40 Mbit/sec SPI if you
are bitbanging over GPIOs ... and if you do, it won't
leave much CPU horsepower for much of anything else.

- Dave

^ permalink raw reply

* Re: [[RFC] 2/5] Emulates PWM hardware using a high-resolution timer and a GPIO pin
From: David Brownell @ 2009-11-17  8:29 UTC (permalink / raw)
  To: Bill Gatliff; +Cc: linux-embedded
In-Reply-To: <1255984366-26952-3-git-send-email-bgat@billgatliff.com>

Worth highlighting that this is necessarily a "low quality"
PWM ... in the sense that it's got lots of jitter because
of needing CPU intervention in IRQ context, so it's subject
to delays from both IRQs being blocked and from other timer
driven activities firing first.

There are lots of applications where that jitter is enough
to preclude using this kind of PWM.  I get the feeling that
some of the Linux folk seeing "this PWM thing" are not very
cognizant of such issues ... they've not had to use PWM to
do anything where the jitter matters.  (Not that I have;
but I know that such apps are a motivation for most of the
PWM hardware on microcontrollers.  A few PWMs plus some
sensors, hall effect or QEI or whatever; then you get a
motor controller.)

- Dave

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Artem Bityutskiy @ 2009-11-17  8:53 UTC (permalink / raw)
  To: Marco Stornelli
  Cc: Simon Kagstrom, David VomLehn, linux-embedded, akpm, dwm2,
	linux-kernel, mpm, paul.gortmaker
In-Reply-To: <4AFE6A14.4010507@gmail.com>

Hi Marko,

On Sat, 2009-11-14 at 09:28 +0100, Marco Stornelli wrote:
> I think in general the procedure should be: at startup or event (for
> example acquired IP address from DHCP) user applications write in flash
> (better in persistent ram) a log with a tag or a timestamp or something
> like this, when there is a kernel panic, it is captured in a file stored
> together the log and when possible the system should send all via
> network for example. Are there problems that I can't see to follow this
> approach? When David says "...so this looks much more like a real file
> than a sysctl file" I quite agree, it seems a normal application/system
> log indeed.

Please, do not top-post, you make it difficult to discuss things by
messing up the context. Be consistent with how other people reply in
this thread, please.

Take a look at my mails where I describe different complications we have
in our system. We really want to have an OOPS/panic + our environment
stuff to go together, at once. This makes things so much simpler.

Really, what is the problem providing this trivial panic-note
capability, where user-space can give the kernel a small buffer, and ask
the kernel to print this buffer at the oops/panic time. Very simple and
elegant, and just solves the problem.

Why perversions with time-stamps, separate storages are needed?

IOW, you suggest a complicated approach, and demand explaining why we do
not go for it. Simply because it is unnecessarily complex. This patch
solves the problem gracefully, and I'd rather demand you to point what
is the technical problem with the patches.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

^ permalink raw reply

* Re: [PATCH, RFC] panic-note: Annotation from user space for panics
From: Artem Bityutskiy @ 2009-11-17  9:03 UTC (permalink / raw)
  To: David VomLehn
  Cc: linux-embedded, akpm, dwm2, linux-kernel, mpm, paul.gortmaker
In-Reply-To: <20091112021322.GA6166@dvomlehn-lnx2.corp.sa.net>

A pair of nit-picks.

On Wed, 2009-11-11 at 21:13 -0500, David VomLehn wrote:
> @@ -0,0 +1,293 @@
> +/*
> + *				panic-note.c

No need to type file name there.

> + *
> + * Allow a blob to be registered with the kernel that will be printed if
> + * the kernel panics.
> + *
> + * Copyright (C) 2009  Cisco Systems, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + */
> +
> +/* Open issues:
> + * Where should the panic_note file be created? It's almost like a sysctl,
> + * but doesn't follow the same rules. When you write to a sysctl file, the
> + * previous data is replaced. When you write to the panic_note file, you
> + * can append to the end of the existing data.
> + */

Please, take a look at what is the kernel comment style at
Documentation/CodingStyle. We use

/*
 * Multi-line
 * comment
 */

style.

> +
> +#include <linux/semaphore.h>
> +#include <linux/fs.h>
> +#include <linux/proc_fs.h>
> +#include <linux/module.h>
> +#include <linux/uaccess.h>
> +
> +/* Maximum size, in bytes, allowed for the blob. Having this limit prevents
> + * an inadvertant denial of service attack that might happen if someone with
> + * root privileges was automatically generating this note and the generator
> + * had an infinite loop. Perhaps this is more of a a denial of service
> + * suicide. */
> +#define PANIC_NOTE_SIZE		(PAGE_SIZE * 4)
> +
> +/*
> + * struct panic_note_data - Information about the panic note
> + * @n:		Number of bytes in the note
> + * @p:		Pointer to the data in the note
> + * @sem:	Semaphore controlling access to data in the note
> + */
> +struct panic_note_state {
> +	size_t			n;
> +	void			*p;
> +	struct rw_semaphore	sem;
> +};
> +
> +static struct panic_note_state panic_note_state = {
> +	0, NULL, __RWSEM_INITIALIZER(panic_note_state.sem)
> +};
> +static const struct file_operations panic_note_fops;
> +static struct inode_operations panic_note_iops;
> +static struct proc_dir_entry *panic_note_entry;
> +
> +/*
> + * panic_note_print - display the panic note
> + * @priority:	Printk priority to use, e.g. KERN_EMERG
> + */
> +void panic_note_print()
> +{
> +	int i;
> +	int linelen;
int i, lineline is more compact.

> +
> +	/* We skip the semaphore stuff because we're in a panic situation and
> +	 * the scheduler isn't available in case the semaphore is already owned
> +	 * by someone else */
> +	for (i = 0; i < panic_note_state.n; i += linelen) {
> +		const char *p;
> +		int remaining;
> +		const char *nl;

const char *p, *nl is more compact. And there are several more places.
But this is matter of taste, really.

> +
> +		p = panic_note_state.p + i;
> +		remaining = panic_note_state.n - i;
> +
> +		nl = memchr(p, '\n', remaining);
> +
> +		if (nl == NULL) {
> +			linelen = remaining;
> +			pr_emerg("%.*s\n", linelen, p);
> +		} else {
> +			linelen = nl - p + 1;
> +			pr_emerg("%.*s", linelen, p);
> +		}
> +	}
> +}
> +
> +/*
> + * read_write_size - calculate the limited copy_to_user/copy_from_user count
> + * @nbytes:	The number of bytes requested
> + * @pos:	Offset, in bytes, into the file
> + * @size:	Maximum I/O offset, in bytes. For a read, this is the actual
> + *		number of bytes in the file, since you can't read past
> + *		the end. Writes can be done after the number of bytes in the
> + *		file, so this is the maximum possible file size, minus one.
> + *
> + * Returns the number of bytes to copy.
> + */
> +static ssize_t read_write_size(size_t nbytes, loff_t pos, size_t size)
> +{
> +	ssize_t retval;
> +
> +	if (pos >= size)
> +		retval = 0;
> +
Unnecessary \n
> +	else {
> +		retval = size - pos;
> +		if (retval > nbytes)
> +			retval = nbytes;
> +	}
> +
> +	return retval;
> +}


-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox