hi Kees Cook, On 2020/5/12 PM 1:15, Kees Cook wrote: > On Tue, May 12, 2020 at 11:55:20AM +0800, WeiXiong Liao wrote: >> On 2020/5/12 AM 7:32, Kees Cook wrote: >>> [...] >>> +struct psz_context { >>> + struct pstore_zone **kpszs; >>> + unsigned int kmsg_max_cnt; >>> + unsigned int kmsg_read_cnt; >>> + unsigned int kmsg_write_cnt; >>> + /* >>> + * These counters should be calculated during recovery. >>> + * It records the oops/panic times after crashes rather than boots. >>> + */ >>> + unsigned int oops_counter; >>> + unsigned int panic_counter; >> >> oops/panic_counter is designed to count the crash times since the >> linux kernel was installed. pstore/zone lookup the max counter from all >> valid kmsg zones when recovery and saves them to oops/panic_counter. >> However, they are unable to get real number if we remove files. It's >> not serious, we can fix it after this series. > > Since the kernel was installed? I don't see a kernel version check in > here? Or do you mean "since ever", in that it's a rolling count? > Yes, "since ever". >> And since pstore supports "max_reason", should pstore/zone count for >> other reason? > > For now, no. I opted to try to keep this as simple as possible a port > from dump_oops to max_reason for now. > OK. >>> +static inline int psz_kmsg_erase(struct psz_context *cxt, >>> + struct pstore_zone *zone, struct pstore_record *record) >>> +{ >>> + struct psz_buffer *buffer = zone->buffer; >>> + struct psz_kmsg_header *hdr = >>> + (struct psz_kmsg_header *)buffer->data; >>> + >>> + if (unlikely(!psz_ok(zone))) >>> + return 0; >>> + /* this zone is already updated, no need to erase */ >>> + if (record->count != hdr->counter) >>> + return 0; >> >> These codes is to fix bug that user remove files on pstore filesystem >> but kmsg zone is already updated and pstore/zone should not erase >> zone. It work for oops and panic because the count number is increasing. >> However, it's useless for other reason of kmsg. We can fix it after this >> series. > > Okay, sounds good. > -- WeiXiong Liao