2014-12-26 00:05:51

by Jeremiah Mahler

[permalink] [raw]
Subject: [PATCH 0/6] staging: lustre: use built in min/max functions

The lustre modules define their own custom MIN, MAX and min_t functions.
However Linux provides these functions (include/linux/kernel.h) and their
design is more robust. They check types and produce compiler warnings
if differing types are used.

This patch set updates the lustre modules to use the built in min/max
functions.

Jeremiah Mahler (6):
staging: lustre: use min/max instead of MIN/MAX, simple cases
staging: lustre: replace MIN with min_t
staging: lustre: replace MIN/MAX with min_t/max_t
staging: lustre: replace MIN with min, cast (__kernel_size_t)
staging: lustre: replace MIN with min_t, remove cast
staging: lustre: remove custom MIN/MAX and min_t operations

.../lustre/include/linux/libcfs/libcfs_private.h | 8 --------
.../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 2 +-
drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c | 2 +-
.../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 6 +++---
drivers/staging/lustre/lnet/lnet/lib-move.c | 20 ++++++++++----------
drivers/staging/lustre/lnet/lnet/router.c | 4 ++--
drivers/staging/lustre/lnet/lnet/router_proc.c | 2 +-
drivers/staging/lustre/lnet/selftest/conrpc.h | 2 +-
drivers/staging/lustre/lnet/selftest/rpc.c | 4 ++--
.../lustre/lustre/libcfs/linux/linux-tracefile.c | 2 +-
drivers/staging/lustre/lustre/osc/osc_internal.h | 5 -----
11 files changed, 22 insertions(+), 35 deletions(-)

--
2.1.4


2014-12-26 00:06:07

by Jeremiah Mahler

[permalink] [raw]
Subject: [PATCH 1/6] staging: lustre: use min/max instead of MIN/MAX, simple cases

Custom MIN/MAX operations are being used which are not as robust
as the built in min/max operations which will warn about potentially
problematic type comparisons.

For the simple cases, where no type warning is produced, simply
replace MIN/MAX with min/max.

Signed-off-by: Jeremiah Mahler <[email protected]>
---
drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 2 +-
drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c | 2 +-
drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 4 ++--
drivers/staging/lustre/lnet/lnet/lib-move.c | 12 ++++++------
drivers/staging/lustre/lnet/lnet/router.c | 4 ++--
drivers/staging/lustre/lnet/selftest/conrpc.h | 2 +-
drivers/staging/lustre/lnet/selftest/rpc.c | 4 ++--
drivers/staging/lustre/lustre/libcfs/linux/linux-tracefile.c | 2 +-
8 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
index b48d7ed..74df2af 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
@@ -1125,7 +1125,7 @@ kiblnd_init_rdma (kib_conn_t *conn, kib_tx_t *tx, int type,
break;
}

- wrknob = MIN(MIN(kiblnd_rd_frag_size(srcrd, srcidx),
+ wrknob = min(min(kiblnd_rd_frag_size(srcrd, srcidx),
kiblnd_rd_frag_size(dstrd, dstidx)), resid);

sge = &tx->tx_sge[tx->tx_nwrq];
diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
index 9188b34..5956dba 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
@@ -773,7 +773,7 @@ ksocknal_select_ips(ksock_peer_t *peer, __u32 *peerips, int n_peerips)
/* Only match interfaces for additional connections
* if I have > 1 interface */
n_ips = (net->ksnn_ninterfaces < 2) ? 0 :
- MIN(n_peerips, net->ksnn_ninterfaces);
+ min(n_peerips, net->ksnn_ninterfaces);

for (i = 0; peer->ksnp_n_passive_ips < n_ips; i++) {
/* ^ yes really... */
diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
index e6c1d36..95f1e17 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
@@ -1950,10 +1950,10 @@ ksocknal_connect (ksock_route_t *route)
/* This is a retry rather than a new connection */
route->ksnr_retry_interval *= 2;
route->ksnr_retry_interval =
- MAX(route->ksnr_retry_interval,
+ max(route->ksnr_retry_interval,
cfs_time_seconds(*ksocknal_tunables.ksnd_min_reconnectms)/1000);
route->ksnr_retry_interval =
- MIN(route->ksnr_retry_interval,
+ min(route->ksnr_retry_interval,
cfs_time_seconds(*ksocknal_tunables.ksnd_max_reconnectms)/1000);

LASSERT (route->ksnr_retry_interval != 0);
diff --git a/drivers/staging/lustre/lnet/lnet/lib-move.c b/drivers/staging/lustre/lnet/lnet/lib-move.c
index c8c1ed8..706c5ee 100644
--- a/drivers/staging/lustre/lnet/lnet/lib-move.c
+++ b/drivers/staging/lustre/lnet/lnet/lib-move.c
@@ -201,9 +201,9 @@ lnet_copy_iov2iov(unsigned int ndiov, struct iovec *diov, unsigned int doffset,
do {
LASSERT(ndiov > 0);
LASSERT(nsiov > 0);
- this_nob = MIN(diov->iov_len - doffset,
+ this_nob = min(diov->iov_len - doffset,
siov->iov_len - soffset);
- this_nob = MIN(this_nob, nob);
+ this_nob = min(this_nob, nob);

memcpy((char *)diov->iov_base + doffset,
(char *)siov->iov_base + soffset, this_nob);
@@ -322,9 +322,9 @@ lnet_copy_kiov2kiov(unsigned int ndiov, lnet_kiov_t *diov, unsigned int doffset,
do {
LASSERT(ndiov > 0);
LASSERT(nsiov > 0);
- this_nob = MIN(diov->kiov_len - doffset,
+ this_nob = min(diov->kiov_len - doffset,
siov->kiov_len - soffset);
- this_nob = MIN(this_nob, nob);
+ this_nob = min(this_nob, nob);

if (daddr == NULL)
daddr = ((char *)kmap(diov->kiov_page)) +
@@ -405,7 +405,7 @@ lnet_copy_kiov2iov(unsigned int niov, struct iovec *iov, unsigned int iovoffset,
LASSERT(nkiov > 0);
this_nob = MIN(iov->iov_len - iovoffset,
kiov->kiov_len - kiovoffset);
- this_nob = MIN(this_nob, nob);
+ this_nob = min(this_nob, nob);

if (addr == NULL)
addr = ((char *)kmap(kiov->kiov_page)) +
@@ -476,7 +476,7 @@ lnet_copy_iov2kiov(unsigned int nkiov, lnet_kiov_t *kiov,
LASSERT(niov > 0);
this_nob = MIN(kiov->kiov_len - kiovoffset,
iov->iov_len - iovoffset);
- this_nob = MIN(this_nob, nob);
+ this_nob = min(this_nob, nob);

if (addr == NULL)
addr = ((char *)kmap(kiov->kiov_page)) +
diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c
index c667b5b..b43e110 100644
--- a/drivers/staging/lustre/lnet/lnet/router.c
+++ b/drivers/staging/lustre/lnet/lnet/router.c
@@ -793,7 +793,7 @@ lnet_update_ni_status_locked(void)
LASSERT(the_lnet.ln_routing);

timeout = router_ping_timeout +
- MAX(live_router_check_interval, dead_router_check_interval);
+ max(live_router_check_interval, dead_router_check_interval);

now = get_seconds();
list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
@@ -1593,7 +1593,7 @@ lnet_router_checker (void)
return;

if (last != 0 &&
- interval > MAX(live_router_check_interval,
+ interval > max(live_router_check_interval,
dead_router_check_interval))
CNETERR("Checker(%d/%d) not called for %d seconds\n",
live_router_check_interval, dead_router_check_interval,
diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.h b/drivers/staging/lustre/lnet/selftest/conrpc.h
index fc1cb56..2353889 100644
--- a/drivers/staging/lustre/lnet/selftest/conrpc.h
+++ b/drivers/staging/lustre/lnet/selftest/conrpc.h
@@ -54,7 +54,7 @@
#define LST_TRANS_TIMEOUT 30
#define LST_TRANS_MIN_TIMEOUT 3

-#define LST_VALIDATE_TIMEOUT(t) MIN(MAX(t, LST_TRANS_MIN_TIMEOUT), LST_TRANS_TIMEOUT)
+#define LST_VALIDATE_TIMEOUT(t) min(max(t, LST_TRANS_MIN_TIMEOUT), LST_TRANS_TIMEOUT)

#define LST_PING_INTERVAL 8

diff --git a/drivers/staging/lustre/lnet/selftest/rpc.c b/drivers/staging/lustre/lnet/selftest/rpc.c
index f753add..b602077 100644
--- a/drivers/staging/lustre/lnet/selftest/rpc.c
+++ b/drivers/staging/lustre/lnet/selftest/rpc.c
@@ -559,7 +559,7 @@ srpc_add_buffer(struct swi_workitem *wi)
LASSERT(scd->scd_buf_posting > 0);
scd->scd_buf_posting--;
scd->scd_buf_total++;
- scd->scd_buf_low = MAX(2, scd->scd_buf_total / 4);
+ scd->scd_buf_low = max(2, scd->scd_buf_total / 4);
}

if (rc != 0) {
@@ -1486,7 +1486,7 @@ srpc_lnet_ev_handler(lnet_event_t *ev)
if (scd->scd_buf_err == 0 && /* adding buffer is enabled */
scd->scd_buf_adjust == 0 &&
scd->scd_buf_nposted < scd->scd_buf_low) {
- scd->scd_buf_adjust = MAX(scd->scd_buf_total / 2,
+ scd->scd_buf_adjust = max(scd->scd_buf_total / 2,
SFW_TEST_WI_MIN);
swi_schedule_workitem(&scd->scd_buf_wi);
}
diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-tracefile.c b/drivers/staging/lustre/lustre/libcfs/linux/linux-tracefile.c
index 976c61e..fdc7189 100644
--- a/drivers/staging/lustre/lustre/libcfs/linux/linux-tracefile.c
+++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-tracefile.c
@@ -269,5 +269,5 @@ int cfs_trace_max_debug_mb(void)
{
int total_mb = (totalram_pages >> (20 - PAGE_SHIFT));

- return MAX(512, (total_mb * 80)/100);
+ return max(512, (total_mb * 80)/100);
}
--
2.1.4

2014-12-26 00:06:15

by Jeremiah Mahler

[permalink] [raw]
Subject: [PATCH 3/6] staging: lustre: replace MIN/MAX with min_t/max_t

Switch from MIN/MAX to min_t/max_t with a size_t type. The size_t type
was chosen because one operand is a size_t and all the others are
immediate integer values.

Signed-off-by: Jeremiah Mahler <[email protected]>
---
drivers/staging/lustre/lnet/lnet/router_proc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lnet/lnet/router_proc.c b/drivers/staging/lustre/lnet/lnet/router_proc.c
index 46cde70..c055afc 100644
--- a/drivers/staging/lustre/lnet/lnet/router_proc.c
+++ b/drivers/staging/lustre/lnet/lnet/router_proc.c
@@ -49,7 +49,7 @@ enum {
*/
#define LNET_PROC_CPT_BITS (LNET_CPT_BITS + 1)
/* change version, 16 bits or 8 bits */
-#define LNET_PROC_VER_BITS MAX(((MIN(LNET_LOFFT_BITS, 64)) / 4), 8)
+#define LNET_PROC_VER_BITS max_t(size_t, min_t(size_t, LNET_LOFFT_BITS, 64) / 4, 8)

#define LNET_PROC_HASH_BITS LNET_PEER_HASH_BITS
/*
--
2.1.4

2014-12-26 00:06:29

by Jeremiah Mahler

[permalink] [raw]
Subject: [PATCH 6/6] staging: lustre: remove custom MIN/MAX and min_t operations

Remove all custom MIN/MAX and min_t operations since they are
no longer needed.

Signed-off-by: Jeremiah Mahler <[email protected]>
---
drivers/staging/lustre/include/linux/libcfs/libcfs_private.h | 8 --------
drivers/staging/lustre/lustre/osc/osc_internal.h | 5 -----
2 files changed, 13 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
index 2817112..3d86fb5 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
@@ -458,14 +458,6 @@ struct libcfs_device_userstate {
struct page *ldu_memhog_root_page;
};

-/* what used to be in portals_lib.h */
-#ifndef MIN
-# define MIN(a, b) (((a) < (b)) ? (a) : (b))
-#endif
-#ifndef MAX
-# define MAX(a, b) (((a) > (b)) ? (a) : (b))
-#endif
-
#define MKSTR(ptr) ((ptr)) ? (ptr) : ""

static inline int cfs_size_round4(int val)
diff --git a/drivers/staging/lustre/lustre/osc/osc_internal.h b/drivers/staging/lustre/lustre/osc/osc_internal.h
index d788dac..af96c7b 100644
--- a/drivers/staging/lustre/lustre/osc/osc_internal.h
+++ b/drivers/staging/lustre/lustre/osc/osc_internal.h
@@ -160,11 +160,6 @@ static inline unsigned long rpcs_in_flight(struct client_obd *cli)
return cli->cl_r_in_flight + cli->cl_w_in_flight;
}

-#ifndef min_t
-#define min_t(type, x, y) \
- ({ type __x = (x); type __y = (y); __x < __y ? __x: __y; })
-#endif
-
struct osc_device {
struct cl_device od_cl;
struct obd_export *od_exp;
--
2.1.4

2014-12-26 00:06:20

by Jeremiah Mahler

[permalink] [raw]
Subject: [PATCH 4/6] staging: lustre: replace MIN with min, cast (__kernel_size_t)

Switch from MIN to min and fix the new type warning. The
warning is produced because a comparison between iov_len,
which is a __kernel_size_t, is made to kiov_len, which is an
unsigned int (include/linux/lnet/types.h). Fix the warning
by casting kiov_len to __kernel_size_t.

Signed-off-by: Jeremiah Mahler <[email protected]>
---
drivers/staging/lustre/lnet/lnet/lib-move.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/lnet/lnet/lib-move.c b/drivers/staging/lustre/lnet/lnet/lib-move.c
index 706c5ee..76eb238 100644
--- a/drivers/staging/lustre/lnet/lnet/lib-move.c
+++ b/drivers/staging/lustre/lnet/lnet/lib-move.c
@@ -403,8 +403,8 @@ lnet_copy_kiov2iov(unsigned int niov, struct iovec *iov, unsigned int iovoffset,
do {
LASSERT(niov > 0);
LASSERT(nkiov > 0);
- this_nob = MIN(iov->iov_len - iovoffset,
- kiov->kiov_len - kiovoffset);
+ this_nob = min(iov->iov_len - iovoffset,
+ (__kernel_size_t) kiov->kiov_len - kiovoffset);
this_nob = min(this_nob, nob);

if (addr == NULL)
@@ -474,7 +474,7 @@ lnet_copy_iov2kiov(unsigned int nkiov, lnet_kiov_t *kiov,
do {
LASSERT(nkiov > 0);
LASSERT(niov > 0);
- this_nob = MIN(kiov->kiov_len - kiovoffset,
+ this_nob = min((__kernel_size_t) kiov->kiov_len - kiovoffset,
iov->iov_len - iovoffset);
this_nob = min(this_nob, nob);

--
2.1.4

2014-12-26 00:06:47

by Jeremiah Mahler

[permalink] [raw]
Subject: [PATCH 5/6] staging: lustre: replace MIN with min_t, remove cast

Switch from MIN to min_t and remove the previous cast of the second
argument to int.

Signed-off-by: Jeremiah Mahler <[email protected]>
---
drivers/staging/lustre/lnet/lnet/lib-move.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lnet/lnet/lib-move.c b/drivers/staging/lustre/lnet/lnet/lib-move.c
index 76eb238..d11686f 100644
--- a/drivers/staging/lustre/lnet/lnet/lib-move.c
+++ b/drivers/staging/lustre/lnet/lnet/lib-move.c
@@ -1530,7 +1530,7 @@ lnet_parse_reply(lnet_ni_t *ni, lnet_msg_t *msg)
LASSERT(md->md_offset == 0);

rlength = hdr->payload_length;
- mlength = MIN(rlength, (int)md->md_length);
+ mlength = min_t(int, rlength, md->md_length);

if (mlength < rlength &&
(md->md_options & LNET_MD_TRUNCATE) == 0) {
--
2.1.4

2014-12-26 00:07:24

by Jeremiah Mahler

[permalink] [raw]
Subject: [PATCH 2/6] staging: lustre: replace MIN with min_t

Switch from MIN to the built in min_t with the int type.

Signed-off-by: Jeremiah Mahler <[email protected]>
---
drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
index 95f1e17..94df05b 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
@@ -1087,7 +1087,7 @@ ksocknal_new_packet (ksock_conn_t *conn, int nob_to_skip)
niov = 0;

do {
- nob = MIN (nob_to_skip, sizeof (ksocknal_slop_buffer));
+ nob = min_t(int, nob_to_skip, sizeof(ksocknal_slop_buffer));

conn->ksnc_rx_iov[niov].iov_base = ksocknal_slop_buffer;
conn->ksnc_rx_iov[niov].iov_len = nob;
--
2.1.4