* [PATCH - V2] Fix missing of last user while dumping slab corruption log @ 2010-04-12 5:50 ShiYong LI 2010-04-13 7:05 ` TAO HU 2010-04-14 17:56 ` Pekka Enberg 0 siblings, 2 replies; 5+ messages in thread From: ShiYong LI @ 2010-04-12 5:50 UTC (permalink / raw) To: linux-kernel; +Cc: Pekka Enberg, cl, mpm, linux-mm, dwmw2, TAO HU Hi, Compared to previous version, add alignment checking to make sure memory space storing redzone2 and last user tags is 8 byte alignment. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH - V2] Fix missing of last user while dumping slab corruption log 2010-04-12 5:50 [PATCH - V2] Fix missing of last user while dumping slab corruption log ShiYong LI @ 2010-04-13 7:05 ` TAO HU 2010-04-13 9:32 ` Pekka Enberg 2010-04-14 17:56 ` Pekka Enberg 1 sibling, 1 reply; 5+ messages in thread From: TAO HU @ 2010-04-13 7:05 UTC (permalink / raw) To: Pekka Enberg; +Cc: linux-kernel, cl, mpm, linux-mm, dwmw2, TAO HU, ShiYong LI Hi, Pekka Enberg Actually we greped "kmem_cache_create" in whole kernel souce tree (2.6.29 and 2.6.32). Either "align" equal to "0" or flag SLAB_HWCACHE_ALIGN is used when calling kmem_cache_create(). Seems all of arch's cache-line-size is multiple of 64-bit/8-byte (sizeof(long long)) except arch-microblaze (4-byte). The smallest (except arch-microblaze) cache-line-size is 2^4= 16-byte as I can see. So even considering possible sizeof(long long) == 128-bit/16-byte, it is still safe to apply Shiyong's original version. Anyway, Shiyong's new patch check the weired situation that "align > sizeof(long long) && align is NOT multiple of sizeof (long long)" Let us know whether the new version address your concerns. -- Best Regards Hu Tao On Mon, Apr 12, 2010 at 1:50 PM, ShiYong LI <shi-yong.li@motorola.com> wrote: > Hi, > > Compared to previous version, add alignment checking to make sure > memory space storing redzone2 and last user tags is 8 byte alignment. > > From 949e8c29e8681a2359e23a8fbd8b9d4833f42344 Mon Sep 17 00:00:00 2001 > From: Shiyong Li <shi-yong.li@motorola.com> > Date: Mon, 12 Apr 2010 13:48:21 +0800 > Subject: [PATCH] Fix missing of last user info while getting > DEBUG_SLAB config enabled. > > Even with SLAB_RED_ZONE and SLAB_STORE_USER enabled, kernel would NOT > store redzone and last user data around allocated memory space if arch > cache line > sizeof(unsigned long long). As a result, last user information > is unexpectedly MISSED while dumping slab corruption log. > > This fix makes sure that redzone and last user tags get stored unless > the required alignment breaks redzone's. > > Signed-off-by: Shiyong Li <shi-yong.li@motorola.com> > --- > mm/slab.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/mm/slab.c b/mm/slab.c > index a8a38ca..b97c57e 100644 > --- a/mm/slab.c > +++ b/mm/slab.c > @@ -2267,8 +2267,8 @@ kmem_cache_create (const char *name, size_t > size, size_t align, > if (ralign < align) { > ralign = align; > } > - /* disable debug if necessary */ > - if (ralign > __alignof__(unsigned long long)) > + /* disable debug if not aligning with REDZONE_ALIGN */ > + if (ralign & (__alignof__(unsigned long long) - 1)) > flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER); > /* > * 4) Store it. > @@ -2289,8 +2289,8 @@ kmem_cache_create (const char *name, size_t > size, size_t align, > */ > if (flags & SLAB_RED_ZONE) { > /* add space for red zone words */ > - cachep->obj_offset += sizeof(unsigned long long); > - size += 2 * sizeof(unsigned long long); > + cachep->obj_offset += align; > + size += align + sizeof(unsigned long long); > } > if (flags & SLAB_STORE_USER) { > /* user store requires one word storage behind the end of > -- > 1.6.0.4 > > > > > > -- > Thanks & Best Regards > Shiyong > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH - V2] Fix missing of last user while dumping slab corruption log 2010-04-13 7:05 ` TAO HU @ 2010-04-13 9:32 ` Pekka Enberg 0 siblings, 0 replies; 5+ messages in thread From: Pekka Enberg @ 2010-04-13 9:32 UTC (permalink / raw) To: TAO HU, Nick Piggin Cc: linux-kernel, cl, mpm, linux-mm, dwmw2, TAO HU, ShiYong LI, david.rientjes TAO HU kirjoitti: > Actually we greped "kmem_cache_create" in whole kernel souce tree > (2.6.29 and 2.6.32). > > Either "align" equal to "0" or flag SLAB_HWCACHE_ALIGN is used when > calling kmem_cache_create(). I don't think that's correct. The "task_xstate" has alignof(struct task_xstate) and there seems to be so GCC attributes that force non-default alignment on the struct. > Seems all of arch's cache-line-size is multiple of 64-bit/8-byte > (sizeof(long long)) except arch-microblaze (4-byte). > The smallest (except arch-microblaze) cache-line-size is 2^4= 16-byte > as I can see. > So even considering possible sizeof(long long) == 128-bit/16-byte, it > is still safe to apply Shiyong's original version. > > Anyway, Shiyong's new patch check the weired situation that "align > > sizeof(long long) && align is NOT multiple of sizeof (long long)" > Let us know whether the new version address your concerns. Yeah, sorry for dragging this issue on. I've been looking at the patch but haven't been able to convince myself that it's correct. Nick, David, Christoph, Matt, could you also please take a look at this? Pekka -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH - V2] Fix missing of last user while dumping slab corruption log 2010-04-12 5:50 [PATCH - V2] Fix missing of last user while dumping slab corruption log ShiYong LI 2010-04-13 7:05 ` TAO HU @ 2010-04-14 17:56 ` Pekka Enberg 2010-04-15 3:04 ` TAO HU 1 sibling, 1 reply; 5+ messages in thread From: Pekka Enberg @ 2010-04-14 17:56 UTC (permalink / raw) To: ShiYong LI; +Cc: linux-kernel, cl, mpm, linux-mm, dwmw2, TAO HU ShiYong LI wrote: > Hi, > > Compared to previous version, add alignment checking to make sure > memory space storing redzone2 and last user tags is 8 byte alignment. > > From 949e8c29e8681a2359e23a8fbd8b9d4833f42344 Mon Sep 17 00:00:00 2001 > From: Shiyong Li <shi-yong.li@motorola.com> > Date: Mon, 12 Apr 2010 13:48:21 +0800 > Subject: [PATCH] Fix missing of last user info while getting > DEBUG_SLAB config enabled. > > Even with SLAB_RED_ZONE and SLAB_STORE_USER enabled, kernel would NOT > store redzone and last user data around allocated memory space if arch > cache line > sizeof(unsigned long long). As a result, last user information > is unexpectedly MISSED while dumping slab corruption log. > > This fix makes sure that redzone and last user tags get stored unless > the required alignment breaks redzone's. > > Signed-off-by: Shiyong Li <shi-yong.li@motorola.com> OK, I added this to linux-next for testing. Thanks! > --- > mm/slab.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/mm/slab.c b/mm/slab.c > index a8a38ca..b97c57e 100644 > --- a/mm/slab.c > +++ b/mm/slab.c > @@ -2267,8 +2267,8 @@ kmem_cache_create (const char *name, size_t > size, size_t align, > if (ralign < align) { > ralign = align; > } > - /* disable debug if necessary */ > - if (ralign > __alignof__(unsigned long long)) > + /* disable debug if not aligning with REDZONE_ALIGN */ > + if (ralign & (__alignof__(unsigned long long) - 1)) > flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER); > /* > * 4) Store it. > @@ -2289,8 +2289,8 @@ kmem_cache_create (const char *name, size_t > size, size_t align, > */ > if (flags & SLAB_RED_ZONE) { > /* add space for red zone words */ > - cachep->obj_offset += sizeof(unsigned long long); > - size += 2 * sizeof(unsigned long long); > + cachep->obj_offset += align; > + size += align + sizeof(unsigned long long); > } > if (flags & SLAB_STORE_USER) { > /* user store requires one word storage behind the end of -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH - V2] Fix missing of last user while dumping slab corruption log 2010-04-14 17:56 ` Pekka Enberg @ 2010-04-15 3:04 ` TAO HU 0 siblings, 0 replies; 5+ messages in thread From: TAO HU @ 2010-04-15 3:04 UTC (permalink / raw) To: Pekka Enberg; +Cc: ShiYong LI, linux-kernel, cl, mpm, linux-mm, dwmw2, TAO HU Hi, Pekka Enberg Thanks! If we hadn't seen the last user info, we would not have fixed a difficult bug in our system. So very glad to know. -- Best Regards Hu Tao On Thu, Apr 15, 2010 at 1:56 AM, Pekka Enberg <penberg@cs.helsinki.fi> wrote: > ShiYong LI wrote: >> >> Hi, >> >> Compared to previous version, add alignment checking to make sure >> memory space storing redzone2 and last user tags is 8 byte alignment. >> >> From 949e8c29e8681a2359e23a8fbd8b9d4833f42344 Mon Sep 17 00:00:00 2001 >> From: Shiyong Li <shi-yong.li@motorola.com> >> Date: Mon, 12 Apr 2010 13:48:21 +0800 >> Subject: [PATCH] Fix missing of last user info while getting >> DEBUG_SLAB config enabled. >> >> Even with SLAB_RED_ZONE and SLAB_STORE_USER enabled, kernel would NOT >> store redzone and last user data around allocated memory space if arch >> cache line > sizeof(unsigned long long). As a result, last user >> information >> is unexpectedly MISSED while dumping slab corruption log. >> >> This fix makes sure that redzone and last user tags get stored unless >> the required alignment breaks redzone's. >> >> Signed-off-by: Shiyong Li <shi-yong.li@motorola.com> > > OK, I added this to linux-next for testing. Thanks! > >> --- >> mm/slab.c | 8 ++++---- >> 1 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/mm/slab.c b/mm/slab.c >> index a8a38ca..b97c57e 100644 >> --- a/mm/slab.c >> +++ b/mm/slab.c >> @@ -2267,8 +2267,8 @@ kmem_cache_create (const char *name, size_t >> size, size_t align, >> if (ralign < align) { >> ralign = align; >> } >> - /* disable debug if necessary */ >> - if (ralign > __alignof__(unsigned long long)) >> + /* disable debug if not aligning with REDZONE_ALIGN */ >> + if (ralign & (__alignof__(unsigned long long) - 1)) >> flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER); >> /* >> * 4) Store it. >> @@ -2289,8 +2289,8 @@ kmem_cache_create (const char *name, size_t >> size, size_t align, >> */ >> if (flags & SLAB_RED_ZONE) { >> /* add space for red zone words */ >> - cachep->obj_offset += sizeof(unsigned long long); >> - size += 2 * sizeof(unsigned long long); >> + cachep->obj_offset += align; >> + size += align + sizeof(unsigned long long); >> } >> if (flags & SLAB_STORE_USER) { >> /* user store requires one word storage behind the end of > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-04-15 3:04 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-04-12 5:50 [PATCH - V2] Fix missing of last user while dumping slab corruption log ShiYong LI 2010-04-13 7:05 ` TAO HU 2010-04-13 9:32 ` Pekka Enberg 2010-04-14 17:56 ` Pekka Enberg 2010-04-15 3:04 ` TAO HU
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).