From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755976Ab3EVMBh (ORCPT ); Wed, 22 May 2013 08:01:37 -0400 Received: from mga09.intel.com ([134.134.136.24]:40469 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754799Ab3EVMBf (ORCPT ); Wed, 22 May 2013 08:01:35 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.87,719,1363158000"; d="scan'208";a="317704327" From: "Kirill A. Shutemov" To: Dave Hansen Cc: "Kirill A. Shutemov" , Andrea Arcangeli , Andrew Morton , Al Viro , Hugh Dickins , Wu Fengguang , Jan Kara , Mel Gorman , linux-mm@kvack.org, Andi Kleen , Matthew Wilcox , "Kirill A. Shutemov" , Hillf Danton , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org In-Reply-To: <519BC3BE.3070702@sr71.net> References: <1368321816-17719-1-git-send-email-kirill.shutemov@linux.intel.com> <1368321816-17719-5-git-send-email-kirill.shutemov@linux.intel.com> <519BC3BE.3070702@sr71.net> Subject: Re: [PATCHv4 04/39] radix-tree: implement preload for multiple contiguous elements Content-Transfer-Encoding: 7bit Message-Id: <20130522120356.9CB12E0090@blue.fi.intel.com> Date: Wed, 22 May 2013 15:03:56 +0300 (EEST) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Dave Hansen wrote: > On 05/11/2013 06:23 PM, Kirill A. Shutemov wrote: > > This patch introduces radix_tree_preload_count(). It allows to > > preallocate nodes enough to insert a number of *contiguous* elements. > > Would radix_tree_preload_contig() be a better name, then? Yes. Will rename. > ... > > On 64-bit system: > > For RADIX_TREE_MAP_SHIFT=3, old array size is 43, new is 107. > > For RADIX_TREE_MAP_SHIFT=4, old array size is 31, new is 63. > > For RADIX_TREE_MAP_SHIFT=6, old array size is 21, new is 30. > > > > On 32-bit system: > > For RADIX_TREE_MAP_SHIFT=3, old array size is 21, new is 84. > > For RADIX_TREE_MAP_SHIFT=4, old array size is 15, new is 46. > > For RADIX_TREE_MAP_SHIFT=6, old array size is 11, new is 19. > > > > On most machines we will have RADIX_TREE_MAP_SHIFT=6. > > Thanks for adding that to the description. The array you're talking > about is just pointers, right? > > 107-43 = 64. So, we have 64 extra pointers * NR_CPUS, plus 64 extra > radix tree nodes that we will keep around most of the time. On x86_64, > that's 512 bytes plus 64*560 bytes of nodes which is ~35k of memory per CPU. > > That's not bad I guess, but I do bet it's something that some folks want > to configure out. Please make sure to call out the actual size cost in > bytes per CPU in future patch postings, at least for the common case > (64-bit non-CONFIG_BASE_SMALL). I will add this to the commit message: On most machines we will have RADIX_TREE_MAP_SHIFT=6. In this case, on 64-bit system the per-CPU feature overhead is for preload array: (30 - 21) * sizeof(void*) = 72 bytes plus, if the preload array is full (30 - 21) * sizeof(struct radix_tree_node) = 9 * 560 = 5040 bytes total: 5112 bytes on 32-bit system the per-CPU feature overhead is for preload array: (19 - 11) * sizeof(void*) = 32 bytes plus, if the preload array is full (19 - 11) * sizeof(struct radix_tree_node) = 8 * 296 = 2368 bytes total: 2400 bytes --- Is it good enough? I probably, will add !BASE_SMALL dependency to TRANSPARENT_HUGEPAGE_PAGECACHE config option. > > > Since only THP uses batched preload at the , we disable (set max preload > > to 1) it if !CONFIG_TRANSPARENT_HUGEPAGE_PAGECACHE. This can be changed > > in the future. > > "at the..." Is there something missing in that sentence? at the moment :) > No major nits, so: > > Acked-by: Dave Hansen Thanks! -- Kirill A. Shutemov