From: Andreas Dilger <adilger@whamcloud.com>
To: lustre-devel@lists.lustre.org
Subject: [lustre-devel] [PATCH 17/37] lustre: simplify lprocfs_read_frac_helper.
Date: Wed, 27 Feb 2019 01:06:47 +0000 [thread overview]
Message-ID: <D8BE4529-E995-43CA-87F6-31F8FF52FB05@whamcloud.com> (raw)
In-Reply-To: <87sgwat82i.fsf@notabene.neil.brown.name>
On Feb 26, 2019, at 16:59, NeilBrown <neilb@suse.com> wrote:
>
> On Sun, Feb 24 2019, James Simmons wrote:
>
>>> This function seems overly complex, the same functionality
>>> is available with much less effort.
>>
>> Actually we are in discussion about removing these functions.
>> The details are at:
>>
>> https://jira.whamcloud.com/browse/LU-11157
>>
>> Since shells typically don't handle "floating point" representation
>> well you can see the following:
>>
>> sanity 64d with the same error message; '209.9: syntax error: invalid
>> arithmetic operator (error token is ".9")'
>>
>> The second issues is that the read values are not true representation
>> of the desired value. The same issues is with the string helpers as
>> well. Take for example
>>
>> test_string_get_size_one(1100, 1, "1.10 kB", "1.07 KiB");
>>
>> Which is taken from linux/lib/test-string_helpers.c.
>>
>> If you pass 1100 to string_get_size() you can get either 1.10 kB or 1.07
>> KiB. Now if do the reverse 1.10 kB you get ~1126 which is not the same as
>> 11000. I discussed how to handle this problem with Andreas and he agreed
>> the best solution is make all the sysfs / debugfs *_mb files turn any
>> values passed in to around up to the nearest MiB. This way we can report
>> the exact MiB settings to the users. We already have a patch to do this
>> for max_dirty_mb which I can push. The other *_mb files need to be updated
>> to round off. If you can wait a few days I can backport the already done
>> patch and push a patch for the rest.
>
> My recommendation would be to deprecate all _mb files and instead have
> _bytes files which report a simple integer - the number of bytes.
That would break a lot of configurations and documentation. Also, bytes
are not very convenient units for a lot of the numbers, for example you
don't want to (nor could you) send 47-byte RPCs.
> Leave all the unit conversion to user-space.
>
> Linus once told me that he prefers sysfs files to always be in basic
> units, and that decimals are fine. So use "seconds" for time, even if
> that means 0.002 for 2 milliseconds. Use bytes for storage, etc.
That is not going to be better than allowing "max_dirty_mb=0.5", which
is what we wanted to get rid of in the first place. Back in the day
when a system had 4MB DIMMs it was useful to allow fractional MB for the
cache size or debug log size, but I don't think that is useful today.
I'm content to just round these to the nearest MB, in the rare case that
someone specifies a fractional unit, but honestly I don't think that
anyone does anymore. It is much more likely they will be setting the
cache size to 16G per target on their 256GB RAM nodes.
Cheers, Andreas
>>> Signed-off-by: NeilBrown <neilb@suse.com>
>>> ---
>>> .../lustre/lustre/obdclass/lprocfs_status.c | 47 +++-----------------
>>> 1 file changed, 7 insertions(+), 40 deletions(-)
>>>
>>> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>>> index 568e6623e0c8..bd24e48f6145 100644
>>> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>>> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>>> @@ -205,53 +205,20 @@ static void obd_connect_data_seqprint(struct seq_file *m,
>>> int lprocfs_read_frac_helper(char *buffer, unsigned long count, long val,
>>> int mult)
>>> {
>>> - long decimal_val, frac_val;
>>> int prtn;
>>>
>>> if (count < 10)
>>> return -EINVAL;
>>>
>>> - decimal_val = val / mult;
>>> - prtn = snprintf(buffer, count, "%ld", decimal_val);
>>> - frac_val = val % mult;
>>> -
>>> - if (prtn < (count - 4) && frac_val > 0) {
>>> - long temp_frac;
>>> - int i, temp_mult = 1, frac_bits = 0;
>>> -
>>> - temp_frac = frac_val * 10;
>>> - buffer[prtn++] = '.';
>>> - while (frac_bits < 2 && (temp_frac / mult) < 1) {
>>> - /* only reserved 2 bits fraction */
>>> - buffer[prtn++] = '0';
>>> - temp_frac *= 10;
>>> - frac_bits++;
>>> - }
>>> - /*
>>> - * Need to think these cases :
>>> - * 1. #echo x.00 > /sys/xxx output result : x
>>> - * 2. #echo x.0x > /sys/xxx output result : x.0x
>>> - * 3. #echo x.x0 > /sys/xxx output result : x.x
>>> - * 4. #echo x.xx > /sys/xxx output result : x.xx
>>> - * Only reserved 2 bits fraction.
>>> - */
>>> - for (i = 0; i < (5 - prtn); i++)
>>> - temp_mult *= 10;
>>> -
>>> - frac_bits = min((int)count - prtn, 3 - frac_bits);
>>> - prtn += snprintf(buffer + prtn, frac_bits, "%ld",
>>> - frac_val * temp_mult / mult);
>>> + prtn = snprintf(buffer, count, "%ld.%02lu",
>>> + val / mult,
>>> + (val % mult) / (mult / 100));
>>>
>>> + /* Strip trailing zeroes, and trailing '.' */
>>> + while (prtn && buffer[prtn-1] == '0')
>>> + prtn--;
>>> + if (prtn && buffer[prtn-1] == '.')
>>> prtn--;
>>> - while (buffer[prtn] < '1' || buffer[prtn] > '9') {
>>> - prtn--;
>>> - if (buffer[prtn] == '.') {
>>> - prtn--;
>>> - break;
>>> - }
>>> - }
>>> - prtn++;
>>> - }
>>> buffer[prtn++] = '\n';
>>> return prtn;
>>> }
Cheers, Andreas
---
Andreas Dilger
Principal Lustre Architect
Whamcloud
next prev parent reply other threads:[~2019-02-27 1:06 UTC|newest]
Thread overview: 105+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-19 0:09 [lustre-devel] [PATCH 00/37] More lustre patches from obdclass NeilBrown
2019-02-19 0:09 ` [lustre-devel] [PATCH 01/37] lustre: obdclass: char obd_ioctl_getdata type NeilBrown
2019-02-24 18:35 ` James Simmons
2019-02-19 0:09 ` [lustre-devel] [PATCH 02/37] lustre: llite: don't use class_setup_tunables() NeilBrown
2019-02-24 16:35 ` James Simmons
2019-02-25 22:27 ` NeilBrown
2019-02-26 22:18 ` James Simmons
2019-02-24 16:52 ` [lustre-devel] [PATCH 03/37] lustre: embed typ_kobj if obd_type James Simmons
2019-02-25 22:38 ` NeilBrown
2019-02-26 20:41 ` Simmons, James A.
2019-02-19 0:09 ` [lustre-devel] [PATCH 14/37] lustre: llog: change lgh_refcount to struct kref NeilBrown
2019-02-25 18:16 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 13/37] lustre: llog: remove lgh_hdr_lock NeilBrown
2019-02-24 20:29 ` James Simmons
2019-02-25 18:16 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 07/37] lustre: obd_type: discard obd_type_lock NeilBrown
2019-02-24 17:02 ` James Simmons
2019-02-19 0:09 ` [lustre-devel] [PATCH 08/37] lustre: obdclass: don't copy ops structures in to new type NeilBrown
2019-02-24 17:03 ` James Simmons
2019-02-19 0:09 ` [lustre-devel] [PATCH 16/37] lustre: obdclass: typo: Banlance -> Balance NeilBrown
2019-02-24 17:39 ` James Simmons
2019-02-19 0:09 ` [lustre-devel] [PATCH 05/37] lustre: obd_type: use typ_kobj.name as typ_name NeilBrown
2019-02-24 16:56 ` James Simmons
2019-02-19 0:09 ` [lustre-devel] [PATCH 03/37] lustre: embed typ_kobj if obd_type NeilBrown
2019-02-19 0:09 ` [lustre-devel] [PATCH 09/37] lustre: obdclass: fix module load locking NeilBrown
2019-02-24 17:04 ` James Simmons
2019-02-19 0:09 ` [lustre-devel] [PATCH 17/37] lustre: simplify lprocfs_read_frac_helper NeilBrown
2019-02-24 17:52 ` James Simmons
2019-02-26 23:59 ` NeilBrown
2019-02-27 1:06 ` Andreas Dilger [this message]
2019-02-19 0:09 ` [lustre-devel] [PATCH 18/37] lustre: obdclass: discard lprocfs_single/seq_release NeilBrown
2019-02-24 17:53 ` James Simmons
2019-02-19 0:09 ` [lustre-devel] [PATCH 12/37] lustre: remove unused function in linkea NeilBrown
2019-02-25 18:16 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 10/37] lustre: kernelcomm: pass correct gfp_t to kmalloc NeilBrown
2019-02-24 17:05 ` James Simmons
2019-02-25 18:16 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 21/37] lustre: remove several MAX_STRING_SIZE defines NeilBrown
2019-02-24 19:07 ` James Simmons
2019-02-27 0:41 ` NeilBrown
2019-02-25 18:16 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 19/37] lustre: discard lprocfs_strnstr() NeilBrown
2019-02-24 17:53 ` James Simmons
2019-02-19 0:09 ` [lustre-devel] [PATCH 15/37] lustre: llog_obd: Convert loc_refcount to refcount_t NeilBrown
2019-02-25 18:16 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 11/37] lustre: kernelcomm: make libcfs_kkuc_msg_put static NeilBrown
2019-02-24 17:15 ` James Simmons
2019-02-26 23:45 ` NeilBrown
2019-02-27 22:36 ` James Simmons
2019-02-27 22:37 ` James Simmons
2019-02-19 0:09 ` [lustre-devel] [PATCH 04/37] lustre: collect all resource releasing for obj_type NeilBrown
2019-02-24 16:54 ` James Simmons
2019-02-19 0:09 ` [lustre-devel] [PATCH 06/37] lustre: obd_type: discard obd_types linked list NeilBrown
2019-02-24 17:00 ` James Simmons
2019-02-19 0:09 ` [lustre-devel] [PATCH 20/37] lustre: convert rsi_sem to a spinlock NeilBrown
2019-02-25 18:16 ` Andreas Dilger
2019-02-27 0:22 ` NeilBrown
2019-02-27 1:00 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 32/37] lustre: portals_handle: rename ops to owner NeilBrown
2019-02-19 0:09 ` [lustre-devel] [PATCH 22/37] lustre: lprocfs: use log2.h macros instead of shift loop NeilBrown
2019-02-24 18:09 ` James Simmons
2019-02-26 20:55 ` James Simmons
2019-02-27 0:51 ` NeilBrown
2019-02-27 0:54 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 25/37] lustre: deprecate libcfs_debug_vmsg2 NeilBrown
2019-02-24 20:02 ` James Simmons
2019-02-25 18:16 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 30/37] lustre: handle: move refcount into the lustre_handle NeilBrown
2019-02-27 6:32 ` Andreas Dilger
2019-02-27 21:48 ` NeilBrown
2019-02-27 22:14 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 28/37] lustre: remove scope and source from class_incref and class_decref NeilBrown
2019-02-27 6:52 ` Andreas Dilger
2019-02-28 0:39 ` NeilBrown
2019-02-19 0:09 ` [lustre-devel] [PATCH 26/37] lustre: remove libcfs_debug_vmsg2 NeilBrown
2019-02-25 18:16 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 23/37] lustre: prefer to use tabs for alignment NeilBrown
2019-02-24 18:51 ` James Simmons
2019-02-25 18:16 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 27/37] lustre: discard lu_ref NeilBrown
2019-02-24 20:28 ` James Simmons
2019-02-27 1:17 ` NeilBrown
2019-02-27 5:35 ` Andreas Dilger
2019-03-01 6:45 ` Mike Pershin
2019-02-19 0:09 ` [lustre-devel] [PATCH 29/37] lustre: handles: discard h_owner in favour of h_ops NeilBrown
2019-02-27 6:37 ` Andreas Dilger
2019-02-27 21:41 ` NeilBrown
2019-02-28 6:41 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 33/37] lustre: portals_handle: remove locking from class_handle2object() NeilBrown
2019-02-19 0:09 ` [lustre-devel] [PATCH 34/37] lustre: portals_handle: use hlist for hash lists NeilBrown
2019-02-19 0:09 ` [lustre-devel] [PATCH 31/37] lustre: discard OBD_FREE_RCU NeilBrown
2019-02-19 0:09 ` [lustre-devel] [PATCH 24/37] lustre: lu_object: remove extra newline from debug printing NeilBrown
2019-02-24 19:08 ` James Simmons
2019-02-25 18:16 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 35/37] lustre: portals_handle: discard h_lock NeilBrown
2019-02-19 0:09 ` [lustre-devel] [PATCH 37/37] lustre: obd_sysfs: error-check value stored in jobid_var NeilBrown
2019-02-27 6:17 ` Andreas Dilger
2019-03-01 2:35 ` NeilBrown
2019-03-01 8:32 ` Andreas Dilger
2019-03-01 14:30 ` Patrick Farrell
2019-03-14 0:34 ` NeilBrown
2019-03-14 14:12 ` Patrick Farrell
2019-03-14 22:56 ` NeilBrown
2019-03-14 23:05 ` Andreas Dilger
2019-02-19 0:09 ` [lustre-devel] [PATCH 36/37] lustre: remove unused fields from struct obd_device NeilBrown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D8BE4529-E995-43CA-87F6-31F8FF52FB05@whamcloud.com \
--to=adilger@whamcloud.com \
--cc=lustre-devel@lists.lustre.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).