SLOB breaks Crypto
in [Kernel]
|
Prev: [RELEASE] LTTng 0.214 for kernel 2.6.33.4
Next: vlynq: make whole Kconfig-menu dependant on architecture
From: Matt Mackall on 18 May 2010 15:40 On Tue, 2010-05-18 at 12:25 -0700, David Miller wrote: > From: Herbert Xu <herbert(a)gondor.apana.org.au> > Date: Tue, 18 May 2010 20:27:01 +1000 > > > I think the simplest fix is to revert this changeset. > > If you revert then you'll break sparc. > > Sparc needs long long alignment, so it's SLOB that needs to > change if it isn't providing at least that much alignment > by default. SLOB honors ARCH_KMALLOC_MINALIGN. If your arch has alignment requirements, I recommend you set it. -- Mathematics is the supreme nostalgia of our time. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: David Miller on 18 May 2010 17:00 From: Matt Mackall <mpm(a)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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Pekka Enberg on 18 May 2010 17:20 Hi David, On Tue, May 18, 2010 at 11:59 PM, David Miller <davem(a)davemloft.net> wrote: > From: Matt Mackall <mpm(a)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. Pekka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: David Miller on 18 May 2010 17:30 From: Pekka Enberg <penberg(a)cs.helsinki.fi> Date: Wed, 19 May 2010 00:15:46 +0300 > On Tue, May 18, 2010 at 11:59 PM, David Miller <davem(a)davemloft.net> wrote: >> From: Matt Mackall <mpm(a)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. It's already obvious from these reports that such dependencies do exist. I'll add the define for sparc, but saying "sparc's fault" is bogus because I defined what was necessary to get SLAB/SLUB to provide the necessary alignment. SLOB pays for choosing not to use the same calculations for minimum alignment as the other allocators, and therefore pays for being different in this regard. And in fact I do know that the ifdef'ery in SLAB/SLUB is derived from a change long ago that was specifically added to handle platforms like sparc. 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Paul Mundt on 18 May 2010 18:40
(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(a)cs.helsinki.fi> > > Date: Wed, 19 May 2010 00:15:46 +0300 > > > > > On Tue, May 18, 2010 at 11:59 PM, David Miller <davem(a)davemloft.net> wrote: > > >> From: Matt Mackall <mpm(a)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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |