[PATCH] drivers: s390: net: ctcm: migrate variables to handle y2038 problem

[PATCH] drivers: s390: net: ctcm: migrate variables to handle y2038 problem

[Aya Mahfouz]

This patch is concerned with migrating the time variables for the s390
network driver. The changes handle the y2038 problem where timespec will
overflow in the year 2038. timespec was replaced by unsigned long and
all time variables get their values from the jiffies global variable.
This was done for the sake of speed and efficiency.

ch->prof.tx_time will now hold the difference between two jiffies'
values instead of the difference between two xtime values
represented in microseconds.

Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
---
v1: Arnd has advised me to provide you with options for time
    calculation. The first option: "accuracy" is used in this
    patch. The second option: "speed" can be done through
    jiffies.

v2: Moved on to the speed option. Let me know if I explicitly
    need to include the jiffies header. The module compiles with
    no problems on my side.

v3: Handled the error pointed out by Ursula. The current version
    does not handle overflows. There are two solutions for this.
    The first is to use jiffies_64 since s/390 is a 64-bit
    architecture after all. The second is to use wrapper functions
    like time_before and time_after. Two files were added too.
    They are: ctcm_main.c and netiucv.c. This patch could be sent
    as a patchset in its final version for convenience.

v4: removed #include <linux/jiffies.h> per Ursula's feedback.
    Changed duration to hold the difference between jiffies
    instead of difference between microsecond values per Arnd's
    suggestion.

v5: added changes written by Ursula to handle missed modifications.

 drivers/s390/net/ctcm_fsms.c  | 18 +++++++-----------
 drivers/s390/net/ctcm_main.c  |  4 ++--
 drivers/s390/net/ctcm_main.h  |  2 +-
 drivers/s390/net/ctcm_sysfs.c |  4 ++--
 drivers/s390/net/netiucv.c    |  6 +++---
 5 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/s390/net/ctcm_fsms.c b/drivers/s390/net/ctcm_fsms.c
index fb92524..49143c3 100644
--- a/drivers/s390/net/ctcm_fsms.c
+++ b/drivers/s390/net/ctcm_fsms.c
@@ -251,13 +251,11 @@ static void chx_txdone(fsm_instance *fi, int event, void *arg)
 	int first = 1;
 	int i;
 	unsigned long duration;
-	struct timespec done_stamp = current_kernel_time(); /* xtime */
+	unsigned long done_stamp jiffies;
 
 	CTCM_PR_DEBUG("%s(%s): %s\n", __func__, ch->id, dev->name);
 
-	duration =
-	    (done_stamp.tv_sec - ch->prof.send_stamp.tv_sec) * 1000000 +
-	    (done_stamp.tv_nsec - ch->prof.send_stamp.tv_nsec) / 1000;
+	duration = done_stamp -ch->prof.send_stamp;
 	if (duration > ch->prof.tx_time)
 		ch->prof.tx_time = duration;
 
@@ -307,7 +305,7 @@ static void chx_txdone(fsm_instance *fi, int event, void *arg)
 		spin_unlock(&ch->collect_lock);
 		ch->ccw[1].count = ch->trans_skb->len;
 		fsm_addtimer(&ch->timer, CTCM_TIME_5_SEC, CTC_EVENT_TIMER, ch);
-		ch->prof.send_stamp = current_kernel_time(); /* xtime */
+		ch->prof.send_stamp = jiffies;
 		rc = ccw_device_start(ch->cdev, &ch->ccw[0],
 						(unsigned long)ch, 0xff, 0);
 		ch->prof.doios_multi++;
@@ -1229,14 +1227,12 @@ static void ctcmpc_chx_txdone(fsm_instance *fi, int event, void *arg)
 	int		rc;
 	struct th_header *header;
 	struct pdu	*p_header;
-	struct timespec done_stamp = current_kernel_time(); /* xtime */
+	unsigned long done_stamp = jiffies;
 
 	CTCM_PR_DEBUG("Enter %s: %s cp:%i\n",
 			__func__, dev->name, smp_processor_id());
 
-	duration =
-		(done_stamp.tv_sec - ch->prof.send_stamp.tv_sec) * 1000000 +
-		(done_stamp.tv_nsec - ch->prof.send_stamp.tv_nsec) / 1000;
+	duration = done_stamp -ch->prof.send_stamp;
 	if (duration > ch->prof.tx_time)
 		ch->prof.tx_time = duration;
 
