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...
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
