Re: [PATCH v4 5/7] efi_pstore: Add a sequence counter to a variable name

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday, October 30, 2012 08:02:54 PM Seiji Aguchi wrote:
> 
> [Issue]
> 
> Currently, a variable name, which identifies each entry, consists of type, id and ctime.
> But if multiple events happens in a short time, a second/third event may fail to log because
> efi_pstore can't distinguish each event with current variable name.
> 
> [Solution]
> 
> A reasonable way to identify all events precisely is introducing a sequence counter to
> the variable name.
> 
> The sequence counter has already supported in a pstore layer with "oopscount".
> So, this patch adds it to a variable name.
> Also, it is passed to read/erase callbacks of platform drivers in accordance with
> the modification of the variable name.
> 
>   <before applying this patch>
>  a variable name of first event: dump-type0-1-12345678
>  a variable name of second event: dump-type0-1-12345678
> 
>   type:0
>   id:1
>   ctime:12345678
> 
>  If multiple events happen in a short time, efi_pstore can't distinguish them because
>  variable names are same among them.
> 
>   <after applying this patch>
> 
>  it can be distinguishable by adding a sequence counter as follows.
> 
>  a variable name of first event: dump-type0-1-1-12345678
>  a variable name of Second event: dump-type0-1-2-12345678
> 
>   type:0
>   id:1
>   sequence counter: 1(first event), 2(second event)
>   ctime:12345678
> 
> Signed-off-by: Seiji Aguchi <seiji.aguchi@xxxxxxx>

Please feel free to add

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

for the erst.c change.

Thanks!


