lkml.org 
[lkml]   [2010]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [BUG] SLOB breaks Crypto
    (adding Christoph and dwmw2 to the Cc..)

    On Wed, May 19, 2010 at 07:35:07AM +0900, Paul Mundt wrote:
    > On Tue, May 18, 2010 at 02:20:21PM -0700, David Miller wrote:
    > > From: Pekka Enberg <penberg@cs.helsinki.fi>
    > > Date: Wed, 19 May 2010 00:15:46 +0300
    > >
    > > > On Tue, May 18, 2010 at 11:59 PM, David Miller <davem@davemloft.net> wrote:
    > > >> From: Matt Mackall <mpm@selenic.com>
    > > >> Date: Tue, 18 May 2010 14:33:55 -0500
    > > >>
    > > >>> SLOB honors ARCH_KMALLOC_MINALIGN. If your arch has alignment
    > > >>> requirements, I recommend you set it.
    > > >>
    > > >> I recommend that the alignment provided by the allocator is not
    > > >> determined by which allocator I happen to have enabled.
    > > >>
    > > >> The values and ifdef'ery should be identical in all of our
    > > >> allocators.
    > > >
    > > > Why? It doesn't make much sense for SLOB, which tries to be as space
    > > > efficient as possible, as a default. If things break on sparc, it
    > > > really needs to set ARCH_KMALLOC_MINALIGN as slab default alignment is
    > > > not something you really want to depend on.
    > >
    > > I think it does make sense to expect that, whatever my architecture
    > > defines or does not define, I can expect the allocators to provide the
    > > same minimum alignment guarentee. Otherwise it is no guarantee at all.
    > >
    > SLAB/SLUB/SLOB all used to have the same BYTES_PER_WORD alignment
    > guarantee, with SLAB and SLUB having moved away from this to unsigned
    > long long in b46b8f19 and 47bfdc0d respectively. This was due to mixing
    > 64-bit integers in data structures, which in the SLAB case resulted in
    > misaligned structures and also broke redzoning (architecture overrides
    > also disabled it completely). The SLUB change was made a couple of days
    > earlier for the same structure misalignment reasons (64-bit integers on
    > 32-bit platforms).
    >
    > The default changes in SLAB/SLUB at least assume that 32-bit
    > architectures can only address 64-bit values on a 64-bit boundary. While
    > this is true for most cases, these have always been handled through the
    > bumping of the architecture minalign values in the past. Indeed, this was
    > the rationale I had for adding the architecture-specific slab minalign
    > override in the first place. The kmalloc one on the other hand is largely
    > just overriden for platforms with DMA requirements -- usually a
    > cacheline boundary.
    >
    > > It's already obvious from these reports that such dependencies do
    > > exist.
    > >
    > These dependencies were then introduced after SLAB/SLUB changed the
    > rules, suggesting that not enough testing was done.
    >
    > > So one of two things should happen:
    > >
    > > 1) SLOB conforms to SLAB/SLUB in it's test
    > >
    > > 2) SLAB/SLUB conforms to SLOB in it's test
    > >
    > > And yes this is an either-or, you can't say they are both valid.
    >
    > I don't see any reason to punish SLOB for the assumptions that SLAB/SLUB
    > arbitrarily took up, presumably on an architecture that should have
    > specified its own alignment requirements and simply couldn't be bothered.
    > Making SLAB redzoning work with arbitrary alignment is another matter
    > entirely, and something that should probably be revisited.
    >
    > Anything that assumes more than BYTES_PER_WORD is simply broken and
    > should be reverted.


    \
     
     \ /
      Last update: 2010-05-19 00:41    [W:2.652 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site