Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755328AbYAFKlX (ORCPT ); Sun, 6 Jan 2008 05:41:23 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752815AbYAFKlN (ORCPT ); Sun, 6 Jan 2008 05:41:13 -0500 Received: from py-out-1112.google.com ([64.233.166.182]:6843 "EHLO py-out-1112.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751950AbYAFKlL (ORCPT ); Sun, 6 Jan 2008 05:41:11 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=googlemail.com; s=gamma; h=message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=wfln1X4s7Dmx5dkELrAbr5Xl7MTPnCZTYxOdekEB+yk0xzI2BwYU6XJ4fNFYf52wMHpceOz0vqSab8799pkCvYzaY1KUbL2JhGh81gJttorSW5XYhbB1xj1L7VobUyuZDMGmMqaxynlq1UlzdjCj9eKll2KDYWn6CL5lnuq1pWM= Message-ID: <64bb37e0801060241o2c6d8172r73b69291fce76ce1@mail.gmail.com> Date: Sun, 6 Jan 2008 11:41:10 +0100 From: "Torsten Kaiser" To: "FUJITA Tomonori" Subject: Re: 2.6.24-rc6-mm1 Cc: akpm@linux-foundation.org, jarkao2@gmail.com, herbert@gondor.apana.org.au, linux-kernel@vger.kernel.org, neilb@suse.de, bfields@fieldses.org, netdev@vger.kernel.org, tom@opengridcomputing.com, fujita.tomonori@lab.ntt.co.jp In-Reply-To: <20080106123103K.tomof@acm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <64bb37e0801050652t7568e438uf93208601df84ef6@mail.gmail.com> <64bb37e0801051410p39746295oab4dad438943372@mail.gmail.com> <20080105172524.fecb63bf.akpm@linux-foundation.org> <20080106123103K.tomof@acm.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4093 Lines: 108 On Jan 6, 2008 4:28 AM, FUJITA Tomonori wrote: > On Sat, 5 Jan 2008 17:25:24 -0800 > Andrew Morton wrote: > > On Sat, 5 Jan 2008 23:10:17 +0100 "Torsten Kaiser" wrote: > > > But the cause of my mail is the following question: > > > Regarding my "iommu-sg-merging-patches are new in -rc3-mm and could be > > > the cause"-suspicion I looked at these patches and came across these > > > hunks: > > > > > > This is removed from arch/x86/lib/bitstr_64.c: > > > -/* Find string of zero bits in a bitmap */ > > > -unsigned long > > > -find_next_zero_string(unsigned long *bitmap, long start, long nbits, int len) > > > -{ > > > - unsigned long n, end, i; > > > - > > > - again: > > > - n = find_next_zero_bit(bitmap, nbits, start); > > > - if (n == -1) > > > - return -1; > > > - > > > - /* could test bitsliced, but it's hardly worth it */ > > > - end = n+len; > > > - if (end > nbits) > > > - return -1; > > > - for (i = n+1; i < end; i++) { > > > - if (test_bit(i, bitmap)) { > > > - start = i+1; > > > - goto again; > > > - } > > > - } > > > - return n; > > > -} > > > > > > This is added to lib/iommu-helper.c: > > > +static unsigned long find_next_zero_area(unsigned long *map, > > > + unsigned long size, > > > + unsigned long start, > > > + unsigned int nr) > > > +{ > > > + unsigned long index, end, i; > > > +again: > > > + index = find_next_zero_bit(map, size, start); > > > + end = index + nr; > > > + if (end > size) > > > + return -1; > > > + for (i = index + 1; i < end; i++) { > > > + if (test_bit(i, map)) { > > > + start = i+1; > > > + goto again; > > > + } > > > + } > > > + return index; > > > +} > > > > > > The old version checks, if find_next_zero_bit returns -1, the new > > > version doesn't do this. > > > Is this intended and can find_next_zero_bit never fail? > > > Hmm... but in the worst case it should only loop forever if I'm > > > reading this right (index = -1 => for-loop counts from 0 to nr, if any > > > bit is set it will jump to "again:" and if the next call to > > > find_next_zero_bit also fails, its an endless loop) > > find_next_zero_bit returns -1? > > It seems that x86_64 doesn't. I'm sorry. I didn't look into find_next_zero_bit, I only noted that the old version did check for -1 and the new one didn't. Obviously the old check was superfluous. > POWER and SPARC64 IOMMUs use > find_next_zero_bit too but both doesn't check if find_next_zero_bit > returns -1. If find_next_zero_bit fails, it returns size. So it > doesn't leads to an endless loop. Yes, this can't happen. It was a wrong assumption on my part. > But this patch has other bugs that break POWER IOMMUs. > > If you use the IOMMUs on POWER, please try the following patch: I'm using CONFIG_GART_IOMMU=y on x86_64. > http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg12702.html I also noted the line "index = (index + align_mask) & ~align_mask;" in iommu_area_alloc() and didn't understand what this was trying to do and how this should work, but as arch/x86/kernel/pci-gart_64.c always uses 0 as align_mask I just ignored it. I will applie your patch and see if this hunk from find_next_zero_area() makes a difference: end = index + nr; - if (end > size) + if (end >= size) return -1; - for (i = index + 1; i < end; i++) { + for (i = index; i < end; i++) { if (test_bit(i, map)) { Torsten -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/