On 2014/12/16, 2:41 AM, "Dan Carpenter" <dan.carpenter@xxxxxxxxxx> wrote: >On Mon, Dec 15, 2014 at 11:41:49PM -0600, Chris Rorvick wrote: >> 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. >> > >That introduces a bug. We need to take the initial '-' into >consideration. Just remove the condition. Also remove the "mult" >parameter since that is always 1. > > bool negative = false; > > ... > > if (*pbuf == '-') { > negative = true; > pbuf++; > } > > ... > > mult = negative ? -units : units; Sorry, that isn't right. Chris' patch is actually doing the right thing to check for units > 1. The proposed change above discards "mult" entirely, which breaks the users of this function that are not in this file (e.g. osc_cached_mb_seq_write() or ll_max_cached_mb_seq_write()) that have tunables in units of MB by default, but can also use parameters with units like "4.5G" for convenience. Cheers, Andreas -- Andreas Dilger Lustre Software Architect Intel High Performance Data Division _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel