From 3b38bb5f7f06356a89bb2cbf92ad346025e95192 Mon Sep 17 00:00:00 2001 From: Huang Ying Date: Thu, 2 Dec 2010 10:40:53 +0800 Subject: ACPI, APEI, use raw spinlock in ERST ERST writing may be used in NMI or Machine Check Exception handler. So it need to use raw spinlock instead of normal spinlock. This patch fixes it. Signed-off-by: Huang Ying Signed-off-by: Len Brown --- drivers/acpi/apei/erst.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) (limited to 'drivers/acpi/apei/erst.c') diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index 1211c03149e8..5850d320404c 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -86,7 +86,7 @@ static struct erst_erange { * It is used to provide exclusive accessing for ERST Error Log * Address Range too. */ -static DEFINE_SPINLOCK(erst_lock); +static DEFINE_RAW_SPINLOCK(erst_lock); static inline int erst_errno(int command_status) { @@ -421,9 +421,9 @@ ssize_t erst_get_record_count(void) if (erst_disable) return -ENODEV; - spin_lock_irqsave(&erst_lock, flags); + raw_spin_lock_irqsave(&erst_lock, flags); count = __erst_get_record_count(); - spin_unlock_irqrestore(&erst_lock, flags); + raw_spin_unlock_irqrestore(&erst_lock, flags); return count; } @@ -456,9 +456,9 @@ int erst_get_next_record_id(u64 *record_id) if (erst_disable) return -ENODEV; - spin_lock_irqsave(&erst_lock, flags); + raw_spin_lock_irqsave(&erst_lock, flags); rc = __erst_get_next_record_id(record_id); - spin_unlock_irqrestore(&erst_lock, flags); + raw_spin_unlock_irqrestore(&erst_lock, flags); return rc; } @@ -624,17 +624,17 @@ int erst_write(const struct cper_record_header *record) return -EINVAL; if (erst_erange.attr & ERST_RANGE_NVRAM) { - if (!spin_trylock_irqsave(&erst_lock, flags)) + if (!raw_spin_trylock_irqsave(&erst_lock, flags)) return -EBUSY; rc = __erst_write_to_nvram(record); - spin_unlock_irqrestore(&erst_lock, flags); + raw_spin_unlock_irqrestore(&erst_lock, flags); return rc; } if (record->record_length > erst_erange.size) return -EINVAL; - if (!spin_trylock_irqsave(&erst_lock, flags)) + if (!raw_spin_trylock_irqsave(&erst_lock, flags)) return -EBUSY; memcpy(erst_erange.vaddr, record, record->record_length); rcd_erange = erst_erange.vaddr; @@ -642,7 +642,7 @@ int erst_write(const struct cper_record_header *record) memcpy(&rcd_erange->persistence_information, "ER", 2); rc = __erst_write_to_storage(0); - spin_unlock_irqrestore(&erst_lock, flags); + raw_spin_unlock_irqrestore(&erst_lock, flags); return rc; } @@ -696,9 +696,9 @@ ssize_t erst_read(u64 record_id, struct cper_record_header *record, if (erst_disable) return -ENODEV; - spin_lock_irqsave(&erst_lock, flags); + raw_spin_lock_irqsave(&erst_lock, flags); len = __erst_read(record_id, record, buflen); - spin_unlock_irqrestore(&erst_lock, flags); + raw_spin_unlock_irqrestore(&erst_lock, flags); return len; } EXPORT_SYMBOL_GPL(erst_read); @@ -719,20 +719,20 @@ ssize_t erst_read_next(struct cper_record_header *record, size_t buflen) if (erst_disable) return -ENODEV; - spin_lock_irqsave(&erst_lock, flags); + raw_spin_lock_irqsave(&erst_lock, flags); rc = __erst_get_next_record_id(&record_id); if (rc) { - spin_unlock_irqrestore(&erst_lock, flags); + raw_spin_unlock_irqrestore(&erst_lock, flags); return rc; } /* no more record */ if (record_id == APEI_ERST_INVALID_RECORD_ID) { - spin_unlock_irqrestore(&erst_lock, flags); + raw_spin_unlock_irqrestore(&erst_lock, flags); return 0; } len = __erst_read(record_id, record, buflen); - spin_unlock_irqrestore(&erst_lock, flags); + raw_spin_unlock_irqrestore(&erst_lock, flags); return len; } @@ -746,12 +746,12 @@ int erst_clear(u64 record_id) if (erst_disable) return -ENODEV; - spin_lock_irqsave(&erst_lock, flags); + raw_spin_lock_irqsave(&erst_lock, flags); if (erst_erange.attr & ERST_RANGE_NVRAM) rc = __erst_clear_from_nvram(record_id); else rc = __erst_clear_from_storage(record_id); - spin_unlock_irqrestore(&erst_lock, flags); + raw_spin_unlock_irqrestore(&erst_lock, flags); return rc; } -- cgit v1.2.2 From e8a8b252fb37489f881957ab0f2f8ea9a2341dd1 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Sun, 2 Jan 2011 15:12:42 +0100 Subject: Fix spelling mistakes in comments milisecond -> millisecond meassge -> message Cc: Kalle Valo Cc: Jiri Kosina Cc: linux-kernel@vger.kernel.org Signed-off-by: Stefan Weil Signed-off-by: Jiri Kosina --- drivers/acpi/apei/erst.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/acpi/apei/erst.c') diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index 5850d320404c..cf6db6b7662a 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -53,7 +53,7 @@ sizeof(struct acpi_table_erst))) #define SPIN_UNIT 100 /* 100ns */ -/* Firmware should respond within 1 miliseconds */ +/* Firmware should respond within 1 milliseconds */ #define FIRMWARE_TIMEOUT (1 * NSEC_PER_MSEC) #define FIRMWARE_MAX_STALL 50 /* 50us */ -- cgit v1.2.2 From 0bb77c465f02e8281e24b9f02e7dc8a7e2b81ee2 Mon Sep 17 00:00:00 2001 From: Tony Luck Date: Mon, 3 Jan 2011 14:22:11 -0800 Subject: pstore: X86 platform interface using ACPI/APEI/ERST The 'error record serialization table' in ACPI provides a suitable amount of persistent storage for use by the pstore filesystem. Signed-off-by: Tony Luck --- drivers/acpi/apei/erst.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) (limited to 'drivers/acpi/apei/erst.c') diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index 5850d320404c..22d70dbe75d1 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include "apei-internal.h" @@ -781,6 +782,128 @@ static int erst_check_table(struct acpi_table_erst *erst_tab) return 0; } +static size_t erst_reader(u64 *id, enum pstore_type_id *type, + struct timespec *time); +static u64 erst_writer(enum pstore_type_id type, size_t size); + +static struct pstore_info erst_info = { + .owner = THIS_MODULE, + .name = "erst", + .read = erst_reader, + .write = erst_writer, + .erase = erst_clear +}; + +#define CPER_CREATOR_PSTORE \ + UUID_LE(0x75a574e3, 0x5052, 0x4b29, 0x8a, 0x8e, 0xbe, 0x2c, \ + 0x64, 0x90, 0xb8, 0x9d) +#define CPER_SECTION_TYPE_DMESG \ + UUID_LE(0xc197e04e, 0xd545, 0x4a70, 0x9c, 0x17, 0xa5, 0x54, \ + 0x94, 0x19, 0xeb, 0x12) +#define CPER_SECTION_TYPE_MCE \ + UUID_LE(0xfe08ffbe, 0x95e4, 0x4be7, 0xbc, 0x73, 0x40, 0x96, \ + 0x04, 0x4a, 0x38, 0xfc) + +struct cper_pstore_record { + struct cper_record_header hdr; + struct cper_section_descriptor sec_hdr; + char data[]; +} __packed; + +static size_t erst_reader(u64 *id, enum pstore_type_id *type, + struct timespec *time) +{ + int rc; + ssize_t len; + unsigned long flags; + u64 record_id; + struct cper_pstore_record *rcd = (struct cper_pstore_record *) + (erst_info.buf - sizeof(*rcd)); + + if (erst_disable) + return -ENODEV; + + raw_spin_lock_irqsave(&erst_lock, flags); +skip: + rc = __erst_get_next_record_id(&record_id); + if (rc) { + raw_spin_unlock_irqrestore(&erst_lock, flags); + return rc; + } + /* no more record */ + if (record_id == APEI_ERST_INVALID_RECORD_ID) { + raw_spin_unlock_irqrestore(&erst_lock, flags); + return 0; + } + + len = __erst_read(record_id, &rcd->hdr, sizeof(*rcd) + + erst_erange.size); + if (uuid_le_cmp(rcd->hdr.creator_id, CPER_CREATOR_PSTORE) != 0) + goto skip; + raw_spin_unlock_irqrestore(&erst_lock, flags); + + *id = record_id; + if (uuid_le_cmp(rcd->sec_hdr.section_type, + CPER_SECTION_TYPE_DMESG) == 0) + *type = PSTORE_TYPE_DMESG; + else if (uuid_le_cmp(rcd->sec_hdr.section_type, + CPER_SECTION_TYPE_MCE) == 0) + *type = PSTORE_TYPE_MCE; + else + *type = PSTORE_TYPE_UNKNOWN; + + if (rcd->hdr.validation_bits & CPER_VALID_TIMESTAMP) + time->tv_sec = rcd->hdr.timestamp; + else + time->tv_sec = 0; + time->tv_nsec = 0; + + return len - sizeof(*rcd); +} + +static u64 erst_writer(enum pstore_type_id type, size_t size) +{ + struct cper_pstore_record *rcd = (struct cper_pstore_record *) + (erst_info.buf - sizeof(*rcd)); + + memset(rcd, 0, sizeof(*rcd)); + memcpy(rcd->hdr.signature, CPER_SIG_RECORD, CPER_SIG_SIZE); + rcd->hdr.revision = CPER_RECORD_REV; + rcd->hdr.signature_end = CPER_SIG_END; + rcd->hdr.section_count = 1; + rcd->hdr.error_severity = CPER_SEV_FATAL; + /* timestamp valid. platform_id, partition_id are invalid */ + rcd->hdr.validation_bits = CPER_VALID_TIMESTAMP; + rcd->hdr.timestamp = get_seconds(); + rcd->hdr.record_length = sizeof(*rcd) + size; + rcd->hdr.creator_id = CPER_CREATOR_PSTORE; + rcd->hdr.notification_type = CPER_NOTIFY_MCE; + rcd->hdr.record_id = cper_next_record_id(); + rcd->hdr.flags = CPER_HW_ERROR_FLAGS_PREVERR; + + rcd->sec_hdr.section_offset = sizeof(*rcd); + rcd->sec_hdr.section_length = size; + rcd->sec_hdr.revision = CPER_SEC_REV; + /* fru_id and fru_text is invalid */ + rcd->sec_hdr.validation_bits = 0; + rcd->sec_hdr.flags = CPER_SEC_PRIMARY; + switch (type) { + case PSTORE_TYPE_DMESG: + rcd->sec_hdr.section_type = CPER_SECTION_TYPE_DMESG; + break; + case PSTORE_TYPE_MCE: + rcd->sec_hdr.section_type = CPER_SECTION_TYPE_MCE; + break; + default: + return -EINVAL; + } + rcd->sec_hdr.section_severity = CPER_SEV_FATAL; + + erst_write(&rcd->hdr); + + return rcd->hdr.record_id; +} + static int __init erst_init(void) { int rc = 0; @@ -788,6 +911,7 @@ static int __init erst_init(void) struct apei_exec_context ctx; struct apei_resources erst_resources; struct resource *r; + char *buf; if (acpi_disabled) goto err; @@ -854,6 +978,18 @@ static int __init erst_init(void) if (!erst_erange.vaddr) goto err_release_erange; + buf = kmalloc(erst_erange.size, GFP_KERNEL); + mutex_init(&erst_info.buf_mutex); + if (buf) { + erst_info.buf = buf + sizeof(struct cper_pstore_record); + erst_info.bufsize = erst_erange.size - + sizeof(struct cper_pstore_record); + if (pstore_register(&erst_info)) { + pr_info(ERST_PFX "Could not register with persistent store\n"); + kfree(buf); + } + } + pr_info(ERST_PFX "Error Record Serialization Table (ERST) support is initialized.\n"); -- cgit v1.2.2 From 885b976fada5bc6595a9fd3e67e3cb1a3d11f50b Mon Sep 17 00:00:00 2001 From: Huang Ying Date: Mon, 21 Feb 2011 13:54:41 +0800 Subject: ACPI, APEI, Add ERST record ID cache APEI ERST firmware interface and implementation has no multiple users in mind. For example, if there is four records in storage with ID: 1, 2, 3 and 4, if two ERST readers enumerate the records via GET_NEXT_RECORD_ID as follow, reader 1 reader 2 1 2 3 4 -1 -1 where -1 signals there is no more record ID. Reader 1 has no chance to check record 2 and 4, while reader 2 has no chance to check record 1 and 3. And any other GET_NEXT_RECORD_ID will return -1, that is, other readers will has no chance to check any record even they are not cleared by anyone. This makes raw GET_NEXT_RECORD_ID not suitable for used by multiple users. To solve the issue, an in-memory ERST record ID cache is designed and implemented. When enumerating record ID, the ID returned by GET_NEXT_RECORD_ID is added into cache in addition to be returned to caller. So other readers can check the cache to get all record ID available. Signed-off-by: Huang Ying Reviewed-by: Andi Kleen Signed-off-by: Len Brown --- drivers/acpi/apei/erst.c | 235 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 191 insertions(+), 44 deletions(-) (limited to 'drivers/acpi/apei/erst.c') diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index cf6db6b7662a..8ff8c32fef58 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -429,6 +429,22 @@ ssize_t erst_get_record_count(void) } EXPORT_SYMBOL_GPL(erst_get_record_count); +#define ERST_RECORD_ID_CACHE_SIZE_MIN 16 +#define ERST_RECORD_ID_CACHE_SIZE_MAX 1024 + +struct erst_record_id_cache { + struct mutex lock; + u64 *entries; + int len; + int size; + int refcount; +}; + +static struct erst_record_id_cache erst_record_id_cache = { + .lock = __MUTEX_INITIALIZER(erst_record_id_cache.lock), + .refcount = 0, +}; + static int __erst_get_next_record_id(u64 *record_id) { struct apei_exec_context ctx; @@ -443,26 +459,179 @@ static int __erst_get_next_record_id(u64 *record_id) return 0; } +int erst_get_record_id_begin(int *pos) +{ + int rc; + + if (erst_disable) + return -ENODEV; + + rc = mutex_lock_interruptible(&erst_record_id_cache.lock); + if (rc) + return rc; + erst_record_id_cache.refcount++; + mutex_unlock(&erst_record_id_cache.lock); + + *pos = 0; + + return 0; +} +EXPORT_SYMBOL_GPL(erst_get_record_id_begin); + +/* erst_record_id_cache.lock must be held by caller */ +static int __erst_record_id_cache_add_one(void) +{ + u64 id, prev_id, first_id; + int i, rc; + u64 *entries; + unsigned long flags; + + id = prev_id = first_id = APEI_ERST_INVALID_RECORD_ID; +retry: + raw_spin_lock_irqsave(&erst_lock, flags); + rc = __erst_get_next_record_id(&id); + raw_spin_unlock_irqrestore(&erst_lock, flags); + if (rc == -ENOENT) + return 0; + if (rc) + return rc; + if (id == APEI_ERST_INVALID_RECORD_ID) + return 0; + /* can not skip current ID, or loop back to first ID */ + if (id == prev_id || id == first_id) + return 0; + if (first_id == APEI_ERST_INVALID_RECORD_ID) + first_id = id; + prev_id = id; + + entries = erst_record_id_cache.entries; + for (i = 0; i < erst_record_id_cache.len; i++) { + if (entries[i] == id) + break; + } + /* record id already in cache, try next */ + if (i < erst_record_id_cache.len) + goto retry; + if (erst_record_id_cache.len >= erst_record_id_cache.size) { + int new_size, alloc_size; + u64 *new_entries; + + new_size = erst_record_id_cache.size * 2; + new_size = clamp_val(new_size, ERST_RECORD_ID_CACHE_SIZE_MIN, + ERST_RECORD_ID_CACHE_SIZE_MAX); + if (new_size <= erst_record_id_cache.size) { + if (printk_ratelimit()) + pr_warning(FW_WARN ERST_PFX + "too many record ID!\n"); + return 0; + } + alloc_size = new_size * sizeof(entries[0]); + if (alloc_size < PAGE_SIZE) + new_entries = kmalloc(alloc_size, GFP_KERNEL); + else + new_entries = vmalloc(alloc_size); + if (!new_entries) + return -ENOMEM; + memcpy(new_entries, entries, + erst_record_id_cache.len * sizeof(entries[0])); + if (erst_record_id_cache.size < PAGE_SIZE) + kfree(entries); + else + vfree(entries); + erst_record_id_cache.entries = entries = new_entries; + erst_record_id_cache.size = new_size; + } + entries[i] = id; + erst_record_id_cache.len++; + + return 1; +} + /* * Get the record ID of an existing error record on the persistent * storage. If there is no error record on the persistent storage, the * returned record_id is APEI_ERST_INVALID_RECORD_ID. */ -int erst_get_next_record_id(u64 *record_id) +int erst_get_record_id_next(int *pos, u64 *record_id) { - int rc; - unsigned long flags; + int rc = 0; + u64 *entries; if (erst_disable) return -ENODEV; - raw_spin_lock_irqsave(&erst_lock, flags); - rc = __erst_get_next_record_id(record_id); - raw_spin_unlock_irqrestore(&erst_lock, flags); + /* must be enclosed by erst_get_record_id_begin/end */ + BUG_ON(!erst_record_id_cache.refcount); + BUG_ON(*pos < 0 || *pos > erst_record_id_cache.len); + + mutex_lock(&erst_record_id_cache.lock); + entries = erst_record_id_cache.entries; + for (; *pos < erst_record_id_cache.len; (*pos)++) + if (entries[*pos] != APEI_ERST_INVALID_RECORD_ID) + break; + /* found next record id in cache */ + if (*pos < erst_record_id_cache.len) { + *record_id = entries[*pos]; + (*pos)++; + goto out_unlock; + } + + /* Try to add one more record ID to cache */ + rc = __erst_record_id_cache_add_one(); + if (rc < 0) + goto out_unlock; + /* successfully add one new ID */ + if (rc == 1) { + *record_id = erst_record_id_cache.entries[*pos]; + (*pos)++; + rc = 0; + } else { + *pos = -1; + *record_id = APEI_ERST_INVALID_RECORD_ID; + } +out_unlock: + mutex_unlock(&erst_record_id_cache.lock); return rc; } -EXPORT_SYMBOL_GPL(erst_get_next_record_id); +EXPORT_SYMBOL_GPL(erst_get_record_id_next); + +/* erst_record_id_cache.lock must be held by caller */ +static void __erst_record_id_cache_compact(void) +{ + int i, wpos = 0; + u64 *entries; + + if (erst_record_id_cache.refcount) + return; + + entries = erst_record_id_cache.entries; + for (i = 0; i < erst_record_id_cache.len; i++) { + if (entries[i] == APEI_ERST_INVALID_RECORD_ID) + continue; + if (wpos != i) + memcpy(&entries[wpos], &entries[i], sizeof(entries[i])); + wpos++; + } + erst_record_id_cache.len = wpos; +} + +void erst_get_record_id_end(void) +{ + /* + * erst_disable != 0 should be detected by invoker via the + * return value of erst_get_record_id_begin/next, so this + * function should not be called for erst_disable != 0. + */ + BUG_ON(erst_disable); + + mutex_lock(&erst_record_id_cache.lock); + erst_record_id_cache.refcount--; + BUG_ON(erst_record_id_cache.refcount < 0); + __erst_record_id_cache_compact(); + mutex_unlock(&erst_record_id_cache.lock); +} +EXPORT_SYMBOL_GPL(erst_get_record_id_end); static int __erst_write_to_storage(u64 offset) { @@ -703,56 +872,34 @@ ssize_t erst_read(u64 record_id, struct cper_record_header *record, } EXPORT_SYMBOL_GPL(erst_read); -/* - * If return value > buflen, the buffer size is not big enough, - * else if return value = 0, there is no more record to read, - * else if return value < 0, something goes wrong, - * else everything is OK, and return value is record length - */ -ssize_t erst_read_next(struct cper_record_header *record, size_t buflen) -{ - int rc; - ssize_t len; - unsigned long flags; - u64 record_id; - - if (erst_disable) - return -ENODEV; - - raw_spin_lock_irqsave(&erst_lock, flags); - rc = __erst_get_next_record_id(&record_id); - if (rc) { - raw_spin_unlock_irqrestore(&erst_lock, flags); - return rc; - } - /* no more record */ - if (record_id == APEI_ERST_INVALID_RECORD_ID) { - raw_spin_unlock_irqrestore(&erst_lock, flags); - return 0; - } - - len = __erst_read(record_id, record, buflen); - raw_spin_unlock_irqrestore(&erst_lock, flags); - - return len; -} -EXPORT_SYMBOL_GPL(erst_read_next); - int erst_clear(u64 record_id) { - int rc; + int rc, i; unsigned long flags; + u64 *entries; if (erst_disable) return -ENODEV; + rc = mutex_lock_interruptible(&erst_record_id_cache.lock); + if (rc) + return rc; raw_spin_lock_irqsave(&erst_lock, flags); if (erst_erange.attr & ERST_RANGE_NVRAM) rc = __erst_clear_from_nvram(record_id); else rc = __erst_clear_from_storage(record_id); raw_spin_unlock_irqrestore(&erst_lock, flags); - + if (rc) + goto out; + entries = erst_record_id_cache.entries; + for (i = 0; i < erst_record_id_cache.len; i++) { + if (entries[i] == record_id) + entries[i] = APEI_ERST_INVALID_RECORD_ID; + } + __erst_record_id_cache_compact(); +out: + mutex_unlock(&erst_record_id_cache.lock); return rc; } EXPORT_SYMBOL_GPL(erst_clear); -- cgit v1.2.2 From 8d38d74b648513dd8ed8bd2b67d899208ef4e09e Mon Sep 17 00:00:00 2001 From: Chen Gong Date: Mon, 16 May 2011 10:58:57 -0700 Subject: pstore: fix one type of return value in pstore the return type of function _read_ in pstore is size_t, but in the callback function of _read_, the logic doesn't consider it too much, which means if negative value (assuming error here) is returned, it will be converted to positive because of type casting. ssize_t is enough for this function. Signed-off-by: Chen Gong Signed-off-by: Tony Luck --- drivers/acpi/apei/erst.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/acpi/apei/erst.c') diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index d6cb0ff6988e..d5a89d067f98 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -929,7 +929,7 @@ static int erst_check_table(struct acpi_table_erst *erst_tab) return 0; } -static size_t erst_reader(u64 *id, enum pstore_type_id *type, +static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, struct timespec *time); static u64 erst_writer(enum pstore_type_id type, size_t size); @@ -957,7 +957,7 @@ struct cper_pstore_record { char data[]; } __packed; -static size_t erst_reader(u64 *id, enum pstore_type_id *type, +static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, struct timespec *time) { int rc; -- cgit v1.2.2 From 06cf91b4b4aafa50ee0a94c81d2c6922a18af242 Mon Sep 17 00:00:00 2001 From: Chen Gong Date: Mon, 16 May 2011 11:00:27 -0700 Subject: pstore: fix pstore filesystem mount/remount issue Currently after mount/remount operation on pstore filesystem, the content on pstore will be lost. It is because current ERST implementation doesn't support multi-user usage, which moves internal pointer to the end after accessing it. Adding multi-user support for pstore usage. Signed-off-by: Chen Gong Signed-off-by: Tony Luck --- drivers/acpi/apei/erst.c | 48 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 13 deletions(-) (limited to 'drivers/acpi/apei/erst.c') diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index d5a89d067f98..ddb68c4f8d3e 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -929,6 +929,8 @@ static int erst_check_table(struct acpi_table_erst *erst_tab) return 0; } +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, struct timespec *time); static u64 erst_writer(enum pstore_type_id type, size_t size); @@ -936,6 +938,8 @@ static u64 erst_writer(enum pstore_type_id type, size_t size); static struct pstore_info erst_info = { .owner = THIS_MODULE, .name = "erst", + .open = erst_open_pstore, + .close = erst_close_pstore, .read = erst_reader, .write = erst_writer, .erase = erst_clear @@ -957,12 +961,32 @@ struct cper_pstore_record { char data[]; } __packed; +static int reader_pos; + +static int erst_open_pstore(struct pstore_info *psi) +{ + int rc; + + if (erst_disable) + return -ENODEV; + + rc = erst_get_record_id_begin(&reader_pos); + + return rc; +} + +static int erst_close_pstore(struct pstore_info *psi) +{ + erst_get_record_id_end(); + + return 0; +} + static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, struct timespec *time) { int rc; - ssize_t len; - unsigned long flags; + ssize_t len = 0; u64 record_id; struct cper_pstore_record *rcd = (struct cper_pstore_record *) (erst_info.buf - sizeof(*rcd)); @@ -970,24 +994,21 @@ static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, if (erst_disable) return -ENODEV; - raw_spin_lock_irqsave(&erst_lock, flags); skip: - rc = __erst_get_next_record_id(&record_id); - if (rc) { - raw_spin_unlock_irqrestore(&erst_lock, flags); - return rc; - } + rc = erst_get_record_id_next(&reader_pos, &record_id); + if (rc) + goto out; + /* no more record */ if (record_id == APEI_ERST_INVALID_RECORD_ID) { - raw_spin_unlock_irqrestore(&erst_lock, flags); - return 0; + rc = -1; + goto out; } - len = __erst_read(record_id, &rcd->hdr, sizeof(*rcd) + + len = erst_read(record_id, &rcd->hdr, sizeof(*rcd) + erst_erange.size); if (uuid_le_cmp(rcd->hdr.creator_id, CPER_CREATOR_PSTORE) != 0) goto skip; - raw_spin_unlock_irqrestore(&erst_lock, flags); *id = record_id; if (uuid_le_cmp(rcd->sec_hdr.section_type, @@ -1005,7 +1026,8 @@ skip: time->tv_sec = 0; time->tv_nsec = 0; - return len - sizeof(*rcd); +out: + return (rc < 0) ? rc : (len - sizeof(*rcd)); } static u64 erst_writer(enum pstore_type_id type, size_t size) -- cgit v1.2.2 From f5ec25deb2471bd49e907ab2f9ef6f860eb7cf95 Mon Sep 17 00:00:00 2001 From: Chen Gong Date: Mon, 16 May 2011 11:01:39 -0700 Subject: pstore: fix potential logic issue in pstore read interface 1) in the calling of erst_read, the parameter of buffer size maybe overflows and cause crash 2) the return value of erst_read should be checked more strictly Signed-off-by: Chen Gong Signed-off-by: Tony Luck --- drivers/acpi/apei/erst.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'drivers/acpi/apei/erst.c') diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index ddb68c4f8d3e..e6cef8e1b534 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -1006,7 +1006,14 @@ skip: } len = erst_read(record_id, &rcd->hdr, sizeof(*rcd) + - erst_erange.size); + erst_info.bufsize); + /* The record may be cleared by others, try read next record */ + if (len == -ENOENT) + goto skip; + else if (len < 0) { + rc = -1; + goto out; + } if (uuid_le_cmp(rcd->hdr.creator_id, CPER_CREATOR_PSTORE) != 0) goto skip; -- cgit v1.2.2