[PATCH 1/1] Staging: bcm: Replace timeval with ktime_t

[Somya Anand]

'stLastUpdateTokenAt' is used to store the last updated time
and hence to calculate time taken or difference.

'stLastUpdateTokenAt' is of type 'struct timeval' and according to
y2038 principle, all the instances of 'struct timeval' should be
removed.

Changelog:

* Change type of 'struct bcm_packet_info.liLastUpdateTokenAt' from
LARGE_INTEGER to ktime_t.

* Change type of 'struct bcm_packet_info.stLastUpdateTokenAt' from
struct timeval to ktime_t.

* Change log timestamp resolution from milli seconds to nano seconds.

Signed-off-by: Somya Anand <somyaa...@gmail.com>
---
drivers/staging/bcm/Adapter.h | 5 +++--
drivers/staging/bcm/LeakyBucket.c | 18 +++++++-----------
drivers/staging/bcm/Misc.c | 11 +++--------
drivers/staging/bcm/headers.h | 1 +
4 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/bcm/Adapter.h b/drivers/staging/bcm/Adapter.h
index 940c852..06a65c3 100644
--- a/drivers/staging/bcm/Adapter.h
+++ b/drivers/staging/bcm/Adapter.h
@@ -6,6 +6,7 @@

#define MAX_FRAGMENTEDIP_CLASSIFICATION_ENTRIES 256
#include "Debug.h"
+#include <linux/ktime.h>