> ---
>  drivers/acpi/apei/erst.c   |   12 ++++++------
>  drivers/firmware/efivars.c |   23 ++++++++++++++---------
>  fs/pstore/inode.c          |    8 +++++---
>  fs/pstore/internal.h       |    2 +-
>  fs/pstore/platform.c       |   11 ++++++-----
>  fs/pstore/ram.c            |    7 +++----
>  include/linux/pstore.h     |    8 +++++---
>  7 files changed, 40 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c
> index 0bd6ae4..6d894bf 100644
> --- a/drivers/acpi/apei/erst.c
> +++ b/drivers/acpi/apei/erst.c
> @@ -931,13 +931,13 @@ 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, int *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, int count,
>  		       size_t size, struct pstore_info *psi);
> -static int erst_clearer(enum pstore_type_id type, u64 id,
> +static int erst_clearer(enum pstore_type_id type, u64 id, int count,
>  			struct timespec time, struct pstore_info *psi);
>  
>  static struct pstore_info erst_info = {
> @@ -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, int *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, int count,
>  		       size_t size, struct pstore_info *psi)
>  {
>  	struct cper_pstore_record *rcd = (struct cper_pstore_record *)
> @@ -1101,7 +1101,7 @@ 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,
> +static int erst_clearer(enum pstore_type_id type, u64 id, int 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 3803621..7ad3aae 100644
> --- a/drivers/firmware/efivars.c
> +++ b/drivers/firmware/efivars.c
> @@ -658,13 +658,14 @@ 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,
> +			       int *count, struct timespec *timespec,
>  			       char **buf, struct pstore_info *psi)
>  {
>  	efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
>  	struct efivars *efivars = psi->data;
>  	char name[DUMP_NAME_LEN];
>  	int i;
> +	int cnt;
>  	unsigned int part, size;
>  	unsigned long time;
>  
> @@ -674,8 +675,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-%d-%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);
> @@ -698,7 +701,8 @@ 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, int count, size_t size,
> +		struct pstore_info *psi)
>  {
>  	char name[DUMP_NAME_LEN];
>  	efi_char16_t efi_name[DUMP_NAME_LEN];
> @@ -725,7 +729,7 @@ static int efi_pstore_write(enum pstore_type_id type,
>  		return -ENOSPC;
>  	}
>  
> -	sprintf(name, "dump-type%u-%u-%lu", type, part,
> +	sprintf(name, "dump-type%u-%u-%d-%lu", type, part, count,
>  		get_seconds());
>  
>  	for (i = 0; i < DUMP_NAME_LEN; i++)
> @@ -746,7 +750,7 @@ static int efi_pstore_write(enum pstore_type_id type,
>  	return ret;
>  };
>  
> -static int efi_pstore_erase(enum pstore_type_id type, u64 id,
> +static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count,
>  			    struct timespec time, struct pstore_info *psi)
>  {
>  	char name[DUMP_NAME_LEN];
> @@ -756,7 +760,7 @@ static int efi_pstore_erase(enum pstore_type_id type, u64 id,
>  	struct efivar_entry *entry, *found = NULL;
>  	int i;
>  
> -	sprintf(name, "dump-type%u-%u-%lu", type, (unsigned int)id,
> +	sprintf(name, "dump-type%u-%u-%d-%lu", type, (unsigned int)id, count,
>  		time.tv_sec);
>  
>  	spin_lock(&efivars->lock);
> @@ -807,7 +811,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, int *count,
>  			       struct timespec *timespec,
>  			       char **buf, struct pstore_info *psi)
>  {
> @@ -816,12 +820,13 @@ 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, int count, size_t size,
> +		struct pstore_info *psi)
>  {
>  	return 0;
>  }
>  
> -static int efi_pstore_erase(enum pstore_type_id type, u64 id,
> +static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count,
>  			    struct timespec time, struct pstore_info *psi)
>  {
>  	return 0;
> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> index 4300af6..ed1d8c7 100644
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -49,6 +49,7 @@ struct pstore_private {
>  	struct pstore_info *psi;
>  	enum pstore_type_id type;
>  	u64	id;
> +	int	count;
>  	ssize_t	size;
>  	char	data[];
>  };
> @@ -175,8 +176,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, dentry->d_inode->i_ctime,
> -			      p->psi);
> +		p->psi->erase(p->type, p->id, p->count,
> +			      dentry->d_inode->i_ctime, p->psi);
>  
>  	return simple_unlink(dir, dentry);
>  }
> @@ -271,7 +272,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, int count,
>  		  char *data, size_t size, struct timespec time,
>  		  struct pstore_info *psi)
>  {
> @@ -307,6 +308,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 4847f58..937d820 100644
> --- a/fs/pstore/internal.h
> +++ b/fs/pstore/internal.h
> @@ -50,7 +50,7 @@ extern struct pstore_info *psinfo;
>  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,
> +			      int 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 a40da07..f911bbd 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -136,7 +136,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;
>  
> @@ -196,7 +196,7 @@ static void pstore_register_console(void) {}
>  
>  static int pstore_write_compat(enum pstore_type_id type,
>  			       enum kmsg_dump_reason reason,
> -			       u64 *id, unsigned int part,
> +			       u64 *id, unsigned int part, int count,
>  			       size_t size, struct pstore_info *psi)
>  {
>  	return psi->write_buf(type, reason, id, part, psinfo->buf, size, psi);
> @@ -266,6 +266,7 @@ void pstore_get_records(int quiet)
>  	char			*buf = NULL;
>  	ssize_t			size;
>  	u64			id;
> +	int			count;
>  	enum pstore_type_id	type;
>  	struct timespec		time;
>  	int			failed = 0, rc;
> @@ -277,9 +278,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 749693f..2bfa36e 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -132,9 +132,8 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint *c, uint max,
>  }
>  
>  static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
> -				   struct timespec *time,
> -				   char **buf,
> -				   struct pstore_info *psi)
> +				   int *count, struct timespec *time,
> +				   char **buf, struct pstore_info *psi)
>  {
>  	ssize_t size;
>  	struct ramoops_context *cxt = psi->data;
> @@ -236,7 +235,7 @@ static int notrace ramoops_pstore_write_buf(enum pstore_type_id type,
>  	return 0;
>  }
>  
> -static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
> +static int ramoops_pstore_erase(enum pstore_type_id type, u64 id, int count,
>  				struct timespec time, struct pstore_info *psi)
>  {
>  	struct ramoops_context *cxt = psi->data;
> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> index f6e9336..1788909 100644
> --- a/include/linux/pstore.h
> +++ b/include/linux/pstore.h
> @@ -50,17 +50,19 @@ 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,
> +			int *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, int count, size_t size,
> +			struct pstore_info *psi);
>  	int		(*write_buf)(enum pstore_type_id type,
>  			enum kmsg_dump_reason reason, u64 *id,
>  			unsigned int part, const char *buf, size_t size,
>  			struct pstore_info *psi);
>  	int		(*erase)(enum pstore_type_id type, u64 id,
> -			struct timespec time, struct pstore_info *psi);
> +			int count, struct timespec time,
> +			struct pstore_info *psi);
>  	void		*data;
>  };
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux