From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ew0-f214.google.com (mail-ew0-f214.google.com [209.85.219.214]) by ozlabs.org (Postfix) with ESMTP id 67A11B7B8B for ; Tue, 13 Oct 2009 13:26:44 +1100 (EST) Received: by ewy10 with SMTP id 10so9827697ewy.9 for ; Mon, 12 Oct 2009 19:26:41 -0700 (PDT) Date: Tue, 13 Oct 2009 11:18:18 +0900 From: Akinobu Mita To: Andrew Morton Subject: Re: [PATCH 2/8] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area Message-ID: <20091013021818.GA3898@localhost.localdomain> References: <1255076961-21325-1-git-send-email-akinobu.mita@gmail.com> <1255076961-21325-2-git-send-email-akinobu.mita@gmail.com> <20091009164100.85a36188.akpm@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20091009164100.85a36188.akpm@linux-foundation.org> Cc: Fenghua Yu , Greg Kroah-Hartman , linux-ia64@vger.kernel.org, Tony Luck , x86@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-altix@sgi.com, Yevgeny Petrilin , FUJITA Tomonori , linuxppc-dev@ozlabs.org, Ingo Molnar , Paul Mackerras , "H. Peter Anvin" , sparclinux@vger.kernel.org, Thomas Gleixner , linux-usb@vger.kernel.org, "David S. Miller" , Lothar Wassmann List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Fri, Oct 09, 2009 at 04:41:00PM -0700, Andrew Morton wrote: > On Fri, 9 Oct 2009 17:29:15 +0900 > Akinobu Mita wrote: > > > This introduces new bitmap functions: > > > > bitmap_set: Set specified bit area > > bitmap_clear: Clear specified bit area > > bitmap_find_next_zero_area: Find free bit area > > > > These are stolen from iommu helper. > > > > I changed the return value of bitmap_find_next_zero_area if there is > > no zero area. > > > > find_next_zero_area in iommu helper: returns -1 > > bitmap_find_next_zero_area: return >= bitmap size > > I'll plan to merge this patch into 2.6.32 so we can trickle all the > other patches into subsystems in an orderly fashion. Sounds good. > > +void bitmap_set(unsigned long *map, int i, int len) > > +{ > > + int end = i + len; > > + > > + while (i < end) { > > + __set_bit(i, map); > > + i++; > > + } > > +} > > This is really inefficient, isn't it? It's a pretty trivial matter to > romp through memory 32 or 64 bits at a time. OK. I'll do > > +unsigned long bitmap_find_next_zero_area(unsigned long *map, > > + unsigned long size, > > + unsigned long start, > > + unsigned int nr, > > + unsigned long align_mask) > > +{ > > + unsigned long index, end, i; > > +again: > > + index = find_next_zero_bit(map, size, start); > > + > > + /* Align allocation */ > > + index = (index + align_mask) & ~align_mask; > > + > > + end = index + nr; > > + if (end >= size) > > + return end; > > + i = find_next_bit(map, end, index); > > + if (i < end) { > > + start = i + 1; > > + goto again; > > + } > > + return index; > > +} > > +EXPORT_SYMBOL(bitmap_find_next_zero_area); > > This needs documentation, please. It appears that `size' is the size > of the bitmap and `nr' is the number of zeroed bits we're looking for, > but an inattentive programmer could get those reversed. > > Also the semantics of `align_mask' could benefit from spelling out. Is > the alignment with respect to memory boundaries or with respect to > `map' or with respect to map+start or what? OK. I will document it. And I plan to change bitmap_find_next_zero_area() to take the alignment instead of an align_mask as Roland said. > And why does align_mask exist at all? I was a bit surprised to see it > there. In which scenarios will it be non-zero? Because the users of iommu-helper and mlx4 need the alignment requirement for the zero area. arch/powerpc/kernel/iommu.c arch/x86/kernel/amd_iommu.c arch/x86/kernel/pci-gart_64.c drivers/net/mlx4/alloc.c