From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755430Ab2GJBgW (ORCPT ); Mon, 9 Jul 2012 21:36:22 -0400 Received: from e23smtp05.au.ibm.com ([202.81.31.147]:36115 "EHLO e23smtp05.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755366Ab2GJBf6 (ORCPT ); Mon, 9 Jul 2012 21:35:58 -0400 Message-ID: <1341884144.2562.18.camel@ThinkPad-T420> Subject: Re: [PATCH SLAB 1/2 v3] duplicate the cache name in SLUB's saved_alias list, SLAB, and SLOB From: Li Zhong To: Christoph Lameter Cc: LKML , Pekka Enberg , Matt Mackall , Benjamin Herrenschmidt , Paul Mackerras , linux-mm , PowerPC email list , Wanlong Gao , Glauber Costa Date: Tue, 10 Jul 2012 09:35:44 +0800 In-Reply-To: References: <1341561286.24895.9.camel@ThinkPad-T420> <1341801721.2439.29.camel@ThinkPad-T420> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit Mime-Version: 1.0 x-cbid: 12070915-1396-0000-0000-00000188A03B Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2012-07-09 at 09:01 -0500, Christoph Lameter wrote: > > I was pointed by Glauber to the slab common code patches. I need some > > more time to read the patches. Now I think the slab/slot changes in this > > v3 are not needed, and can be ignored. > > That may take some kernel cycles. You have a current issue here that needs > to be fixed. I'm a little confused ... and what need I do for the next step? > > > > down_write(&slub_lock); > > > - s = find_mergeable(size, align, flags, name, ctor); > > > + s = find_mergeable(size, align, flags, n, ctor); > > > if (s) { > > > s->refcount++; > > > /* > > > > ...... > > up_write(&slub_lock); > > return s; > > } > > > > Here, the function returns without name string n be kfreed. > > That is intentional since the string n is still referenced by the entry > that sysfs_slab_alias has created. I'm not sure whether the "referenced by ..." you mentioned is what I understood. From my understanding: if slab_state == SYS_FS, after return sysfs_create_link(&slab_kset->kobj, &s->kobj, name); is called, the name string passed in sysfs_slab_alias is no longer referenced (sysfs_new_dirent duplicates the string for sysfs to use). else, the name sting is referenced by al->name = name; temporarily. After slab_sysfs_init is finished, the name is not referenced any more. So in my patch (slub part), the string is duplicated here, and kfreed in slab_sysfs_init. > > But we couldn't kfree n here, because in sysfs_slab_alias(), if > > (slab_state < SYS_FS), the name need to be kept valid until > > slab_sysfs_init() is finished adding the entry into sysfs. > > Right that is why it is not freed and that is what fixes the issue you > see. > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx198.postini.com [74.125.245.198]) by kanga.kvack.org (Postfix) with SMTP id 080316B0073 for ; Mon, 9 Jul 2012 21:36:12 -0400 (EDT) Received: from /spool/local by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 10 Jul 2012 02:22:35 +1000 Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q6A1Zm9S65536184 for ; Tue, 10 Jul 2012 11:35:51 +1000 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q6A1Zlqu015102 for ; Tue, 10 Jul 2012 11:35:48 +1000 Message-ID: <1341884144.2562.18.camel@ThinkPad-T420> Subject: Re: [PATCH SLAB 1/2 v3] duplicate the cache name in SLUB's saved_alias list, SLAB, and SLOB From: Li Zhong Date: Tue, 10 Jul 2012 09:35:44 +0800 In-Reply-To: References: <1341561286.24895.9.camel@ThinkPad-T420> <1341801721.2439.29.camel@ThinkPad-T420> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Mime-Version: 1.0 Sender: owner-linux-mm@kvack.org List-ID: To: Christoph Lameter Cc: LKML , Pekka Enberg , Matt Mackall , Benjamin Herrenschmidt , Paul Mackerras , linux-mm , PowerPC email list , Wanlong Gao , Glauber Costa On Mon, 2012-07-09 at 09:01 -0500, Christoph Lameter wrote: > > I was pointed by Glauber to the slab common code patches. I need some > > more time to read the patches. Now I think the slab/slot changes in this > > v3 are not needed, and can be ignored. > > That may take some kernel cycles. You have a current issue here that needs > to be fixed. I'm a little confused ... and what need I do for the next step? > > > > down_write(&slub_lock); > > > - s = find_mergeable(size, align, flags, name, ctor); > > > + s = find_mergeable(size, align, flags, n, ctor); > > > if (s) { > > > s->refcount++; > > > /* > > > > ...... > > up_write(&slub_lock); > > return s; > > } > > > > Here, the function returns without name string n be kfreed. > > That is intentional since the string n is still referenced by the entry > that sysfs_slab_alias has created. I'm not sure whether the "referenced by ..." you mentioned is what I understood. From my understanding: if slab_state == SYS_FS, after return sysfs_create_link(&slab_kset->kobj, &s->kobj, name); is called, the name string passed in sysfs_slab_alias is no longer referenced (sysfs_new_dirent duplicates the string for sysfs to use). else, the name sting is referenced by al->name = name; temporarily. After slab_sysfs_init is finished, the name is not referenced any more. So in my patch (slub part), the string is duplicated here, and kfreed in slab_sysfs_init. > > But we couldn't kfree n here, because in sysfs_slab_alias(), if > > (slab_state < SYS_FS), the name need to be kept valid until > > slab_sysfs_init() is finished adding the entry into sysfs. > > Right that is why it is not freed and that is what fixes the issue you > see. > -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e23smtp06.au.ibm.com (e23smtp06.au.ibm.com [202.81.31.148]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "e23smtp06.au.ibm.com", Issuer "GeoTrust SSL CA" (not verified)) by ozlabs.org (Postfix) with ESMTPS id A8C162C01FA for ; Tue, 10 Jul 2012 11:35:57 +1000 (EST) Received: from /spool/local by e23smtp06.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 10 Jul 2012 01:29:14 +1000 Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q6A1RnhT50266290 for ; Tue, 10 Jul 2012 11:27:50 +1000 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q6A1Zlr0015102 for ; Tue, 10 Jul 2012 11:35:48 +1000 Message-ID: <1341884144.2562.18.camel@ThinkPad-T420> Subject: Re: [PATCH SLAB 1/2 v3] duplicate the cache name in SLUB's saved_alias list, SLAB, and SLOB From: Li Zhong To: Christoph Lameter Date: Tue, 10 Jul 2012 09:35:44 +0800 In-Reply-To: References: <1341561286.24895.9.camel@ThinkPad-T420> <1341801721.2439.29.camel@ThinkPad-T420> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Cc: LKML , Glauber Costa , Pekka Enberg , linux-mm , Paul Mackerras , Matt Mackall , PowerPC email list , Wanlong Gao List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Mon, 2012-07-09 at 09:01 -0500, Christoph Lameter wrote: > > I was pointed by Glauber to the slab common code patches. I need some > > more time to read the patches. Now I think the slab/slot changes in this > > v3 are not needed, and can be ignored. > > That may take some kernel cycles. You have a current issue here that needs > to be fixed. I'm a little confused ... and what need I do for the next step? > > > > down_write(&slub_lock); > > > - s = find_mergeable(size, align, flags, name, ctor); > > > + s = find_mergeable(size, align, flags, n, ctor); > > > if (s) { > > > s->refcount++; > > > /* > > > > ...... > > up_write(&slub_lock); > > return s; > > } > > > > Here, the function returns without name string n be kfreed. > > That is intentional since the string n is still referenced by the entry > that sysfs_slab_alias has created. I'm not sure whether the "referenced by ..." you mentioned is what I understood. From my understanding: if slab_state == SYS_FS, after return sysfs_create_link(&slab_kset->kobj, &s->kobj, name); is called, the name string passed in sysfs_slab_alias is no longer referenced (sysfs_new_dirent duplicates the string for sysfs to use). else, the name sting is referenced by al->name = name; temporarily. After slab_sysfs_init is finished, the name is not referenced any more. So in my patch (slub part), the string is duplicated here, and kfreed in slab_sysfs_init. > > But we couldn't kfree n here, because in sysfs_slab_alias(), if > > (slab_state < SYS_FS), the name need to be kept valid until > > slab_sysfs_init() is finished adding the entry into sysfs. > > Right that is why it is not freed and that is what fixes the issue you > see. >