lkml.org 
[lkml]   [2012]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [ 03/34] lib: proportion: lower PROP_MAX_SHIFT to 32 on 64-bit kernel
    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 <fengguang.wu@intel.com>
    > > > >
    > > > > 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 <a.p.zijlstra@chello.nl>
    > > > > Reported-by: Ilya Tumaykin <librarian_rus@yahoo.com>
    > > > > Tested-by: Ilya Tumaykin <librarian_rus@yahoo.com>
    > > > > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
    > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    > > > >
    > > > > ---
    > > > > 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 majordomo@vger.kernel.org
    > > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > > > > Please read the FAQ at http://www.tux.org/lkml/
    > > > --
    > > > Jan Kara <jack@suse.cz>
    > > > SUSE Labs, CR
    > > --
    > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > > the body of a message to majordomo@vger.kernel.org
    > > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > > Please read the FAQ at http://www.tux.org/lkml/
    > --
    > Jan Kara <jack@suse.cz>
    > SUSE Labs, CR


    \
     
     \ /
      Last update: 2012-03-07 06:21    [W:2.288 / U:0.128 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site