On Tue, Mar 06, 2012 at 09:35:08PM +0100, Jan Kara wrote: > On Mon 05-03-12 13:31:26, Wu Fengguang wrote: > > On Mon, Mar 05, 2012 at 04:06:57PM +0100, Jan Kara wrote: > > > On Thu 01-03-12 13:39:25, Greg KH wrote: > > > > 2.6.32-longterm review patch. If anyone has any objections, please let > > > > me know. > > > Not that I'd see anything wrong with this patch for 2.6.32. But it is > > > also unnecessary since the code which was triggering the overflow does not > > > exist in 2.6.32. So maybe just on the grounds of not applying unneeded > > > patchs I'd skip this one. > > > > FYI I never see this divide error for pre-3.2 kernels. However I've > > run into problem (2) before 3.2 which makes bdi dirty threshold go > > wild. So it seems safer to go with this patch. > > > > To be frank the boxes that run into bugs (1) or (2) do not have > > Terabytes of memory to create the big shift value in > > calc_period_shift() which is the sufficient condition for triggering > > the bugs as described in the below changelog. However the bugs do > > magically go away with the patch applied. Perhaps this patch breaks > > one necessary condition for triggering the bugs in a small memory box. > The patch went in -stable kernel so this is mostly an academic discussion > but Documentation/stable_kernel_rules.txt says among other things: > - It must fix a real bug that bothers people (not a, "This could be a > problem..." type thing). > - It must fix a problem that causes a build error (but not for things > marked CONFIG_BROKEN), an oops, a hang, data corruption, a real > security issue, or some "oh, that's not good" issue. In short, > something critical. > > This patch simply didn't pass these two conditions for me for 2.6.32 and > your arguments didn't convince me it's a critical thing either...
I see the point. We need *demonstrated* critical bugs for pushing the fix to -stable. By this criterion, I agree that there are no strong reasons for including this patch in -stable. Thanks, Fengguang > Honza > > > > > ------------------ > > > > > > > > From: Wu Fengguang <[email protected]> > > > > > > > > commit 3310225dfc71a35a2cc9340c15c0e08b14b3c754 upstream. > > > > > > > > PROP_MAX_SHIFT should be set to <=32 on 64-bit box. This fixes two bugs > > > > in the below lines of bdi_dirty_limit(): > > > > > > > > bdi_dirty *= numerator; > > > > do_div(bdi_dirty, denominator); > > > > > > > > 1) divide error: do_div() only uses the lower 32 bit of the denominator, > > > > which may trimmed to be 0 when PROP_MAX_SHIFT > 32. > > > > > > > > 2) overflow: (bdi_dirty * numerator) could easily overflow if numerator > > > > used up to 48 bits, leaving only 16 bits to bdi_dirty > > > > > > > > Cc: Peter Zijlstra <[email protected]> > > > > Reported-by: Ilya Tumaykin <[email protected]> > > > > Tested-by: Ilya Tumaykin <[email protected]> > > > > Signed-off-by: Wu Fengguang <[email protected]> > > > > Signed-off-by: Greg Kroah-Hartman <[email protected]> > > > > > > > > --- > > > > include/linux/proportions.h | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > --- a/include/linux/proportions.h > > > > +++ b/include/linux/proportions.h > > > > @@ -81,7 +81,11 @@ void prop_inc_percpu(struct prop_descrip > > > > * Limit the time part in order to ensure there are some bits left for > > > > the > > > > * cycle counter and fraction multiply. > > > > */ > > > > +#if BITS_PER_LONG == 32 > > > > #define PROP_MAX_SHIFT (3*BITS_PER_LONG/4) > > > > +#else > > > > +#define PROP_MAX_SHIFT (BITS_PER_LONG/2) > > > > +#endif > > > > > > > > #define PROP_FRAC_SHIFT (BITS_PER_LONG - PROP_MAX_SHIFT > > > > - 1) > > > > #define PROP_FRAC_BASE (1UL << PROP_FRAC_SHIFT) > > > > > > > > > > > > -- > > > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" > > > > in > > > > the body of a message to [email protected] > > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > Please read the FAQ at http://www.tux.org/lkml/ > > > -- > > > Jan Kara <[email protected]> > > > SUSE Labs, CR > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to [email protected] > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > -- > Jan Kara <[email protected]> > SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
