Skip to contentSkip to conversation

efi-pstore: Make efi-pstore return a unique id

PATCH In review Madper Xie 3 participants 4 messages Nov 23, 2013 – Nov 28, 2013

Subsystems (2)
EFI Pstore filesystem
lore
Share & tools
lore

Madper Xie

author
#0 lore raw

PATCH v2 1/1 efi-pstore: Make efi-pstore return a unique id

Pstore fs expects that backends provide a uniqued id which could avoid pstore making entries as duplication or denominating entries the same name. So I combine the timestamp, part and count into id.

Signed-off-by Madper Xie <[email protected]>
---
 drivers/firmware/efi/efi-pstore.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
drivers/firmware/efi/efi-pstore.c +13 -6
index 5002d50..c6075f8 100644
@@ -39,6 +39,12 @@ struct pstore_read_data {
 	char **buf;
 };
 
+static inline u64 generic_id(unsigned long timestamp,
+			     unsigned int part, int count)
+{
+	return (timestamp * 100 + part) * 1000 + count;
+}
+
 static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
 {
 	efi_guid_t vendor = LINUX_EFI_CRASH_GUID;

@@ -57,7 +63,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
 
 	if (sscanf(name, "dump-type%u-%u-%d-%lu-%c",
 		   cb_data->type, &part, &cnt, &time, &data_type) == 5) {
-		*cb_data->id = part;
+		*cb_data->id = generic_id(time, part, cnt);
 		*cb_data->count = cnt;
 		cb_data->timespec->tv_sec = time;
 		cb_data->timespec->tv_nsec = 0;

@@ -67,7 +73,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
 			*cb_data->compressed = false;
 	} else if (sscanf(name, "dump-type%u-%u-%d-%lu",
 		   cb_data->type, &part, &cnt, &time) == 4) {
-		*cb_data->id = part;
+		*cb_data->id = generic_id(time, part, cnt);
 		*cb_data->count = cnt;
 		cb_data->timespec->tv_sec = time;
 		cb_data->timespec->tv_nsec = 0;

@@ -79,7 +85,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
 		 * which doesn't support holding
 		 * multiple logs, remains.
 		 */
-		*cb_data->id = part;
+		*cb_data->id = generic_id(time, part, 0);
 		*cb_data->count = 0;
 		cb_data->timespec->tv_sec = time;
 		cb_data->timespec->tv_nsec = 0;

@@ -199,14 +205,15 @@ static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count,
 	char name[DUMP_NAME_LEN];
 	efi_char16_t efi_name[DUMP_NAME_LEN];
 	int found, i;
+	unsigned int part;
 
-	sprintf(name, "dump-type%u-%u-%d-%lu", type, (unsigned int)id, count,
-		time.tv_sec);
+	part = (id / 1000) % 100;
+	sprintf(name, "dump-type%u-%u-%d-%lu", type, part, count, time.tv_sec);
 
 	for (i = 0; i < DUMP_NAME_LEN; i++)
 		efi_name[i] = name[i];
 
-	edata.id = id;
+	edata.id = part;
 	edata.type = type;
 	edata.count = count;
 	edata.time = time;
Signature · 1 line
1.8.4.2
#1 lore raw
Quoted 9 lines · Madper Xie

Pstore fs expects that backends provide a uniqued id which could avoid pstore making entries as duplication or denominating entries the same name. So I combine the timestamp, part and count into id.

Signed-off-by Madper Xie <[email protected]>
---
 drivers/firmware/efi/efi-pstore.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Thanks unless anyone complains, I'm going to apply this.

Does this need to be tagged for stable?

Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
#2 lore raw
Quoted 15 lines

-----Original Message-----
From: Matt Fleming [mailto:matt-HNK1S37rvNbeXh+fF434Mdi2O/[email protected]]
Sent: Wednesday, November 27, 2013 6:45 AM
To: Madper Xie
Cc: '[email protected]'; [email protected]; [email protected]; Seiji Aguchi; richard-/[email protected]; Tony Luck
Subject: Re: [PATCH v2] efi-pstore: Make efi-pstore return a unique id

Quoted 5 lines · Madper Xie

Pstore fs expects that backends provide a uniqued id which could avoid pstore making entries as duplication or denominating entries the same name. So I combine the timestamp, part and count into id.

Quoted 7 lines
Quoted 4 lines
Signed-off-by Madper Xie <[email protected]>
---
 drivers/firmware/efi/efi-pstore.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Thanks unless anyone complains, I'm going to apply this.

I'm OK with this patch based on Madper's comment below.

<snip>
So i'd like to using fixed length. My dell xps has 128kb nvram space. and it at most store 126 pstore entries. So I think 1000 will be a safe value for count. For part, 100 should be okay. I don't think there will be a large kmsg and we must split it into more than 100 parts. <snip>

Quoted 2 lines

Does this need to be tagged for stable?

I think so.

Seiji

Quoted 2 lines

--
Matt Fleming, Intel Open Source Technology Center

author
#3 lore raw
In reply to Matt Fleming
Quoted 6 lines · matt-HNK1S37rvNbeXh+fF434Mdi2O/[email protected] writes:
Quoted 5 lines · Madper Xie

Pstore fs expects that backends provide a uniqued id which could avoid pstore making entries as duplication or denominating entries the same name. So I combine the timestamp, part and count into id.

Quoted 8 lines
Quoted 4 lines
Signed-off-by Madper Xie <[email protected]>
---
 drivers/firmware/efi/efi-pstore.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Thanks unless anyone complains, I'm going to apply this.

Does this need to be tagged for stable?

Since it fix a real issue, my answer is "Yes".