@@ -1361,7 +1357,7 @@ static void ctcmpc_chx_txdone(fsm_instance *fi, int event, void *arg)
 
 	ch->ccw[1].count = ch->trans_skb->len;
 	fsm_addtimer(&ch->timer, CTCM_TIME_5_SEC, CTC_EVENT_TIMER, ch);
-	ch->prof.send_stamp = current_kernel_time(); /* xtime */
+	ch->prof.send_stamp = jiffies;
 	if (do_debug_ccw)
 		ctcmpc_dumpit((char *)&ch->ccw[0], sizeof(struct ccw1) * 3);
 	rc = ccw_device_start(ch->cdev, &ch->ccw[0],
@@ -1827,7 +1823,7 @@ static void ctcmpc_chx_send_sweep(fsm_instance *fsm, int event, void *arg)
 	fsm_newstate(wch->fsm, CTC_STATE_TX);
 
 	spin_lock_irqsave(get_ccwdev_lock(wch->cdev), saveflags);
-	wch->prof.send_stamp = current_kernel_time(); /* xtime */
+	wch->prof.send_stamp = jiffies;
 	rc = ccw_device_start(wch->cdev, &wch->ccw[3],
 					(unsigned long) wch, 0xff, 0);
 	spin_unlock_irqrestore(get_ccwdev_lock(wch->cdev), saveflags);
diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
index e056dd4..05c37d6 100644
--- a/drivers/s390/net/ctcm_main.c
+++ b/drivers/s390/net/ctcm_main.c
@@ -567,7 +567,7 @@ static int ctcm_transmit_skb(struct channel *ch, struct sk_buff *skb)
 	fsm_newstate(ch->fsm, CTC_STATE_TX);
 	fsm_addtimer(&ch->timer, CTCM_TIME_5_SEC, CTC_EVENT_TIMER, ch);
 	spin_lock_irqsave(get_ccwdev_lock(ch->cdev), saveflags);
-	ch->prof.send_stamp = current_kernel_time(); /* xtime */
+	ch->prof.send_stamp = jiffies;
 	rc = ccw_device_start(ch->cdev, &ch->ccw[ccw_idx],
 					(unsigned long)ch, 0xff, 0);
 	spin_unlock_irqrestore(get_ccwdev_lock(ch->cdev), saveflags);
@@ -831,7 +831,7 @@ static int ctcmpc_transmit_skb(struct channel *ch, struct sk_buff *skb)
 					sizeof(struct ccw1) * 3);
 
 	spin_lock_irqsave(get_ccwdev_lock(ch->cdev), saveflags);
-	ch->prof.send_stamp = current_kernel_time(); /* xtime */
+	ch->prof.send_stamp = jiffies;
 	rc = ccw_device_start(ch->cdev, &ch->ccw[ccw_idx],
 					(unsigned long)ch, 0xff, 0);
 	spin_unlock_irqrestore(get_ccwdev_lock(ch->cdev), saveflags);
diff --git a/drivers/s390/net/ctcm_main.h b/drivers/s390/net/ctcm_main.h
index 477c933..6f4417c 100644
--- a/drivers/s390/net/ctcm_main.h
+++ b/drivers/s390/net/ctcm_main.h
@@ -121,7 +121,7 @@ struct ctcm_profile {
 	unsigned long doios_multi;
 	unsigned long txlen;
 	unsigned long tx_time;
-	struct timespec send_stamp;
+	unsigned long send_stamp;
 };
 
 /*
diff --git a/drivers/s390/net/ctcm_sysfs.c b/drivers/s390/net/ctcm_sysfs.c
index 6bcfbbb..c88f465 100644
--- a/drivers/s390/net/ctcm_sysfs.c
+++ b/drivers/s390/net/ctcm_sysfs.c
@@ -100,8 +100,8 @@ static void ctcm_print_statistics(struct ctcm_priv *priv)
 		     priv->channel[WRITE]->prof.doios_multi);
 	p += sprintf(p, "  Netto bytes written: %ld\n",
 		     priv->channel[WRITE]->prof.txlen);
-	p += sprintf(p, "  Max. TX IO-time: %ld\n",
-		     priv->channel[WRITE]->prof.tx_time);
+	p += sprintf(p, "  Max. TX IO-time: %u\n",
+		jiffies_to_usecs(priv->channel[WRITE]->prof.tx_time));
 
 	printk(KERN_INFO "Statistics for %s:\n%s",
 				priv->channel[CTCM_WRITE]->netdev->name, sbuf);
diff --git a/drivers/s390/net/netiucv.c b/drivers/s390/net/netiucv.c
index 0a87809..7e91f54 100644
--- a/drivers/s390/net/netiucv.c
+++ b/drivers/s390/net/netiucv.c
@@ -178,7 +178,7 @@ struct connection_profile {
 	unsigned long doios_multi;
 	unsigned long txlen;
 	unsigned long tx_time;
-	struct timespec send_stamp;
+	unsigned long send_stamp;
 	unsigned long tx_pending;
 	unsigned long tx_max_pending;
 };
@@ -786,7 +786,7 @@ static void conn_action_txdone(fsm_instance *fi, int event, void *arg)
 
 	header.next = 0;
 	memcpy(skb_put(conn->tx_buff, NETIUCV_HDRLEN), &header, NETIUCV_HDRLEN);
-	conn->prof.send_stamp = current_kernel_time();
+	conn->prof.send_stamp = jiffies;
 	txmsg.class = 0;
 	txmsg.tag = 0;
 	rc = iucv_message_send(conn->path, &txmsg, 0, 0,
@@ -1220,7 +1220,7 @@ static int netiucv_transmit_skb(struct iucv_connection *conn,
 		memcpy(skb_put(nskb, NETIUCV_HDRLEN), &header,  NETIUCV_HDRLEN);
 
 		fsm_newstate(conn->fsm, CONN_STATE_TX);
-		conn->prof.send_stamp = current_kernel_time();
+		conn->prof.send_stamp = jiffies;
 
 		msg.tag = 1;
 		msg.class = 0;
-- 
1.9.3

Re: [PATCH] drivers: s390: net: ctcm: migrate variables to handle y2038 problem

[Ursula Braun]

Aya,

thanks for this patch and thanks for calling our attention to this
future problem in our code. Here you come up with a solution optimized
for "accuracy", but you mention already an alternative solution
optimized for "speed". In ctcm the time stamps are just used to deliver
a statistic value. Since the time stamps are used in the hot traffic
path of the ctcm-driver, I prefer the "speed" optimized solution based
on jiffies here.

Regards, Ursula Braun

On Wed, 2014-10-29 at 18:02 +0200, Aya Mahfouz wrote:
> This patch is concerned with migrating the time variables for the s390
> network driver. The changes handle the y2038 problem where timespec will
> overflow in the year 2038. timespec64 and unsigned long long were used
> instead of timespec and unsigned long respectively in two files.
> All timespec variables use monotonic time values through the function
> ktime_get_ts64(). This is to make sure that all the time values
> are calculated from a fixed point.
> 
> Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> ---
> v1: Arnd has advised me to provide you with options for time 
>     calculation. The first option: "accuracy" is used in this
>     patch. The second option: "speed" can be done through
>     jiffies and current_kernel_time().
> 
>  drivers/s390/net/ctcm_fsms.c | 18 +++++++++++-------
>  drivers/s390/net/ctcm_main.h |  4 ++--
>  2 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/s390/net/ctcm_fsms.c b/drivers/s390/net/ctcm_fsms.c
> index fb92524..233aef8 100644
> --- a/drivers/s390/net/ctcm_fsms.c
> +++ b/drivers/s390/net/ctcm_fsms.c
> @@ -250,8 +250,10 @@ static void chx_txdone(fsm_instance *fi, int event, void *arg)
>  	struct sk_buff *skb;
>  	int first = 1;
>  	int i;
> -	unsigned long duration;
> -	struct timespec done_stamp = current_kernel_time(); /* xtime */
> +	unsigned long long duration;
> +	struct timespec64 done_stamp;
> +
> +	ktime_get_ts64(&done_stamp);
> 
>  	CTCM_PR_DEBUG("%s(%s): %s\n", __func__, ch->id, dev->name);
> 
> @@ -307,7 +309,7 @@ static void chx_txdone(fsm_instance *fi, int event, void *arg)
>  		spin_unlock(&ch->collect_lock);
>  		ch->ccw[1].count = ch->trans_skb->len;
>  		fsm_addtimer(&ch->timer, CTCM_TIME_5_SEC, CTC_EVENT_TIMER, ch);
> -		ch->prof.send_stamp = current_kernel_time(); /* xtime */
> +		ktime_get_ts64(&ch->prof.sendstamp);
>  		rc = ccw_device_start(ch->cdev, &ch->ccw[0],
>  						(unsigned long)ch, 0xff, 0);
>  		ch->prof.doios_multi++;
> @@ -1224,12 +1226,14 @@ static void ctcmpc_chx_txdone(fsm_instance *fi, int event, void *arg)
>  	int		first = 1;
>  	int		i;
>  	__u32		data_space;
> -	unsigned long	duration;
> +	unsigned long long	duration;
>  	struct sk_buff	*peekskb;
>  	int		rc;
>  	struct th_header *header;
>  	struct pdu	*p_header;
> -	struct timespec done_stamp = current_kernel_time(); /* xtime */
> +	struct timespec64 done_stamp;
> +
> +	ktime_get_ts64(&done_stamp);
> 
>  	CTCM_PR_DEBUG("Enter %s: %s cp:%i\n",
>  			__func__, dev->name, smp_processor_id());
> @@ -1361,7 +1365,7 @@ static void ctcmpc_chx_txdone(fsm_instance *fi, int event, void *arg)
> 
>  	ch->ccw[1].count = ch->trans_skb->len;
>  	fsm_addtimer(&ch->timer, CTCM_TIME_5_SEC, CTC_EVENT_TIMER, ch);
> -	ch->prof.send_stamp = current_kernel_time(); /* xtime */
> +	ktime_get_ts64(&ch->prof.send_stamp);
>  	if (do_debug_ccw)
>  		ctcmpc_dumpit((char *)&ch->ccw[0], sizeof(struct ccw1) * 3);
>  	rc = ccw_device_start(ch->cdev, &ch->ccw[0],
> @@ -1827,7 +1831,7 @@ static void ctcmpc_chx_send_sweep(fsm_instance *fsm, int event, void *arg)
>  	fsm_newstate(wch->fsm, CTC_STATE_TX);
> 
>  	spin_lock_irqsave(get_ccwdev_lock(wch->cdev), saveflags);
> -	wch->prof.send_stamp = current_kernel_time(); /* xtime */
> +	ktime_get_ts64(&wch->prof.send_stamp);
>  	rc = ccw_device_start(wch->cdev, &wch->ccw[3],
>  					(unsigned long) wch, 0xff, 0);
>  	spin_unlock_irqrestore(get_ccwdev_lock(wch->cdev), saveflags);
> diff --git a/drivers/s390/net/ctcm_main.h b/drivers/s390/net/ctcm_main.h
> index 477c933..ed7d44a 100644
> --- a/drivers/s390/net/ctcm_main.h
> +++ b/drivers/s390/net/ctcm_main.h
> @@ -120,8 +120,8 @@ struct ctcm_profile {
>  	unsigned long doios_single;
>  	unsigned long doios_multi;
>  	unsigned long txlen;
> -	unsigned long tx_time;
> -	struct timespec send_stamp;
> +	unsigned long long tx_time;
> +	struct timespec64 send_stamp;
>  };
> 
>  /*

[PATCH] drivers: s390: net: ctcm: migrate variables to handle y2038 problem

[Aya Mahfouz]

This patch is concerned with migrating the time variables for the s390
network driver. The changes handle the y2038 problem where timespec will
overflow in the year 2038. timespec64 and unsigned long long were used
instead of timespec and unsigned long respectively in two files.
All timespec variables use monotonic time values through the function
ktime_get_ts64(). This is to make sure that all the time values
are calculated from a fixed point.

Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
---
v1: Arnd has advised me to provide you with options for time 
    calculation. The first option: "accuracy" is used in this
    patch. The second option: "speed" can be done through
    jiffies and current_kernel_time().

 drivers/s390/net/ctcm_fsms.c | 18 +++++++++++-------
 drivers/s390/net/ctcm_main.h |  4 ++--
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/s390/net/ctcm_fsms.c b/drivers/s390/net/ctcm_fsms.c
index fb92524..233aef8 100644
--- a/drivers/s390/net/ctcm_fsms.c
+++ b/drivers/s390/net/ctcm_fsms.c
@@ -250,8 +250,10 @@ static void chx_txdone(fsm_instance *fi, int event, void *arg)
 	struct sk_buff *skb;
 	int first = 1;
 	int i;
-	unsigned long duration;
-	struct timespec done_stamp = current_kernel_time(); /* xtime */
+	unsigned long long duration;
+	struct timespec64 done_stamp;
+
+	ktime_get_ts64(&done_stamp);
 
 	CTCM_PR_DEBUG("%s(%s): %s\n", __func__, ch->id, dev->name);
 
@@ -307,7 +309,7 @@ static void chx_txdone(fsm_instance *fi, int event, void *arg)
 		spin_unlock(&ch->collect_lock);
 		ch->ccw[1].count = ch->trans_skb->len;
 		fsm_addtimer(&ch->timer, CTCM_TIME_5_SEC, CTC_EVENT_TIMER, ch);
-		ch->prof.send_stamp = current_kernel_time(); /* xtime */
+		ktime_get_ts64(&ch->prof.sendstamp);
 		rc = ccw_device_start(ch->cdev, &ch->ccw[0],
 						(unsigned long)ch, 0xff, 0);
 		ch->prof.doios_multi++;
@@ -1224,12 +1226,14 @@ static void ctcmpc_chx_txdone(fsm_instance *fi, int event, void *arg)
 	int		first = 1;
 	int		i;
 	__u32		data_space;
-	unsigned long	duration;
+	unsigned long long	duration;
 	struct sk_buff	*peekskb;
 	int		rc;
 	struct th_header *header;
 	struct pdu	*p_header;
-	struct timespec done_stamp = current_kernel_time(); /* xtime */
+	struct timespec64 done_stamp;
+
+	ktime_get_ts64(&done_stamp);
 
 	CTCM_PR_DEBUG("Enter %s: %s cp:%i\n",
 			__func__, dev->name, smp_processor_id());
@@ -1361,7 +1365,7 @@ static void ctcmpc_chx_txdone(fsm_instance *fi, int event, void *arg)
 
 	ch->ccw[1].count = ch->trans_skb->len;
 	fsm_addtimer(&ch->timer, CTCM_TIME_5_SEC, CTC_EVENT_TIMER, ch);
-	ch->prof.send_stamp = current_kernel_time(); /* xtime */
+	ktime_get_ts64(&ch->prof.send_stamp);
 	if (do_debug_ccw)
 		ctcmpc_dumpit((char *)&ch->ccw[0], sizeof(struct ccw1) * 3);
 	rc = ccw_device_start(ch->cdev, &ch->ccw[0],
@@ -1827,7 +1831,7 @@ static void ctcmpc_chx_send_sweep(fsm_instance *fsm, int event, void *arg)
 	fsm_newstate(wch->fsm, CTC_STATE_TX);
 
 	spin_lock_irqsave(get_ccwdev_lock(wch->cdev), saveflags);
-	wch->prof.send_stamp = current_kernel_time(); /* xtime */
+	ktime_get_ts64(&wch->prof.send_stamp);
 	rc = ccw_device_start(wch->cdev, &wch->ccw[3],
 					(unsigned long) wch, 0xff, 0);
 	spin_unlock_irqrestore(get_ccwdev_lock(wch->cdev), saveflags);
diff --git a/drivers/s390/net/ctcm_main.h b/drivers/s390/net/ctcm_main.h
index 477c933..ed7d44a 100644
--- a/drivers/s390/net/ctcm_main.h
+++ b/drivers/s390/net/ctcm_main.h
@@ -120,8 +120,8 @@ struct ctcm_profile {
 	unsigned long doios_single;
 	unsigned long doios_multi;
 	unsigned long txlen;
-	unsigned long tx_time;
-	struct timespec send_stamp;
+	unsigned long long tx_time;
+	struct timespec64 send_stamp;
 };
 
 /*
-- 
1.9.3