From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751953AbdHGFUL (ORCPT ); Mon, 7 Aug 2017 01:20:11 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:56546 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751424AbdHGFUK (ORCPT ); Mon, 7 Aug 2017 01:20:10 -0400 Subject: Re: [PATCH 2/2] Save current timestamp part of dmesg while writing oops message to pstore From: Ankit Kumar To: Kees Cook Cc: Anton Vorontsov , Colin Cross , Tony Luck , LKML , "mahesh@linux.vnet.ibm.com" , Hari Bathini , hegdevasant@linux.vnet.ibm.com References: <1495448437-15398-1-git-send-email-ankit@linux.vnet.ibm.com> <1495448437-15398-2-git-send-email-ankit@linux.vnet.ibm.com> Date: Mon, 7 Aug 2017 10:49:56 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-TM-AS-MML: disable x-cbid: 17080705-0012-0000-0000-00000258A8C6 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17080705-0013-0000-0000-000007738DD7 Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-08-07_04:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1706020000 definitions=main-1708070093 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Kees, On Tuesday 23 May 2017 02:19 PM, Ankit Kumar wrote: > Hi Kees, > > > > On Tuesday 23 May 2017 05:21 AM, Kees Cook wrote: >> On Mon, May 22, 2017 at 3:20 AM, Ankit Kumar >> wrote: >>> Currently on panic or Oops, kernel saves the last few bytes from dmesg >>> buffer to nvram. Usually kdump does capture kernel memory and provide >>> dmesg logs as well. But in some cases where kdump fails to capture >>> vmcore, the dmesg buffer stored in nvram/pstore turns out to be very >>> helpful in analyzing root cause. >>> >>> Present code creates pstore dump file(/sys/fs/pstore/dmesg-***) >>> based on >>> timestamp(retrieved from header). Current pstore code creates dump file >>> (/sys/fs/pstore/dmesg-***) with that timestamp. Dump file can be >>> analyzed >>> based on file creation time and we can make out whether dump file >>> has latest >>> data or not. >>> >>> But when we transfer pstore dump file(/sys/fs/pstore/dmesg-***) to >>> other >>> machine or collect file using some >>> utilities(sosreport/supportconfig) then file >>> timestamp gets changed and hence by looking at device file >>> (dmesg-***) we won't >>> be able to identify whether dump has latest data or not. >>> >>> Above issue can be fixed if we also have timestamp(dump creation >>> time) as >>> initial few bytes while capturing dmesg buffer to pstore dump file >>> (/sys/fs/pstore/dmesg-***). >>> >>> >>> This patch enhances pstore write code to also write timestamp as >>> part of data. >>> >>> Here is sample log of dump file:(/sys/fs/pstore/dmesg-***) >>> Oops#1 Part1 [timestamp:1494939359.590463] >> While I understand your rationale about possibly losing file timestamp >> information in userspace, I think this is a solvable problem on the >> collection side. If an additional header is needed, perhaps copy the >> dmesg files like this: >> >> for i in dmesg-*; do >> (stat --format=%y /sys/fs/pstore/$i; \ >> cat /sys/fs/pstore/$i) > $collect_dir/$i >> done > > Yes. We can handle this in userspace. But we wanted to see if we can > add this as part of pstore > log itself. > > >> One of the primary concerns for pstore is the stored dump size, > > I understand. How about adding timestamp to file name itself? > Something like below How about appending time as part of file name itself. ? Did you get time to look at above approach. Code can be something like below piece. ~Ankit > > index 792a4e5..0837365 100644 > --- a/fs/pstore/inode.c > +++ b/fs/pstore/inode.c > @@ -349,9 +349,10 @@ int pstore_mkfile(struct dentry *root, struct > pstore_record *record) > > switch (record->type) { > case PSTORE_TYPE_DMESG: > - scnprintf(name, sizeof(name), "dmesg-%s-%lld%s", > + scnprintf(name, sizeof(name), "dmesg-%s-%lld%s-%lu.%lu", > record->psi->name, record->id, > - record->compressed ? ".enc.z" : ""); > + record->compressed ? ".enc.z" : "", > + record->time.tv_sec, record->time.tv_nsec / > 1000); > break; > case PSTORE_TYPE_CONSOLE: > > > ~Ankit