From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935759AbdEWINy (ORCPT ); Tue, 23 May 2017 04:13:54 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:54932 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933221AbdEWINt (ORCPT ); Tue, 23 May 2017 04:13:49 -0400 Subject: Re: [PATCH 1/2]pstore: Move timestamp collection code to common pstore place 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> From: Ankit Kumar Date: Tue, 23 May 2017 13:42:51 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.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: 17052308-0016-0000-0000-0000023F826C X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17052308-0017-0000-0000-000006BCEA0F Message-Id: <27f53f80-769c-9e22-7a01-441967805248@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-05-23_03:,, 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-1703280000 definitions=main-1705230042 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Kees, Thank you so much for your response. On Tuesday 23 May 2017 05:07 AM, Kees Cook wrote: > On Mon, May 22, 2017 at 3:20 AM, Ankit Kumar wrote: >> Current pstore code(ram.c) gets dump timestamp and make it part of header. >> Different diffent architecture uses different header format and hence it >> won't be good idea to make ramoops_write_kmsg_hdr/ramoops_read_kmsg_hdr >> part of generic pstore code. >> >> ramoops_write_kmsg_hdr calls __getnstimeofday to get timestamp and also >> takes care of condition if timekeeping has not resumed. >> >> This patch moves code for retrieving timestamp to common place and hence >> can be used by other functions as well. > Ah! Heh, funny, I had just done a version of this while working on EFI > pstore fixes. Here's what I did instead: > > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=pstore/fixes&id=ea209b760f104daf0cf62953090b66c5628b0842 > > This makes it so the time field is prepopulated for all backend > writes, which should simplify things (no need to have the backends > call out). Above mentioned patch moves code to common place and also simplifies things. I agree, My patch is not required anymore. Thank you; ~Ankit > -Kees > >> Signed-off-by: Ankit Kumar >> --- >> fs/pstore/internal.h | 1 + >> fs/pstore/platform.c | 13 +++++++++++++ >> fs/pstore/ram.c | 9 ++++----- >> 3 files changed, 18 insertions(+), 5 deletions(-) >> >> diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h >> index c416e65..339dcdc 100644 >> --- a/fs/pstore/internal.h >> +++ b/fs/pstore/internal.h >> @@ -22,6 +22,7 @@ static inline void pstore_unregister_pmsg(void) {} >> #endif >> >> extern struct pstore_info *psinfo; >> +void pstore_get_timestamp(struct timespec *timestamp); >> >> extern void pstore_set_kmsg_bytes(int); >> extern void pstore_get_records(int); >> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c >> index d468eec..5cb25b0 100644 >> --- a/fs/pstore/platform.c >> +++ b/fs/pstore/platform.c >> @@ -474,6 +474,19 @@ static size_t copy_kmsg_to_buffer(int hsize, size_t len) >> return total_len; >> } >> >> +void pstore_get_timestamp(struct timespec *timestamp) >> +{ >> + if (!timestamp) >> + return; >> + >> + /* Report zeroed timestamp if called before timekeeping has resumed. */ >> + if (__getnstimeofday(timestamp)) { >> + timestamp->tv_sec = 0; >> + timestamp->tv_nsec = 0; >> + } >> +} >> +EXPORT_SYMBOL_GPL(pstore_get_timestamp); >> + >> /* >> * callback from kmsg_dump. (s2,l2) has the most recently >> * written bytes, older bytes are in (s1,l1). Save as much >> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c >> index 5cb022c..e39297d 100644 >> --- a/fs/pstore/ram.c >> +++ b/fs/pstore/ram.c >> @@ -37,6 +37,8 @@ >> #include >> #include >> >> +#include "internal.h" >> + >> #define RAMOOPS_KERNMSG_HDR "====" >> #define MIN_MEM_SIZE 4096UL >> >> @@ -362,11 +364,8 @@ static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz, >> struct timespec timestamp; >> size_t len; >> >> - /* Report zeroed timestamp if called before timekeeping has resumed. */ >> - if (__getnstimeofday(×tamp)) { >> - timestamp.tv_sec = 0; >> - timestamp.tv_nsec = 0; >> - } >> + pstore_get_timestamp(×tamp); >> + >> hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR "%lu.%lu-%c\n", >> (long)timestamp.tv_sec, (long)(timestamp.tv_nsec / 1000), >> compressed ? 'C' : 'D'); >> -- >> 2.7.4 >> > >