[OPW,kernel,v2,3/4] fs: pstore: Add timespec to timespec64 conversion
diff mbox

Message ID 1414758146-11906-4-git-send-email-somyaanand214@gmail.com
State New, archived
Headers show

Commit Message

Somya Anand Oct. 31, 2014, 12:22 p.m. UTC
32-bit systems using 'struct timespec' will break in the year 2038,
so we have to replace that code with more appropriate types.

Introducing a local variable of type 'struct timespec64' to update
interface of pstore_info.read and pstore_info.erase in order to support
new pstore_info API.

pstore_mkfile(..) is modified to use 'struct timespec64' instead of
'struct timespec'.

__getnstimeofday64() is used instead of __getnstimeofday() as it uses
'struct timespec64'.

Signed-off-by: Somya Anand <somyaanand214@gmail.com>
---
Changes since version 1:
  * Use timespec_to_timespec64 conversion.
  * Use __getnstimeofday64() instead of getnstimeofday64().
  * Reword commit message.
  
 fs/pstore/inode.c    |  2 +-
 fs/pstore/internal.h |  2 +-
 fs/pstore/platform.c |  5 +++--
 fs/pstore/ram.c      | 11 ++++++-----
 4 files changed, 11 insertions(+), 9 deletions(-)

Comments

Arnd Bergmann Oct. 31, 2014, 4:15 p.m. UTC | #1
On Friday 31 October 2014 17:52:25 Somya Anand wrote:
> 32-bit systems using 'struct timespec' will break in the year 2038,
> so we have to replace that code with more appropriate types.
> 
> Introducing a local variable of type 'struct timespec64' to update
> interface of pstore_info.read and pstore_info.erase in order to support
> new pstore_info API.
> 
> pstore_mkfile(..) is modified to use 'struct timespec64' instead of
> 'struct timespec'.
> 
> __getnstimeofday64() is used instead of __getnstimeofday() as it uses
> 'struct timespec64'.
> 
> Signed-off-by: Somya Anand <somyaanand214@gmail.com>

Also much better than v1, but still a couple of bugs:

> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> index fafb7a0..ce1a093 100644
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -277,7 +277,7 @@ int pstore_is_mounted(void)
>   */
>  int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count,
>  		  char *data, bool compressed, size_t size,
> -		  struct timespec time, struct pstore_info *psi)
> +		  struct timespec64 time, struct pstore_info *psi)
>  {
>  	struct dentry		*root = pstore_sb->s_root;
>  	struct dentry		*dentry;

Here you will get a compiler warning for 32-bit systems. I think you should
to build everything twice, once with CONFIG_64BIT enabled and once
disabled, to catch this kind of problem.

> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 0a9b72c..4a96a1c 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -469,7 +469,7 @@ EXPORT_SYMBOL_GPL(pstore_register);
>   */
>  void pstore_get_records(int quiet)
>  {
> -	struct pstore_info *psi = psinfo;
> +	struct pstore_info	*psi = psinfo;
>  	char			*buf = NULL;
>  	ssize_t			size;
>  	u64			id;

This looks like an unrelated change. While the change is good in
principle, it doesn't belong into this patch.

> @@ -507,7 +507,8 @@ void pstore_get_records(int quiet)
>  			}
>  		}
>  		rc = pstore_mkfile(type, psi->name, id, count, buf,
> -				  compressed, (size_t)size, time, psi);
> +				  compressed, (size_t)size,
> +				  timespec_to_timespec64(time), psi);
>  		if (unzipped_len < 0) {
>  			/* Free buffer other than big oops */
>  			kfree(buf);

Not wrong, but I think it would make the follow-on patch that
changes the API simpler if you changed the value of 'time' as
well here and did the conversion right after calling psi->read().

> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 3b57443..55a16ef 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -135,7 +135,7 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint *c, uint max,
>  	return prz;
>  }
>  
> -static void ramoops_read_kmsg_hdr(char *buffer, struct timespec *time,
> +static void ramoops_read_kmsg_hdr(char *buffer, struct timespec64 *time,
>  				  bool *compressed)
>  {
>  	char data_type;

Another valid warning you missed.

Also, ram.c is one of the front-end drivers, you should patch that
separately from the core.

> @@ -157,10 +157,12 @@ static void ramoops_read_kmsg_hdr(char *buffer, struct timespec *time,
>  }
>  
>  static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
> -				   int *count, struct timespec *time,
> +				   int *count, struct timespec *ts,
>  				   char **buf, bool *compressed,
>  				   struct pstore_info *psi)
>  {
> +	struct timespec64 ts64 = timespec_to_timespec64(*ts);
> +	struct timespec64 *time = &ts64;
>  	ssize_t size;
>  	ssize_t ecc_notice_size;

Same bug that I commented on for patch 2.

>  	struct ramoops_context *cxt = psi->data;
> @@ -198,11 +200,10 @@ static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz,
>  				     bool compressed)
>  {
>  	char *hdr;
> -	struct timespec timestamp;
> +	struct timespec64 timestamp;
>  	size_t len;
> -
>  	/* Report zeroed timestamp if called before timekeeping has resumed. */
> -	if (__getnstimeofday(&timestamp)) {
> +	if (__getnstimeofday64(&timestamp)) {
>  		timestamp.tv_sec = 0;
>  		timestamp.tv_nsec = 0;
>  	}
> 

and another missed warning.

	Arnd

Patch
diff mbox

diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index fafb7a0..ce1a093 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -277,7 +277,7 @@  int pstore_is_mounted(void)
  */
 int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count,
 		  char *data, bool compressed, size_t size,
-		  struct timespec time, struct pstore_info *psi)
+		  struct timespec64 time, struct pstore_info *psi)
 {
 	struct dentry		*root = pstore_sb->s_root;
 	struct dentry		*dentry;
diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h
index 3b3d305..29778d1 100644
--- a/fs/pstore/internal.h
+++ b/fs/pstore/internal.h
@@ -51,7 +51,7 @@  extern void	pstore_set_kmsg_bytes(int);
 extern void	pstore_get_records(int);
 extern int	pstore_mkfile(enum pstore_type_id, char *psname, u64 id,
 			      int count, char *data, bool compressed,
-			      size_t size, struct timespec time,
+			      size_t size, struct timespec64 time,
 			      struct pstore_info *psi);
 extern int	pstore_is_mounted(void);
 
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 0a9b72c..4a96a1c 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -469,7 +469,7 @@  EXPORT_SYMBOL_GPL(pstore_register);
  */
 void pstore_get_records(int quiet)
 {
-	struct pstore_info *psi = psinfo;
+	struct pstore_info	*psi = psinfo;
 	char			*buf = NULL;
 	ssize_t			size;
 	u64			id;
@@ -507,7 +507,8 @@  void pstore_get_records(int quiet)
 			}
 		}
 		rc = pstore_mkfile(type, psi->name, id, count, buf,
-				  compressed, (size_t)size, time, psi);
+				  compressed, (size_t)size,
+				  timespec_to_timespec64(time), psi);
 		if (unzipped_len < 0) {
 			/* Free buffer other than big oops */
 			kfree(buf);
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 3b57443..55a16ef 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -135,7 +135,7 @@  ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint *c, uint max,
 	return prz;
 }
 
-static void ramoops_read_kmsg_hdr(char *buffer, struct timespec *time,
+static void ramoops_read_kmsg_hdr(char *buffer, struct timespec64 *time,
 				  bool *compressed)
 {
 	char data_type;
@@ -157,10 +157,12 @@  static void ramoops_read_kmsg_hdr(char *buffer, struct timespec *time,
 }
 
 static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
-				   int *count, struct timespec *time,
+				   int *count, struct timespec *ts,
 				   char **buf, bool *compressed,
 				   struct pstore_info *psi)
 {
+	struct timespec64 ts64 = timespec_to_timespec64(*ts);
+	struct timespec64 *time = &ts64;
 	ssize_t size;
 	ssize_t ecc_notice_size;
 	struct ramoops_context *cxt = psi->data;
@@ -198,11 +200,10 @@  static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz,
 				     bool compressed)
 {
 	char *hdr;
-	struct timespec timestamp;
+	struct timespec64 timestamp;
 	size_t len;
-
 	/* Report zeroed timestamp if called before timekeeping has resumed. */
-	if (__getnstimeofday(&timestamp)) {
+	if (__getnstimeofday64(&timestamp)) {
 		timestamp.tv_sec = 0;
 		timestamp.tv_nsec = 0;
 	}