From mboxrd@z Thu Jan 1 00:00:00 1970 From: Josh Boyer Subject: Re: Oops in minixfs_statfs Date: Tue, 16 Aug 2011 20:31:39 -0400 Message-ID: <20110817003138.GF2227@zod.bos.redhat.com> References: <20110816164809.GE2227@zod.bos.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Al Viro , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org To: Bob Copeland Return-path: Received: from mx1.redhat.com ([209.132.183.28]:5024 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752592Ab1HQAcI (ORCPT ); Tue, 16 Aug 2011 20:32:08 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Aug 16, 2011 at 06:20:05PM -0400, Bob Copeland wrote: > On Tue, Aug 16, 2011 at 12:48 PM, Josh Boyer wro= te: > > it seems we're getting a negative number in this particular calcula= tion > > in fs/minix/bitmap.c, count_free: > > > > =A0 =A0 =A0 =A0i =3D ((numbits - (numblocks-1) * bh->b_size * 8) / = 16) * 2; > [...] > > I'm not familiar enough with minixfs to know what the above is tryi= ng to > > actually accomplish. =A0I instrumented that function a bit and here= is > > some data from the 3 filesytems in question: >=20 > I don't know minix either but I'll take a shot. This is trying to > count the number of bits in the free map for inodes or data blocks > that don't fit in a full file system block. >=20 > count_free takes 3 params: > map - an array of buffer heads that represent the free map for a 't= hingy' > (thingy being the technical term for inode, or data block) > numblocks - the number of blocks to scan > numbits - the maximum-valued thingy >=20 > So, for example, there might be 4800 inodes in the filesystem. That > means there are 4800/8 =3D 600 bits to represent their in-use state, > and supposing a buffer head represents a 512-byte buffer (bh->b_size=3D= 512), > you would need two blocks to store that. numblocks in this hypotheti= cal > example is 2 and numbits is 4801. Yep, I gathered that. In the real case, there are 7 blocks, and the block size of the fs is 4096. Though numbits still seems to be "maximu= m number of blocks" based on the value and what is being passed to count_free. > Here's some annotated code with uninteresting parts removed: >=20 > Loop over all but the last block: >=20 > for (i=3D0; i sum +=3D nibblemap[bh->b_data[j] & 0xf] > + nibblemap[(bh->b_data[j]>>4) & 0xf]; > } >=20 > Since we're counting zero bits this is just computing hweight(~x)... >=20 > if (numblocks=3D=3D0 || !(bh=3Dmap[numblocks-1])) > return(0); >=20 > Lookout, bh is assigned in the conditional! >=20 > Ok so bh is the last block which might not be full of thingys, so > we only want to look at the part that has thingys and not the rest. >=20 > numblocks - 1 * bh->b_size * 8 is the number of bits we've already > looked at. We'll call it nthingys. numbits - nthingys is the number > of bits we want to continue to look at. "(x / 16) * 2 is a fancy way Yes, except here numbits - nthingys winds up being a negative number in the failing case. > of saying "x / 8" except the result is on 2-byte boundaries. So that= 's > the number of bytes left to look at, except for up to 15 possible > extra bits. >=20 > i =3D ((numbits - (numblocks-1) * bh->b_size * 8) / 16) * 2; > for (j=3D0; j sum +=3D nibblemap[bh->b_data[j] & 0xf] > + nibblemap[(bh->b_data[j]>>4) & 0xf]; > } And we blow up in here somewhere. > And then count the remaining 15 bits by masking off whatever portion > is less than numbits and doing some fancy u16 casting. >=20 > i =3D numbits%16; > if (i!=3D0) { > i =3D *(__u16 *)(&bh->b_data[j]) | ~((1< sum +=3D nibblemap[i & 0xf] + nibblemap[(i>>4) & 0xf]; > sum +=3D nibblemap[(i>>8) & 0xf] + nibblemap[(i>>12) & 0xf]; > } >=20 > return(sum); > } >=20 > Does that help? It helps me understand what the code is doing a bit better, yes. Thank you for that. Unfortunately, it doesn't really tell me why we get a negative number in this case, and if we should be checking for that her= e and what to do about it. josh -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html