Re: [PATCH v3 3/9] pstore: add support for external writers

[Anton Vorontsov]

On Thu, Oct 18, 2012 at 02:06:01PM +0300, dragos.tatulea@xxxxxxxxx wrote:
> From: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> 
> Other modules that may wish to write to persistent storage
> are supported by adding a notifier and write function.
> 
> The notifier has 3 events: PSTORE_BEGIN, PSTORE_DUMP and
> PSTORE_END.  External writers use the PSTORE_DUMP event
> whereas the PSTORE_BEGIN and PSTORE_END can be used by
> platform code to ensure the back end is powered up.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
>  fs/pstore/platform.c   |   88 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pstore.h |   43 +++++++++++++++++++----
>  2 files changed, 124 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 9d65723..63ff377 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
[...]
> +/* pstore_write must only be called from PSTORE_DUMP notifier callbacks */
> +int pstore_write(enum pstore_type_id type, const char *buf, size_t size)
> +{
> +	size_t len;
> +	int err = 0, err2;

I guess there is no need for err2 here, you can place it in the while
loop, in the if block.

> +
> +	if (!psinfo)
> +		return -ENODEV;
> +
> +	/*
> +	 * No locking is needed because pstore_write is called only from
> +	 * PSTORE_DUMP notifier callbacks.
> +	 */
> +
> +	if (type != psinfo->ext_type) {
> +		err = pstore_ext_flush();
> +		psinfo->ext_type = type;
> +		psinfo->ext_part = 1;
> +	}
> +
> +	while (size) {
> +		len = min(size, psinfo->bufsize - psinfo->ext_len);
> +		memcpy(psinfo->buf + psinfo->ext_len, buf, len);
> +		psinfo->ext_len += len;
> +		buf += len;
> +		size -= len;
> +		if (psinfo->ext_len == psinfo->bufsize) {
> +			err2 = pstore_ext_flush();
> +			if (err2 && !err)
> +				err = err2;
> +		}
> +	}
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(pstore_write);
> +
>  module_param(backend, charp, 0444);
>  MODULE_PARM_DESC(backend, "Pstore backend to use");
> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> index 55ab23f..1bf7f4b 100644
> --- a/include/linux/pstore.h
> +++ b/include/linux/pstore.h
> @@ -40,17 +40,27 @@ enum pstore_type_id {
>  
>  struct module;
>  
> +/* Notifier events */
> +#define PSTORE_BEGIN		1
> +#define PSTORE_DUMP		2
> +#define PSTORE_END		3
> +
>  #define PSTORE_NO_HEADINGS	BIT(0)
>  #define PSTORE_MAX_KMSG_BYTES	BIT(1)
>  
>  struct pstore_info {
> -	struct module	*owner;
> -	char		*name;
> -	unsigned int	flags;
> -	spinlock_t	buf_lock;	/* serialize access to 'buf' */
> -	char		*buf;
> -	size_t		bufsize;
> -	struct mutex	read_mutex;	/* serialize open/read/close */

That's why I don't like "pretty" formatting for structs. :) The change is
unrelated, so please don't do this.

> +	struct module		*owner;
> +	char			*name;
> +	unsigned int		flags;
> +	spinlock_t		buf_lock;	/* serialize access to 'buf' */
> +	char			*buf;
> +	size_t			bufsize;
> +	struct mutex		read_mutex;	/* serialize open/read/close */
> +	u64			ext_id;
> +	size_t			ext_len;
> +	unsigned int		ext_part;
> +	enum pstore_type_id	ext_type;
> +	enum kmsg_dump_reason	ext_reason;
>  	int		(*open)(struct pstore_info *psi);
>  	int		(*close)(struct pstore_info *psi);
>  	ssize_t		(*read)(u64 *id, enum pstore_type_id *type,
> @@ -70,12 +80,31 @@ struct pstore_info {
>  
>  #ifdef CONFIG_PSTORE
>  extern int pstore_register(struct pstore_info *);
> +extern int pstore_notifier_register(struct notifier_block *n);
> +extern int pstore_notifier_unregister(struct notifier_block *n);
> +/* pstore_write must only be called from PSTORE_DUMP notifier callbacks */
> +extern int pstore_write(enum pstore_type_id type, const char *buf, size_t size);
>  #else
>  static inline int
>  pstore_register(struct pstore_info *psi)
>  {
>  	return -ENODEV;
>  }
> +static inline int
> +pstore_notifier_register(struct notifier_block *n)
> +{
> +	return 0;
> +}
> +static inline int
> +pstore_notifier_unregister(struct notifier_block *n)
> +{
> +	return 0;
> +}
> +static inline int
> +pstore_write(enum pstore_type_id type, const char *buf, size_t size)
> +{
> +	return 0;
> +}
>  #endif
>  
>  #endif /*_LINUX_PSTORE_H*/
> -- 
> 1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/