From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751461AbZH1DcB (ORCPT ); Thu, 27 Aug 2009 23:32:01 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751206AbZH1DcA (ORCPT ); Thu, 27 Aug 2009 23:32:00 -0400 Received: from mga03.intel.com ([143.182.124.21]:48497 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750933AbZH1DcA (ORCPT ); Thu, 27 Aug 2009 23:32:00 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.44,271,1249282800"; d="scan'208";a="181324443" Date: Fri, 28 Aug 2009 11:31:49 +0800 From: Wu Fengguang To: "Yan, Zheng " Cc: Zhu Yanhai , Andrew Morton , Jiri Kosina , Huang Shijie , "linux-kernel@vger.kernel.org" , Zhu Yanhai , Nick Piggin Subject: Re: [PATCH] Make radix_tree_preload alloc one more slot Message-ID: <20090828033149.GA24838@localhost> References: <4A9677C1.3090804@gmail.com> <20090827122144.GA20443@localhost> <3d0408630908270946n7302e3c5u94f133d8ebbe8c36@mail.gmail.com> <20090828015032.GA13271@localhost> <3d0408630908271933v1d136b85he33e7d4b6e0bccf6@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <3d0408630908271933v1d136b85he33e7d4b6e0bccf6@mail.gmail.com> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Aug 28, 2009 at 10:33:03AM +0800, Yan, Zheng wrote: > 2009/8/28 Wu Fengguang : > > On Fri, Aug 28, 2009 at 12:46:46AM +0800, Yan, Zheng  wrote: > >> 2009/8/27 Wu Fengguang : > >> > > >> > Hi Yanhai, > >> > > >> > [Nick CCed] > >> > > >> > On Thu, Aug 27, 2009 at 08:10:41PM +0800, Zhu Yanhai wrote: > >> >> The operations against radix tree always use paths with RADIX_TREE_MAX_PATH > >> >> + 1 slots, but radix_tree_preload only pre-allocs RADIX_TREE_MAX_PATH > >> >> slots at present, which causes radix_tree_node_alloc tries to do > >> >> kmem_cache_alloc at the last slot even if we don't have gfp_mask & > >> >> __GFP_WAIT in hand. > >> > > >> > Are you sure?  The comments read: > >> > > >> >        /* > >> >         * The radix tree path needs to be one longer than the maximum path > >> >         * since the "list" is null terminated. > >> >         */ > >> >        struct radix_tree_path path[RADIX_TREE_MAX_PATH + 1], *pathp = path; > >> > > >> > Thanks, > >> > Fengguang > >> > > >> > > >> >> Signed-off-by: Zhu Yanhai > >> >> > >> >> --- > >> >>  lib/radix-tree.c |    2 +- > >> >>  1 files changed, 1 insertions(+), 1 deletions(-) > >> >> > >> >> diff --git a/lib/radix-tree.c b/lib/radix-tree.c > >> >> index 23abbd9..72225a8 100644 > >> >> --- a/lib/radix-tree.c > >> >> +++ b/lib/radix-tree.c > >> >> @@ -79,7 +79,7 @@ static struct kmem_cache *radix_tree_node_cachep; > >> >>   */ > >> >>  struct radix_tree_preload { > >> >>       int nr; > >> >> -     struct radix_tree_node *nodes[RADIX_TREE_MAX_PATH]; > >> >> +     struct radix_tree_node *nodes[RADIX_TREE_MAX_PATH + 1]; > >> >>  }; > >> >>  static DEFINE_PER_CPU(struct radix_tree_preload, radix_tree_preloads) = { 0, }; > >> >> > >> >> -- > >> >> 1.6.2.2 > >> > >> here is test case. > >> --- > >> #include > >> #include > >> #include > >> > >> static void __exit exit_test(void) > >> { > >> } > >> > >> static int __init init_test(void) > >> { > >>         struct radix_tree_root radix_tree; > >>         int foo; > >> > >>         INIT_RADIX_TREE(&radix_tree, GFP_KERNEL); > >>         radix_tree_preload(GFP_KERNEL); > >>         preempt_disable(); > >>         radix_tree_insert(&radix_tree, (unsigned long)-2, &foo); > >>         preempt_enable(); > >>         radix_tree_preload_end(); > >>         radix_tree_delete(&radix_tree, (unsigned long)-2); > >>         return -1; > >> } > >> > >> module_init(init_test) > >> module_exit(exit_test) > >> MODULE_LICENSE("GPL"); > >> -- end -- > >> > >> I got following oops. > >> --- > >> BUG: sleeping function called from invalid context at > >> /home/zhyan/linux-2.6/mm/slub.c:1697 > >> in_atomic(): 1, irqs_disabled(): 0, pid: 2791, name: insmod > >> Pid: 2791, comm: insmod Not tainted 2.6.31-rc7 #1 > >> Call Trace: > >>  [] __might_sleep+0x101/0x108 > >>  [] kmem_cache_alloc+0x39/0x141 > >>  [] ? radix_tree_node_alloc+0x4c/0x5d > >>  [] radix_tree_node_alloc+0x4c/0x5d > >>  [] radix_tree_insert+0xc2/0x174 > >>  [] init_test+0x3f/0x89 [test] > >>  [] do_one_initcall+0x4f/0x11f > >>  [] ? init_test+0x0/0x89 [test] > >>  [] ? __blocking_notifier_call_chain+0x45/0x51 > >>  [] sys_init_module+0xac/0x1bc > >>  [] sysenter_do_call+0x12/0x2d > >> -- end -- > > This is weird, I cannot reproduce this message. Here is my .config. > > I use SLAB but it also has the might_sleep_if(__GFP_WAIT) debug call. > > Thanks, > > Fengguang > > I realize it's my fault, I use GFP_KERNEL to initialize the radix tree. > radix_tree_node_alloc does not use pre-loaded memory when the > radix tree is initialized with GFP_KERNEL. Yesterday when I saw the Ah yes! > Oops, I suspect it's an radix_tree_preload bug. I told Zhu Yanhai > my thought. Please forgive my rash act. That's all right. So we can just do nothing? :) Thanks, Fengguang