struct bcm_leader {
USHORT Vcid;
@@ -155,7 +156,7 @@ struct bcm_packet_info {
UINT uiThisPeriodSentBytes;
LARGE_INTEGER liDrainCalculated;
UINT uiCurrentTokenCount;
- LARGE_INTEGER liLastUpdateTokenAt;
+ ktime_t liLastUpdateTokenAt;
UINT uiMaxAllowedRate;
UINT NumOfPacketsSent;
UCHAR ucDirection;
@@ -192,7 +193,7 @@ struct bcm_packet_info {
bool bHeaderSuppressionEnabled;
spinlock_t SFQueueLock;
void *pstSFIndication;
- struct timeval stLastUpdateTokenAt;
+ ktime_t stLastUpdateTokenAt;
atomic_t uiPerSFTxResourceCount;
UINT uiMaxLatency;
UCHAR bIPCSSupport;
diff --git a/drivers/staging/bcm/LeakyBucket.c b/drivers/staging/bcm/LeakyBucket.c
index d6b55f9..2f7f12d 100644
--- a/drivers/staging/bcm/LeakyBucket.c
+++ b/drivers/staging/bcm/LeakyBucket.c
@@ -13,9 +13,9 @@
*/
static VOID UpdateTokenCount(register struct bcm_mini_adapter *Adapter)
{
- ULONG liCurrentTime;
+ ktime_t liCurrentTime;
INT i = 0;
- struct timeval tv;
+ ktime_t kt;
struct bcm_packet_info *curr_pi;

BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, TOKEN_COUNTS, DBG_LVL_ALL,
@@ -26,21 +26,17 @@ static VOID UpdateTokenCount(register struct bcm_mini_adapter *Adapter)
return;
}

- do_gettimeofday(&tv);
+ kt = ktime_get();
for (i = 0; i < NO_OF_QUEUES; i++) {
curr_pi = &Adapter->PackInfo[i];

if (TRUE == curr_pi->bValid && (1 == curr_pi->ucDirection)) {
- liCurrentTime = ((tv.tv_sec -
- curr_pi->stLastUpdateTokenAt.tv_sec)*1000 +
- (tv.tv_usec - curr_pi->stLastUpdateTokenAt.tv_usec) /
- 1000);
- if (0 != liCurrentTime) {
+ liCurrentTime = ktime_sub(kt, curr_pi->stLastUpdateTokenAt);
+ if (ktime_compare(liCurrentTime, ktime_set(0, NSEC_PER_MSEC)) < 0) {
curr_pi->uiCurrentTokenCount += (ULONG)
((curr_pi->uiMaxAllowedRate) *
- ((ULONG)((liCurrentTime)))/1000);
- memcpy(&curr_pi->stLastUpdateTokenAt, &tv,
- sizeof(struct timeval));
+ ((ULONG)((ktime_to_ms(liCurrentTime))))/1000);
+ curr_pi->stLastUpdateTokenAt = kt;
curr_pi->liLastUpdateTokenAt = liCurrentTime;
if (curr_pi->uiCurrentTokenCount >=
curr_pi->uiMaxBucketSize) {
diff --git a/drivers/staging/bcm/Misc.c b/drivers/staging/bcm/Misc.c
index 883f739..b40f00f 100644
--- a/drivers/staging/bcm/Misc.c
+++ b/drivers/staging/bcm/Misc.c
@@ -180,7 +180,6 @@ static int BcmFileDownload(struct bcm_mini_adapter *Adapter, const char *path, u
{
int errorno = 0;
struct file *flp = NULL;
- struct timeval tv = {0};

flp = open_firmware_file(Adapter, path);
if (!flp) {
@@ -188,9 +187,8 @@ static int BcmFileDownload(struct bcm_mini_adapter *Adapter, const char *path, u
return -ENOENT;
}
BCM_DEBUG_PRINT(Adapter, DBG_TYPE_INITEXIT, MP_INIT, DBG_LVL_ALL, "Opened file is = %s and length =0x%lx to be downloaded at =0x%x", path, (unsigned long)file_inode(flp)->i_size, loc);
- do_gettimeofday(&tv);

- BCM_DEBUG_PRINT(Adapter, DBG_TYPE_INITEXIT, MP_INIT, DBG_LVL_ALL, "download start %lx", ((tv.tv_sec * 1000) + (tv.tv_usec / 1000)));
+ BCM_DEBUG_PRINT(Adapter, DBG_TYPE_INITEXIT, MP_INIT, DBG_LVL_ALL, "download start %llu", ktime_get_real_ns());
if (Adapter->bcm_file_download(Adapter->pvInterfaceAdapter, flp, loc)) {
BCM_DEBUG_PRINT(Adapter, DBG_TYPE_INITEXIT, MP_INIT, DBG_LVL_ALL, "Failed to download the firmware with error %x!!!", -EIO);
errorno = -EIO;
@@ -548,10 +546,8 @@ void LinkControlResponseMessage(struct bcm_mini_adapter *Adapter, PUCHAR pucBuff
void SendIdleModeResponse(struct bcm_mini_adapter *Adapter)
{
int status = 0, NVMAccess = 0, lowPwrAbortMsg = 0;
- struct timeval tv;
struct bcm_link_request stIdleResponse = {{0} };

- memset(&tv, 0, sizeof(tv));
stIdleResponse.Leader.Status = IDLE_MESSAGE;
stIdleResponse.Leader.PLength = IDLE_MODE_PAYLOAD_LENGTH;
stIdleResponse.szData[0] = GO_TO_IDLE_MODE_PAYLOAD;
@@ -632,8 +628,7 @@ void SendIdleModeResponse(struct bcm_mini_adapter *Adapter)
Adapter->bPreparingForLowPowerMode = false;
StartInterruptUrb((struct bcm_interface_adapter *)(Adapter->pvInterfaceAdapter));
}
- do_gettimeofday(&tv);
- BCM_DEBUG_PRINT(Adapter, DBG_TYPE_RX, RX_DPC, DBG_LVL_ALL, "IdleMode Msg submitter to Q :%ld ms", tv.tv_sec * 1000 + tv.tv_usec / 1000);
+ BCM_DEBUG_PRINT(Adapter, DBG_TYPE_RX, RX_DPC, DBG_LVL_ALL, "IdleMode Msg submitter to Q :%llu ns", ktime_get_real_ns());
}

/******************************************************************
@@ -742,7 +737,7 @@ void DumpPackInfo(struct bcm_mini_adapter *Adapter)
BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, DUMP_INFO, DBG_LVL_ALL, "uiThisPeriodSentBytes:%X\n", Adapter->PackInfo[uiLoopIndex].uiThisPeriodSentBytes);
BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, DUMP_INFO, DBG_LVL_ALL, "liDrainCalculated:%llX\n", Adapter->PackInfo[uiLoopIndex].liDrainCalculated);
BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, DUMP_INFO, DBG_LVL_ALL, "uiCurrentTokenCount:%X\n", Adapter->PackInfo[uiLoopIndex].uiCurrentTokenCount);
- BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, DUMP_INFO, DBG_LVL_ALL, "liLastUpdateTokenAt:%llX\n", Adapter->PackInfo[uiLoopIndex].liLastUpdateTokenAt);
+ BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, DUMP_INFO, DBG_LVL_ALL, "liLastUpdateTokenAt:%llu\n", ktime_to_ms(Adapter->PackInfo[uiLoopIndex].liLastUpdateTokenAt));
BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, DUMP_INFO, DBG_LVL_ALL, "uiMaxAllowedRate:%X\n", Adapter->PackInfo[uiLoopIndex].uiMaxAllowedRate);
BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, DUMP_INFO, DBG_LVL_ALL, "uiPendedLast:%X\n", Adapter->PackInfo[uiLoopIndex].uiPendedLast);
BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, DUMP_INFO, DBG_LVL_ALL, "NumOfPacketsSent:%X\n", Adapter->PackInfo[uiLoopIndex].NumOfPacketsSent);
diff --git a/drivers/staging/bcm/headers.h b/drivers/staging/bcm/headers.h
index a7d4af5..e0be207 100644
--- a/drivers/staging/bcm/headers.h
+++ b/drivers/staging/bcm/headers.h
@@ -35,6 +35,7 @@
#include <linux/udp.h>
#include <linux/usb.h>
#include <linux/uaccess.h>
+#include <linux/ktime.h>
#include <net/ip.h>

#include "Typedefs.h"
--
1.9.1

[Arnd Bergmann]

On Saturday 11 October 2014 13:41:25 Somya Anand wrote:
> 'stLastUpdateTokenAt' is used to store the last updated time
> and hence to calculate time taken or difference.
>
> 'stLastUpdateTokenAt' is of type 'struct timeval' and according to
> y2038 principle, all the instances of 'struct timeval' should be
> removed.
>
> Changelog:
>
> * Change type of 'struct bcm_packet_info.liLastUpdateTokenAt' from
> LARGE_INTEGER to ktime_t.
>
> * Change type of 'struct bcm_packet_info.stLastUpdateTokenAt' from
> struct timeval to ktime_t.
>
> * Change log timestamp resolution from milli seconds to nano seconds.
>
> Signed-off-by: Somya Anand <somyaa...@gmail.com>

Looks almost perfect now, two more things:

- for the description, don't use a separate 'Changelog' section, the
description is the same as the changelog. See also the other mail
I just sent.

- this code is still slightly inefficient:

> curr_pi->uiCurrentTokenCount += (ULONG)
> ((curr_pi->uiMaxAllowedRate) *
> - ((ULONG)((liCurrentTime)))/1000);

> + ((ULONG)((ktime_to_ms(liCurrentTime))))/1000);


since you do two divisions here where one should be enough.
Unfortunately there is no ktime_to_seconds interface, but you
could use

(u32)do_div(ktime_to_ns(liCurrentTime), NSEC_PER_SEC)

Unfortunately, I had missed this use of liCurrentTime in my earlier
review, so the idea to pass down liCurrentTime as a ktime_t does not
actually avoid the conversion in this function.

Arnd

[Somya Anand]

'stLastUpdateTokenAt' is used to store the last updated time
and hence to calculate time taken or difference.

'stLastUpdateTokenAt' is of type 'struct timeval' and according to
y2038 principle, all the instances of 'struct timeval' should be

removed because 32-bit systems using 'struct timeval' will break
in the year 2038, so we have to replace that code with more
appropriate types. This patch changes the bcm driver to use ktime_t

Changelog:

* Change type of 'struct bcm_packet_info.liLastUpdateTokenAt' from
LARGE_INTEGER to ktime_t.

* Change type of 'struct bcm_packet_info.stLastUpdateTokenAt' from
struct timeval to ktime_t.

* Change log timestamp resolution from milli seconds to nano seconds.

Signed-off-by: Somya Anand <somyaa...@gmail.com>

curr_pi->uiCurrentTokenCount += (ULONG)
((curr_pi->uiMaxAllowedRate) *
- ((ULONG)((liCurrentTime)))/1000);

[Somya Anand]

'stLastUpdateTokenAt' is used to store the last updated time
and hence to calculate time taken or difference.

'stLastUpdateTokenAt' is of type 'struct timeval' and according to
y2038 principle, all the instances of 'struct timeval' should be
removed because 32-bit systems using 'struct timeval' will break
in the year 2038, so we have to replace that code with more

appropriate types. This patch changes the bcm driver to use ktime_t.

* Change type of 'struct bcm_packet_info.liLastUpdateTokenAt' from
LARGE_INTEGER to ktime_t.

* Change type of 'struct bcm_packet_info.stLastUpdateTokenAt' from
struct timeval to ktime_t.

* Change log timestamp resolution from milli seconds to nano seconds.

Signed-off-by: Somya Anand <somyaa...@gmail.com>
---
drivers/staging/bcm/Adapter.h | 5 +++--

drivers/staging/bcm/LeakyBucket.c | 23 ++++++++++++-----------

drivers/staging/bcm/Misc.c | 11 +++--------
drivers/staging/bcm/headers.h | 1 +

4 files changed, 19 insertions(+), 21 deletions(-)

index d6b55f9..c15da7c 100644

--- a/drivers/staging/bcm/LeakyBucket.c
+++ b/drivers/staging/bcm/LeakyBucket.c
@@ -13,9 +13,9 @@
*/
static VOID UpdateTokenCount(register struct bcm_mini_adapter *Adapter)
{
- ULONG liCurrentTime;
+ ktime_t liCurrentTime;
INT i = 0;
- struct timeval tv;
+ ktime_t kt;
struct bcm_packet_info *curr_pi;

BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, TOKEN_COUNTS, DBG_LVL_ALL,

@@ -26,21 +26,22 @@ static VOID UpdateTokenCount(register struct bcm_mini_adapter *Adapter)

return;
}

- do_gettimeofday(&tv);
+ kt = ktime_get();
for (i = 0; i < NO_OF_QUEUES; i++) {
curr_pi = &Adapter->PackInfo[i];

if (TRUE == curr_pi->bValid && (1 == curr_pi->ucDirection)) {
- liCurrentTime = ((tv.tv_sec -
- curr_pi->stLastUpdateTokenAt.tv_sec)*1000 +
- (tv.tv_usec - curr_pi->stLastUpdateTokenAt.tv_usec) /
- 1000);
- if (0 != liCurrentTime) {
+ liCurrentTime = ktime_sub(kt,

+ curr_pi->stLastUpdateTokenAt);
+ if (
+ ktime_compare(liCurrentTime,
+ ktime_set(0, NSEC_PER_MSEC)) < 0
+ ) {

curr_pi->uiCurrentTokenCount += (ULONG)
((curr_pi->uiMaxAllowedRate) *
- ((ULONG)((liCurrentTime)))/1000);
- memcpy(&curr_pi->stLastUpdateTokenAt, &tv,
- sizeof(struct timeval));

+ ((u32)do_div(ktime_to_ns(liCurrentTime),
+ NSEC_PER_SEC)));

[Greg KH]

On Sun, Oct 12, 2014 at 01:26:42PM +0530, Somya Anand wrote:
> 'stLastUpdateTokenAt' is used to store the last updated time
> and hence to calculate time taken or difference.
>
> 'stLastUpdateTokenAt' is of type 'struct timeval' and according to
> y2038 principle, all the instances of 'struct timeval' should be
> removed because 32-bit systems using 'struct timeval' will break
> in the year 2038, so we have to replace that code with more
> appropriate types. This patch changes the bcm driver to use ktime_t.
>
> * Change type of 'struct bcm_packet_info.liLastUpdateTokenAt' from
> LARGE_INTEGER to ktime_t.
>
> * Change type of 'struct bcm_packet_info.stLastUpdateTokenAt' from
> struct timeval to ktime_t.
>
> * Change log timestamp resolution from milli seconds to nano seconds.
>
> Signed-off-by: Somya Anand <somyaa...@gmail.com>
> ---
> drivers/staging/bcm/Adapter.h | 5 +++--
> drivers/staging/bcm/LeakyBucket.c | 23 ++++++++++++-----------
> drivers/staging/bcm/Misc.c | 11 +++--------
> drivers/staging/bcm/headers.h | 1 +
> 4 files changed, 19 insertions(+), 21 deletions(-)

This driver is now deleted from my tree, so there's nothing to change
anymore, sorry :(

greg k-h