On Tue, Dec 16, 2014 at 5:35 AM, Dan Carpenter <dan.carpen...@oracle.com> wrote: > On Tue, Dec 16, 2014 at 11:14:35AM +0000, Dilger, Andreas wrote: > > > > Sorry, that isn't right. Chris' patch is actually doing the right thing > > to check for units > 1. > > It's not right because it discards the negative.
I don't think this patch introduces a bug. If anything, it was already there. It looked to me like the value passed in to `mult' was assumed to be positive and was simply being used as a flag to indicate whether `buffer' started with a '-' when units were passed. For example, say the value passed in is "-2K" and the `mult' is 1. The check for '-' will negate `mult' making it -1. Then the units conditional will override mult with `-units' (i.e., -1024.) Now say we pass "-2" with `mult' equal to 1024. The result is same, but the path is a bit different. `mult' will again be negated due to `buffer' beginning with '-', but then it will be left alone at the units check. In both of the above cases the negative sign is properly accounted for. > > 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. > > I think you are confusing lprocfs_write_frac_helper() and > lprocfs_write_frac_u64_helper(). There is only one caller for this > function. By this logic, lprocfs_write_frac_u64_helper() should just be removed and it's code should be folded into lprocfs_write_u64_helper(), no? Regards, Chris _______________________________________________ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
