10:42 a.m.
Removed "drivers" from the subject. I saw examples of this in the
commit log but have since realized this is not preferred.
v3:
Added Andreas as reviewer (thanks!)
v2:
Added a second patch to address Dan Carpenter's concern with the
complexity of passing the sign through `mult'. Compile tested only.
Chris Rorvick (2):
staging: lustre: Use mult if units not specified
staging: lustre: Track sign separately
drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
--
2.1.0
10:42 a.m.
New subject: [PATCH v4 1/2] staging: lustre: Use mult if units not specified
Units can be passed to lprocfs_write_frac_u64_helper() via a suffix
(e.g., "...K", "...M", etc.) tacked onto the value. A comment states
that "specified units override the multiplier," though the multiplier is
overridden regardless. Update the conditional logic so that it only
applies when units are specified.
Signed-off-by: Chris Rorvick <chris(a)rorvick.com>
Reviewed-by: Andreas Dilger <andreas.dilger(a)intel.com>
---
drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index 3b7dfc3..a7b270e 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -1908,7 +1908,7 @@ int lprocfs_write_frac_u64_helper(const char *buffer, unsigned long
count,
units <<= 10;
}
/* Specified units override the multiplier */
- if (units)
+ if (units > 1)
mult = mult < 0 ? -units : units;
frac *= mult;
--
2.1.0
10:42 a.m.
New subject: [PATCH v4 2/2] staging: lustre: Track sign separately
The `mult' parameter is negated if the user data begins with a '-' so
that the final value has the appropriate sign. But `mult' is only used
if the user data does not include a "units" suffix. In this case,
`mult' is overridden with the numeric scale conveyed by the units suffix,
but retains the sign of the original value.
Having `mult' serving double-duty works but is confusing. Use a new
local variable to store the sign of the user data instead. This also
fixes a pitfall of passing 0 to `mult', expecting it to be ignored when
a units suffix is specified, but having the effect of taking the
absolute value of the user-provided data.
Signed-off-by: Chris Rorvick <chris(a)rorvick.com>
Reviewed-by: Andreas Dilger <andreas.dilger(a)intel.com>
---
drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index a7b270e..22e13f0 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -1862,6 +1862,7 @@ int lprocfs_write_frac_u64_helper(const char *buffer, unsigned long
count,
char kernbuf[22], *end, *pbuf;
__u64 whole, frac = 0, units;
unsigned frac_d = 1;
+ int sign = 1;
if (count > (sizeof(kernbuf) - 1))
return -EINVAL;
@@ -1872,7 +1873,7 @@ int lprocfs_write_frac_u64_helper(const char *buffer, unsigned long
count,
kernbuf[count] = '\0';
pbuf = kernbuf;
if (*pbuf == '-') {
- mult = -mult;
+ sign = -1;
pbuf++;
}
@@ -1909,11 +1910,11 @@ int lprocfs_write_frac_u64_helper(const char *buffer, unsigned
long count,
}
/* Specified units override the multiplier */
if (units > 1)
- mult = mult < 0 ? -units : units;
+ mult = units;
frac *= mult;
do_div(frac, frac_d);
- *val = whole * mult + frac;
+ *val = sign * (whole * mult + frac);
return 0;
}
EXPORT_SYMBOL(lprocfs_write_frac_u64_helper);
--
2.1.0
9:30 p.m.
Since no one had any responses to the original thread, Greg is going to
see that first and apply it and this one will not apply. It's not worth
resending patches for trivial stuff like this anyway...
regards,
dan carpenter
2805
days inactive
2807
days old
3 comments
2 participants
participants (2)
-
Chris Rorvick -
Dan Carpenter