Re: BUG: isofs broken (2.2 and 2.4)

Re: BUG: isofs broken (2.2 and 2.4)

[Andries.Brouwer]

>> I take it you'll also do the third part?

> Are you talking about isofs_lookup_grandparent()?

No, about isofs_read_inode.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Linus Torvalds]

There's a test11-pre7 there now, and I'd really ask people to check out
the isofs changes because slight worry about those is what held me up from
just calling it test11 outright.

It's almost guaranteed to be better than what we had before, but anyway..

		Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

test11-pre7 compile failure

[J Sloan]

Just a quick heads-up -

looks like the md fixes broke something -

In file included from /usr/src/linux/include/linux/pagemap.h:17,
                 from /usr/src/linux/include/linux/locks.h:9,
                 from /usr/src/linux/include/linux/raid/md.h:37,
                 from init/main.c:25:
/usr/src/linux/include/linux/highmem.h: In function `bh_kmap':
/usr/src/linux/include/linux/highmem.h:23: structure has no member named
`p_page'
In file included from /usr/src/linux/include/linux/raid/md.h:51,
                 from init/main.c:25:
/usr/src/linux/include/linux/raid/md_k.h: In function `pers_to_level':
/usr/src/linux/include/linux/raid/md_k.h:39: warning: control reaches end of
non-void function
make: *** [init/main.o] Error 1


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: test11-pre7 compile failure

[Linus Torvalds]

In article <3A15DDCB.42B0F208@toyota.com>, J Sloan  <jjs@toyota.com> wrote:
>
>looks like the md fixes broke something -
>
>In file included from /usr/src/linux/include/linux/pagemap.h:17,
>                 from /usr/src/linux/include/linux/locks.h:9,
>                 from /usr/src/linux/include/linux/raid/md.h:37,
>                 from init/main.c:25:
>/usr/src/linux/include/linux/highmem.h: In function `bh_kmap':
>/usr/src/linux/include/linux/highmem.h:23: structure has no member named
>`p_page'

The "p_page" should be a "b_page". Duh.

		Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Keith Owens]

On Fri, 17 Nov 2000 17:21:53 -0800 (PST), 
Linus Torvalds <torvalds@transmeta.com> wrote:
>There's a test11-pre7 there now, and I'd really ask people to check out
>the isofs changes because slight worry about those is what held me up from
>just calling it test11 outright.
>
>It's almost guaranteed to be better than what we had before, but anyway..
>
>		Linus

namei.c: In function `isofs_find_entry':
namei.c:130: warning: passing arg 2 of `get_joliet_filename' from incompatible pointer type
namei.c:130: warning: passing arg 3 of `get_joliet_filename' from incompatible pointer type


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Linus Torvalds]

On Sat, 18 Nov 2000, Keith Owens wrote:

> On Fri, 17 Nov 2000 17:21:53 -0800 (PST), 
> Linus Torvalds <torvalds@transmeta.com> wrote:
> >There's a test11-pre7 there now, and I'd really ask people to check out
> >the isofs changes because slight worry about those is what held me up from
> >just calling it test11 outright.
> >
> >It's almost guaranteed to be better than what we had before, but anyway..
> >
> >		Linus
> 
> namei.c: In function `isofs_find_entry':
> namei.c:130: warning: passing arg 2 of `get_joliet_filename' from incompatible pointer type
> namei.c:130: warning: passing arg 3 of `get_joliet_filename' from incompatible pointer type

Thanks. The second and third arguments were switched around to match all
the other filename conversion stuff, and because I don't have joliet
enabled I didn't notice this. Just switch them around where the warning
occurs, and you should be golden.

		Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Andries.Brouwer]

    From koenig@orion.tat.physik.uni-tuebingen.de Mon Dec 18 11:34:14 2000

    On Nov 17, Linus Torvalds wrote:

    > ...

    better you'd have tested it;)  while Andries' patch works fine (2 CDs of
    data copied and checked a bit, seems to work ok with no obvious problems)
    your new patch still shows a number of problems:

    I've got a SIGSEGV in "find" and ...

Ah yes, but Nov 17 and 2.4.0test10 is ancient history.
You do not mention a kernel version, but if it is older
than 2.4.0test12, upgrade.

(Before 2.4.0test11: a few complaints. On 2.4.0test11: a deluge
of complaints. On later kernels: one or two complaints. Must still
look at the case where someone has problems with isofs over nfs -
maybe this is nfs-related, not isofs-related.)

(The story here was interesting: Linus' patch did part of the
work required, good enough for most people. Nevertheless there were
many complaints, and it turned out that gcc 2.95.2 mistranslated
the code. Removing a superfluous line made things work again,
leaving us worried how many other problems in kernel and user
software are caused by this compiler bug. Then I added the part of
my patch that Linus hadnt done yet, so now all should be well again.)

Andries

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Andries.Brouwer]

Linus:

> How about this version (full patch against test10 - it includes a
> slightly corrected version of my earlier dir.c patch)?

> It's entirely untested, but it looks good and compiles. Ship it!

There are three files that have to be changed.
You changed dir.c yesterday, and namei.c today
but still have to do inode.c.

Your stuff resembles my stuff. In namei.c I also replaced the 15
lines following
	} else if (dir->i_sb->u.isofs_sb.s_mapping == 'n') {
by the line
	dlen = isofs_name_translate(dpnt, dlen, page);

But now that you did two-thirds of the job I take it you'll
also do the third part? It is again precisely the same stuff.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Linus Torvalds]

On Sat, 18 Nov 2000 Andries.Brouwer@cwi.nl wrote:
> 
> But now that you did two-thirds of the job I take it you'll
> also do the third part? It is again precisely the same stuff.

Are you talking about isofs_lookup_grandparent()?

The code is now dead, and has been for a long time actually (as the VFS
layer keeps track of ".." for us these days). Removed.

I'll look at the isofs_read_level3_size() thing. At least that one doesn't
have the name translation crap in it.

		Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Harald Koenig]

On Nov 17, Andries.Brouwer@cwi.nl wrote:

> 
> > > +               if (cpnt)
> > > +                       kfree(cpnt);
> 
> > this seems to make things much worse
> 
> Yes, I meant
> 
> 		if (cpnt) {
> 			kfree(cpnt);
> 			cpnt = NULL;
> 		}
> 
> at that place, otherwise things will be freed multiple times.

_MUCH_ better now!!!  no lockups anymore, no memory leak(s).


BUT:  there is still this small performace and memory usage issue:

each of these CDs contains >80k files in ~110 directories each
(the full db consists of 18 CDs!) and running "find" or "du"
on one CDROM (or 4MB isofs loop mount from hard disk) takes
a huge amount of time (real and system cpu) with cold cache:

	time find /cdrom
	0.610u 97.250s 1:40.58 97.2%    0+0k 0+0io 98pf+0w

flush cache...

	time du /cdrom
	0.470u 97.220s 1:40.29 97.4%    0+0k 0+0io 112pf+0w


whereas with hot cache (takes ~45MB memory off the value
for "free + buffer/cache" for a 4MB isofs image!) gives (PPro200, 128MB):

	time find /cdrom
	0.460u 1.280s 0:01.79 97.2%     0+0k 0+0io 102pf+0w

	time du /cdrom
	0.270u 1.260s 0:01.54 99.3%     0+0k 0+0io 108pf+0w



so it seems to work  (data still not checked, can do this only next week)
but performace really sucks :(



anyway, thanks a lot for your help and quick patch !  
now at least we can copy all the data to some hard disk
and use it that way.  

a patch for 2.2.x (the real production machine can't run 2.4.x yet)
and/or fixes for the bad performace would be appreciated anyway ;^)




thanks!

Harald
-- 
All SCSI disks will from now on                     ___       _____
be required to send an email notice                0--,|    /OOOOOOO\
24 hours prior to complete hardware failure!      <_/  /  /OOOOOOOOOOO\
                                                    \  \/OOOOOOOOOOOOOOO\
                                                      \ OOOOOOOOOOOOOOOOO|//
Harald Koenig,                                         \/\/\/\/\/\/\/\/\/
Inst.f.Theoret.Astrophysik                              //  /     \\  \
koenig@tat.physik.uni-tuebingen.de                     ^^^^^       ^^^^^
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Andries.Brouwer]

> memory leak

Aha. Must be a missing kfree().
Does this help?

--- namei.c~    Fri Nov 17 00:48:37 2000
+++ namei.c     Fri Nov 17 21:59:49 2000
@@ -197,6 +197,8 @@
                        bh = NULL;
                        break;
                }
+               if (cpnt)
+                       kfree(cpnt);
        }
        if (page)
                free_page((unsigned long) page);

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Harald Koenig]

On Nov 17, Andries.Brouwer@cwi.nl wrote:

> > memory leak
> 
> Aha. Must be a missing kfree().
> Does this help?
> 
> --- namei.c~    Fri Nov 17 00:48:37 2000
> +++ namei.c     Fri Nov 17 21:59:49 2000
> @@ -197,6 +197,8 @@
>                         bh = NULL;
>                         break;
>                 }
> +               if (cpnt)
> +                       kfree(cpnt);
>         }
>         if (page)
>                 free_page((unsigned long) page);
> 
> Andries

this seems to make things much worse:  starting with ~90M free memory
"du" again started leaking (or maybe just using memory?) down to ~80M free
memory when the system suddently locked up completely, no console switch
was possible anymore (but Sysrq-B did reboot).



Harald
-- 
All SCSI disks will from now on                     ___       _____
be required to send an email notice                0--,|    /OOOOOOO\
24 hours prior to complete hardware failure!      <_/  /  /OOOOOOOOOOO\
                                                    \  \/OOOOOOOOOOOOOOO\
                                                      \ OOOOOOOOOOOOOOOOO|//
Harald Koenig,                                         \/\/\/\/\/\/\/\/\/
Inst.f.Theoret.Astrophysik                              //  /     \\  \
koenig@tat.physik.uni-tuebingen.de                     ^^^^^       ^^^^^
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Linus Torvalds]

On Fri, 17 Nov 2000, Harald Koenig wrote:
> 
> this seems to make things much worse:  starting with ~90M free memory
> "du" again started leaking (or maybe just using memory?) down to ~80M free
> memory when the system suddently locked up completely, no console switch
> was possible anymore (but Sysrq-B did reboot).

How about this version (full patch against test10 - it includes a
slightly corrected version of my earlier dir.c patch)?

It's entirely untested, but it looks good and compiles. Ship it!

		Linus

-----
diff -u --recursive --new-file v2.4.0-test10/linux/fs/isofs/dir.c linux/fs/isofs/dir.c
--- v2.4.0-test10/linux/fs/isofs/dir.c	Fri Aug 11 14:29:01 2000
+++ linux/fs/isofs/dir.c	Fri Nov 17 13:38:01 2000
@@ -94,6 +94,14 @@
 	return retnamlen;
 }
 
+static struct buffer_head *isofs_bread(struct inode *inode, unsigned int bufsize, unsigned int block)
+{
+	unsigned int blknr = isofs_bmap(inode, block);
+	if (!blknr)
+		return NULL;
+	return bread(inode->i_dev, blknr, bufsize);
+}
+
 /*
  * This should _really_ be cleaned up some day..
  */
@@ -105,7 +113,7 @@
 	unsigned char bufbits = ISOFS_BUFFER_BITS(inode);
 	unsigned int block, offset;
 	int inode_number = 0;	/* Quiet GCC */
-	struct buffer_head *bh;
+	struct buffer_head *bh = NULL;
 	int len;
 	int map;
 	int high_sierra;
@@ -117,46 +125,25 @@
 		return 0;
  
 	offset = filp->f_pos & (bufsize - 1);
-	block = isofs_bmap(inode, filp->f_pos >> bufbits);
+	block = filp->f_pos >> bufbits;
 	high_sierra = inode->i_sb->u.isofs_sb.s_high_sierra;
 
-	if (!block)
-		return 0;
-
-	if (!(bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size)))
-		return 0;
-
 	while (filp->f_pos < inode->i_size) {
 		int de_len;
-#ifdef DEBUG
-		printk("Block, offset, f_pos: %x %x %x\n",
-		       block, offset, filp->f_pos);
-	        printk("inode->i_size = %x\n",inode->i_size);
-#endif
-		/* Next directory_record on next CDROM sector */
-		if (offset >= bufsize) {
-#ifdef DEBUG
-			printk("offset >= bufsize\n");
-#endif
-			brelse(bh);
-			offset = 0;
-			block = isofs_bmap(inode, (filp->f_pos) >> bufbits);
-			if (!block)
-				return 0;
-			bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size);
+
+		if (!bh) {
+			bh = isofs_bread(inode, bufsize, block);
 			if (!bh)
 				return 0;
-			continue;
 		}
 
 		de = (struct iso_directory_record *) (bh->b_data + offset);
-		if(first_de) inode_number = (block << bufbits) + (offset & (bufsize - 1));
+		if (first_de) inode_number = (bh->b_blocknr << bufbits) + offset;
 
 		de_len = *(unsigned char *) de;
 #ifdef DEBUG
 		printk("de_len = %d\n", de_len);
-#endif
-	    
+#endif	    
 
 		/* If the length byte is zero, we should move on to the next
 		   CDROM sector.  If we are at the end of the directory, we
@@ -164,36 +151,33 @@
 
 		if (de_len == 0) {
 			brelse(bh);
-			filp->f_pos = ((filp->f_pos & ~(ISOFS_BLOCK_SIZE - 1))
-				       + ISOFS_BLOCK_SIZE);
+			bh = NULL;
+			filp->f_pos = ((filp->f_pos & ~(ISOFS_BLOCK_SIZE - 1)) + ISOFS_BLOCK_SIZE);
+			block = filp->f_pos >> bufbits;
 			offset = 0;
-
-			if (filp->f_pos >= inode->i_size)
-				return 0;
-
-			block = isofs_bmap(inode, (filp->f_pos) >> bufbits);
-			if (!block)
-				return 0;
-			bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size);
-			if (!bh)
-				return 0;
 			continue;
 		}
 
-		offset +=  de_len;
-		if (offset > bufsize) {
-			/*
-			 * This would only normally happen if we had
-			 * a buggy cdrom image.  All directory
-			 * entries should terminate with a null size
-			 * or end exactly at the end of the sector.
-			 */
-		        printk("next_offset (%x) > bufsize (%lx)\n",
-			       offset,bufsize);
-			break;
+		offset += de_len;
+
+		/* Make sure we have a full directory entry */
+		if (offset >= bufsize) {
+			int slop = bufsize - offset + de_len;
+			memcpy(tmpde, de, slop);
+			offset &= bufsize - 1;
+			block++;
+			brelse(bh);
+			bh = NULL;
+			if (offset == bufsize) {
+				bh = isofs_bread(inode, bufsize, block);
+				if (!bh)
+					return 0;
+				memcpy((void *) tmpde + slop, bh->b_data, de_len - slop);
+			}
+			de = tmpde;				
 		}
 
-		if(de->flags[-high_sierra] & 0x80) {
+		if (de->flags[-high_sierra] & 0x80) {
 			first_de = 0;
 			filp->f_pos += de_len;
 			continue;
@@ -265,7 +249,7 @@
 
 		continue;
 	}
-	brelse(bh);
+	if (bh) brelse(bh);
 	return 0;
 }
 
diff -u --recursive --new-file v2.4.0-test10/linux/fs/isofs/namei.c linux/fs/isofs/namei.c
--- v2.4.0-test10/linux/fs/isofs/namei.c	Mon May 10 14:14:28 1999
+++ linux/fs/isofs/namei.c	Fri Nov 17 14:22:33 2000
@@ -49,6 +49,15 @@
 	return dentry->d_op->d_compare(dentry, &dentry->d_name, &qstr);
 }
 
+static struct buffer_head *isofs_bread(struct inode *inode, unsigned int bufsize, unsigned int block)
+{
+	unsigned int blknr = isofs_bmap(inode, block);
+	if (!blknr)
+		return NULL;
+	return bread(inode->i_dev, blknr, bufsize);
+}
+
+
 /*
  *	isofs_find_entry()
  *
@@ -57,129 +66,82 @@
  * itself (as an inode number). It does NOT read the inode of the
  * entry - you'll have to do that yourself if you want to.
  */
-static struct buffer_head *
-isofs_find_entry(struct inode *dir, struct dentry *dentry, unsigned long *ino)
+static unsigned long
+isofs_find_entry(struct inode *dir, struct dentry *dentry,
+	char * tmpname, struct iso_directory_record * tmpde)
 {
+	unsigned long inode_number;
 	unsigned long bufsize = ISOFS_BUFFER_SIZE(dir);
 	unsigned char bufbits = ISOFS_BUFFER_BITS(dir);
-	unsigned int block, i, f_pos, offset, 
-		inode_number = 0; /* shut gcc up */
-	struct buffer_head * bh , * retval = NULL;
-	unsigned int old_offset;
-	int dlen, match;
-	char * dpnt;
-	unsigned char *page = NULL;
-	struct iso_directory_record * de = NULL; /* shut gcc up */
-	char de_not_in_buf = 0;	  /* true if de is in kmalloc'd memory */
-	char c;
-
-	*ino = 0;
-	
-	if (!(block = dir->u.isofs_i.i_first_extent)) return NULL;
+	unsigned int block, f_pos, offset;
+	struct buffer_head * bh = NULL;
+
+	if (!dir->u.isofs_i.i_first_extent)
+		return 0;
   
 	f_pos = 0;
+	offset = 0;
+	block = 0;
 
-	offset = f_pos & (bufsize - 1);
-	block = isofs_bmap(dir,f_pos >> bufbits);
+	while (f_pos < dir->i_size) {
+		struct iso_directory_record * de;
+		int de_len, match, i, dlen;
+		char *dpnt, c;
 
-	if (!block || !(bh = bread(dir->i_dev,block,bufsize))) return NULL;
+		if (!bh) {
+			bh = isofs_bread(dir, bufsize, block);
+			if (!bh)
+				return 0;
+		}
 
-	while (f_pos < dir->i_size) {
+		de = (struct iso_directory_record *) (bh->b_data + offset);
+		inode_number = (bh->b_blocknr << bufbits) + offset;
 
-		/* if de is in kmalloc'd memory, do not point to the
-                   next de, instead we will move to the next sector */
-		if(!de_not_in_buf) {
-			de = (struct iso_directory_record *) 
-				(bh->b_data + offset);
-		}
-		inode_number = (block << bufbits) + (offset & (bufsize - 1));
-
-		/* If byte is zero, or we had to fetch this de past
-		   the end of the buffer, this is the end of file, or
-		   time to move to the next sector. Usually 2048 byte
-		   boundaries. */
-		
-		if (*((unsigned char *) de) == 0 || de_not_in_buf) {
-			if(de_not_in_buf) {
-				/* james@bpgc.com: Since we slopped
-                                   past the end of the last buffer, we
-                                   must start some way into the new
-                                   one */
-				de_not_in_buf = 0;
-				kfree(de);
-				f_pos += offset;
-			}
-			else { 
-				offset = 0;
-				f_pos = ((f_pos & ~(ISOFS_BLOCK_SIZE - 1))
-					 + ISOFS_BLOCK_SIZE);
-			}
+		de_len = *(unsigned char *) de;
+		if (!de_len) {
 			brelse(bh);
 			bh = NULL;
-
-			if (f_pos >= dir->i_size) 
-				break;
-
-			block = isofs_bmap(dir,f_pos>>bufbits);
-			if (!block || !(bh = bread(dir->i_dev,block,bufsize)))
-				break;
-
-			continue; /* Will kick out if past end of directory */
+			f_pos = (f_pos + ISOFS_BLOCK_SIZE) & ~(ISOFS_BLOCK_SIZE - 1);
+			block = f_pos >> bufbits;
+			offset = 0;
+			continue;
 		}
 
-		old_offset = offset;
-		offset += *((unsigned char *) de);
-		f_pos += *((unsigned char *) de);
+		offset += de_len;
+		f_pos += de_len;
 
-		/* james@bpgc.com: new code to handle case where the
-		   directory entry spans two blocks.  Usually 1024
-		   byte boundaries */
+		/* Make sure we have a full directory entry */
 		if (offset >= bufsize) {
-			struct buffer_head *bh_next;
-
-			/* james@bpgc.com: read the next block, and
-                           copy the split de into a newly kmalloc'd
-                           buffer */
-			block = isofs_bmap(dir,f_pos>>bufbits);
-			if (!block || 
-			    !(bh_next = bread(dir->i_dev,block,bufsize)))
-				break;
-			
-			de = (struct iso_directory_record *)
-				kmalloc(offset - old_offset, GFP_KERNEL);
-			memcpy((char *)de, bh->b_data + old_offset, 
-			       bufsize - old_offset);
-			memcpy((char *)de + bufsize - old_offset,
-			       bh_next->b_data, offset - bufsize);
-			brelse(bh_next);
-			de_not_in_buf = 1;
-			offset -= bufsize;
+			int slop = bufsize - offset + de_len;
+			memcpy(tmpde, de, slop);
+			offset &= bufsize - 1;
+			block++;
+			brelse(bh);
+			bh = NULL;
+			if (offset == bufsize) {
+				bh = isofs_bread(dir, bufsize, block);
+				if (!bh)
+					return 0;
+				memcpy((void *) tmpde + slop, bh->b_data, de_len - slop);
+			}
+			de = tmpde;
 		}
+
 		dlen = de->name_len[0];
 		dpnt = de->name;
 
-		if (dir->i_sb->u.isofs_sb.s_rock ||
-		    dir->i_sb->u.isofs_sb.s_joliet_level || 
-		    dir->i_sb->u.isofs_sb.s_mapping == 'n' ||
-		    dir->i_sb->u.isofs_sb.s_mapping == 'a') {
-			if (! page) {
-				page = (unsigned char *)
-					__get_free_page(GFP_KERNEL);
-				if (!page) break;
-			}
-		}
 		if (dir->i_sb->u.isofs_sb.s_rock &&
-		    ((i = get_rock_ridge_filename(de, page, dir)))) {
+		    ((i = get_rock_ridge_filename(de, tmpname, dir)))) {
 			dlen = i;
-			dpnt = page;
+			dpnt = tmpname;
 #ifdef CONFIG_JOLIET
 		} else if (dir->i_sb->u.isofs_sb.s_joliet_level) {
-			dlen = get_joliet_filename(de, dir, page);
-			dpnt = page;
+			dlen = get_joliet_filename(de, dir, tmpname);
+			dpnt = tmpname;
 #endif
 		} else if (dir->i_sb->u.isofs_sb.s_mapping == 'a') {
-			dlen = get_acorn_filename(de, page, dir);
-			dpnt = page;
+			dlen = get_acorn_filename(de, tmpname, dir);
+			dpnt = tmpname;
 		} else if (dir->i_sb->u.isofs_sb.s_mapping == 'n') {
 			for (i = 0; i < dlen; i++) {
 				c = dpnt[i];
@@ -190,13 +152,13 @@
 					break;
 				}
 				if (c == ';') c = '.';
-				page[i] = c;
+				tmpname[i] = c;
 			}
 			/* This allows us to match with and without
 			 * a trailing period. */
-			if(page[dlen-1] == '.' && dentry->d_name.len == dlen-1)
+			if(tmpname[dlen-1] == '.' && dentry->d_name.len == dlen-1)
 				dlen--;
-			dpnt = page;
+			dpnt = tmpname;
 		}
 		/*
 		 * Skip hidden or associated files unless unhide is set 
@@ -208,43 +170,32 @@
 			match = (isofs_cmp(dentry,dpnt,dlen) == 0);
 		}
 		if (match) {
-			if(inode_number == -1) {
-				/* Should only happen for the '..' entry */
-				inode_number = 
-					isofs_lookup_grandparent(dir,
-					   find_rock_ridge_relocation(de,dir));
-			}
-			*ino = inode_number;
-			retval = bh;
-			bh = NULL;
-			break;
+			if (bh) brelse(bh);
+			return inode_number;
 		}
 	}
-	if (page) free_page((unsigned long) page);
 	if (bh) brelse(bh);
-	if(de_not_in_buf) 
-		kfree(de);
-	return retval;
+	return 0;
 }
 
 struct dentry *isofs_lookup(struct inode * dir, struct dentry * dentry)
 {
 	unsigned long ino;
-	struct buffer_head * bh;
 	struct inode *inode;
+	struct page *page;
 
 #ifdef DEBUG
 	printk("lookup: %x %s\n",dir->i_ino, dentry->d_name.name);
 #endif
 	dentry->d_op = dir->i_sb->s_root->d_op;
 
-	bh = isofs_find_entry(dir, dentry, &ino);
+	page = alloc_page(GFP_USER);
+	ino = isofs_find_entry(dir, dentry, page_address(page), 1024 + page_address(page));
+	__free_page(page);
 
 	inode = NULL;
-	if (bh) {
-		brelse(bh);
-
-		inode = iget(dir->i_sb,ino);
+	if (ino) {
+		inode = iget(dir->i_sb, ino);
 		if (!inode)
 			return ERR_PTR(-EACCES);
 	}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Harald Koenig]

On Nov 17, Linus Torvalds wrote:

> 
> 
> On Fri, 17 Nov 2000, Harald Koenig wrote:
> > 
> > this seems to make things much worse:  starting with ~90M free memory
> > "du" again started leaking (or maybe just using memory?) down to ~80M free
> > memory when the system suddently locked up completely, no console switch
> > was possible anymore (but Sysrq-B did reboot).
> 
> How about this version (full patch against test10 - it includes a
> slightly corrected version of my earlier dir.c patch)?
> 
> It's entirely untested, but it looks good and compiles. Ship it!

it looks slightly better performacewise with cold cache when compared
with Andries' patch:

Linus:		0.380u 76.850s 1:19.12 97.6%    0+0k 0+0io 113pf+0w
Andries:	0.470u 97.220s 1:40.29 97.4%    0+0k 0+0io 112pf+0w


BUT: there are some obvious bugs in the output of "du" and "find".
some samples (all file names (should) match the format "xe%03d/xe%03d.%c%c"
with both %03d being the _same_ number and both %c are in [a-z0-9]).

from "find" output:

...
/mnt/xe001/xe001.hg
find: /mnt/xe001/xe001.h: No such file or directory
/mnt/xe001/xe001.hi
...
/mnt/xe001/xe001.ib
find: /mnt/xe001/xe001.h: No such file or directory
/mnt/xe001/xe001.id
...
find: /mnt/xe003/xe002.rg: No such file or directory
...
find: /mnt/xe004/xe003.rg: No such file or directory
...
find: /mnt/xe005/xe004.rg: No such file or directory


"find" from hot cache even shows some binary garbage:

...
/mnt/xe001/xe001.0k
find: /mnt/xe001/¶\x04\b^¶\x04\b ¶\x04\b\x10¶\x04\bp¶\x04\b$¶\x04\b^¶\x04\b\x1a¶\x04\b^¶\x04\b^¶\x04\b ¶\x04\b\x10¶\x04\b\x10¶\x04\b{}: No such file or directory
/mnt/xe001/xe001.0m
...
/mnt/xe001/xe001.gl
find: /mnt/xe001/xe105/xe105.p1
/mnt/xe105/xe105.p2
/mnt/xe105/x: No such file or directory
/mnt/xe001/xe001.gn
...



and from "du":
...
du: /mnt/xe001/xe001.k: No such file or directory
du: /mnt/xe001/xe001.k: No such file or directory
du: /mnt/xe001/xe001.k: No such file or directory
du: /mnt/xe001/xe001.m: No such file or directory
du: /mnt/xe001/xe001.m: No such file or directory
du: /mnt/xe001/xe001.m: No such file or directory
du: /mnt/xe001/xe001.o: No such file or directory
du: /mnt/xe001/xe001.o: No such file or directory
du: /mnt/xe001/xe001.o: No such file or directory
du: /mnt/xe001/xe001.p: No such file or directory
du: /mnt/xe001/xe001.p: No such file or directory
du: /mnt/xe001/xe001.p: No such file or directory
du: /mnt/xe001/xe001.r: No such file or directory
3378    /mnt/xe001
du: /mnt/xe002/xe001.og: No such file or directory
du: /mnt/xe002/xe001.og: No such file or directory
du: /mnt/xe002/xe001.og: No such file or directory
4587    /mnt/xe002
du: /mnt/xe003/xe002.rg: No such file or directory
du: /mnt/xe003/xe002.rg: No such file or directory
du: /mnt/xe003/xe002.rg: No such file or directory
3669    /mnt/xe003
4451    /mnt/xe004
du: /mnt/xe005/xe004.rg: No such file or directory
du: /mnt/xe005/xe004.rg: No such file or directory
du: /mnt/xe005/xe004.rg: No such file or directory
3728    /mnt/xe005
...
du: /mnt/xe010/

# note: this file is far from being complete: No such file or directory
du: /mnt/xe010/

# note: this file is far from being complete: No such file or directory
du: /mnt/xe010/

# note: this file is far from being complete: No such file or directory
4263    /mnt/xe010






Harald
-- 
All SCSI disks will from now on                     ___       _____
be required to send an email notice                0--,|    /OOOOOOO\
24 hours prior to complete hardware failure!      <_/  /  /OOOOOOOOOOO\
                                                    \  \/OOOOOOOOOOOOOOO\
                                                      \ OOOOOOOOOOOOOOOOO|//
Harald Koenig,                                         \/\/\/\/\/\/\/\/\/
Inst.f.Theoret.Astrophysik                              //  /     \\  \
koenig@tat.physik.uni-tuebingen.de                     ^^^^^       ^^^^^
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Linus Torvalds]

On Fri, 17 Nov 2000, Harald Koenig wrote:
> 
> Linus:		0.380u 76.850s 1:19.12 97.6%    0+0k 0+0io 113pf+0w
> Andries:	0.470u 97.220s 1:40.29 97.4%    0+0k 0+0io 112pf+0w

The biggest difference is just the system times and the fact that it's
more efficient coding. 

> BUT: there are some obvious bugs in the output of "du" and "find".
> some samples (all file names (should) match the format "xe%03d/xe%03d.%c%c"
> with both %03d being the _same_ number and both %c are in [a-z0-9]).

Yes. There's a silly bug there, now that I've tested it a bit. Basically
the test for stuff that traversed a boundary was wrong.

The whole name conversion code is pretty horrible. It's been written over
the years, and it was doing the same thing with small modifications in
both readdir() and lookup(). I've got a cleaned up version that also
should have the above bug fixed.

Still ready to test? This time I went over the files rather carefully, and
while I've not tested the fixed version I'm getting pretty happy with it.

I'll merge some more of the name translation logic, but before I do that
here's the newest patch..

		Linus

-----
diff -u --recursive --new-file v2.4.0-test10/linux/fs/isofs/dir.c linux/fs/isofs/dir.c
--- v2.4.0-test10/linux/fs/isofs/dir.c	Fri Aug 11 14:29:01 2000
+++ linux/fs/isofs/dir.c	Fri Nov 17 15:43:36 2000
@@ -40,14 +40,17 @@
 	lookup:		isofs_lookup,
 };
 
-static int isofs_name_translate(char * old, int len, char * new)
+int isofs_name_translate(struct iso_directory_record *de, char *new, struct inode *inode)
 {
-	int i, c;
+	char * old = de->name;
+	int len = de->name_len[0];
+	int i;
 			
 	for (i = 0; i < len; i++) {
-		c = old[i];
+		unsigned char c = old[i];
 		if (!c)
 			break;
+
 		if (c >= 'A' && c <= 'Z')
 			c |= 0x20;	/* lower case */
 
@@ -74,8 +77,7 @@
 {
 	int std;
 	unsigned char * chr;
-	int retnamlen = isofs_name_translate(de->name,
-				de->name_len[0], retname);
+	int retnamlen = isofs_name_translate(de, retname, inode);
 	if (retnamlen == 0) return 0;
 	std = sizeof(struct iso_directory_record) + de->name_len[0];
 	if (std & 1) std++;
@@ -105,7 +107,7 @@
 	unsigned char bufbits = ISOFS_BUFFER_BITS(inode);
 	unsigned int block, offset;
 	int inode_number = 0;	/* Quiet GCC */
-	struct buffer_head *bh;
+	struct buffer_head *bh = NULL;
 	int len;
 	int map;
 	int high_sierra;
@@ -117,46 +119,22 @@
 		return 0;
  
 	offset = filp->f_pos & (bufsize - 1);
-	block = isofs_bmap(inode, filp->f_pos >> bufbits);
+	block = filp->f_pos >> bufbits;
 	high_sierra = inode->i_sb->u.isofs_sb.s_high_sierra;
 
-	if (!block)
-		return 0;
-
-	if (!(bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size)))
-		return 0;
-
 	while (filp->f_pos < inode->i_size) {
 		int de_len;
-#ifdef DEBUG
-		printk("Block, offset, f_pos: %x %x %x\n",
-		       block, offset, filp->f_pos);
-	        printk("inode->i_size = %x\n",inode->i_size);
-#endif
-		/* Next directory_record on next CDROM sector */
-		if (offset >= bufsize) {
-#ifdef DEBUG
-			printk("offset >= bufsize\n");
-#endif
-			brelse(bh);
-			offset = 0;
-			block = isofs_bmap(inode, (filp->f_pos) >> bufbits);
-			if (!block)
-				return 0;
-			bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size);
+
+		if (!bh) {
+			bh = isofs_bread(inode, bufsize, block);
 			if (!bh)
 				return 0;
-			continue;
 		}
 
 		de = (struct iso_directory_record *) (bh->b_data + offset);
-		if(first_de) inode_number = (block << bufbits) + (offset & (bufsize - 1));
+		if (first_de) inode_number = (bh->b_blocknr << bufbits) + offset;
 
 		de_len = *(unsigned char *) de;
-#ifdef DEBUG
-		printk("de_len = %d\n", de_len);
-#endif
-	    
 
 		/* If the length byte is zero, we should move on to the next
 		   CDROM sector.  If we are at the end of the directory, we
@@ -164,36 +142,33 @@
 
 		if (de_len == 0) {
 			brelse(bh);
-			filp->f_pos = ((filp->f_pos & ~(ISOFS_BLOCK_SIZE - 1))
-				       + ISOFS_BLOCK_SIZE);
+			bh = NULL;
+			filp->f_pos = ((filp->f_pos & ~(ISOFS_BLOCK_SIZE - 1)) + ISOFS_BLOCK_SIZE);
+			block = filp->f_pos >> bufbits;
 			offset = 0;
-
-			if (filp->f_pos >= inode->i_size)
-				return 0;
-
-			block = isofs_bmap(inode, (filp->f_pos) >> bufbits);
-			if (!block)
-				return 0;
-			bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size);
-			if (!bh)
-				return 0;
 			continue;
 		}
 
-		offset +=  de_len;
-		if (offset > bufsize) {
-			/*
-			 * This would only normally happen if we had
-			 * a buggy cdrom image.  All directory
-			 * entries should terminate with a null size
-			 * or end exactly at the end of the sector.
-			 */
-		        printk("next_offset (%x) > bufsize (%lx)\n",
-			       offset,bufsize);
-			break;
+		offset += de_len;
+
+		/* Make sure we have a full directory entry */
+		if (offset >= bufsize) {
+			int slop = bufsize - offset + de_len;
+			memcpy(tmpde, de, slop);
+			offset &= bufsize - 1;
+			block++;
+			brelse(bh);
+			bh = NULL;
+			if (offset) {
+				bh = isofs_bread(inode, bufsize, block);
+				if (!bh)
+					return 0;
+				memcpy((void *) tmpde + slop, bh->b_data, de_len - slop);
+			}
+			de = tmpde;				
 		}
 
-		if(de->flags[-high_sierra] & 0x80) {
+		if (de->flags[-high_sierra] & 0x80) {
 			first_de = 0;
 			filp->f_pos += de_len;
 			continue;
@@ -240,7 +215,7 @@
 		if (map) {
 #ifdef CONFIG_JOLIET
 			if (inode->i_sb->u.isofs_sb.s_joliet_level) {
-				len = get_joliet_filename(de, inode, tmpname);
+				len = get_joliet_filename(de, tmpname, inode);
 				p = tmpname;
 			} else
 #endif
@@ -249,8 +224,7 @@
 				p = tmpname;
 			} else
 			if (inode->i_sb->u.isofs_sb.s_mapping == 'n') {
-				len = isofs_name_translate(de->name,
-					de->name_len[0], tmpname);
+				len = isofs_name_translate(de, tmpname, inode);
 				p = tmpname;
 			} else {
 				p = de->name;
@@ -265,7 +239,7 @@
 
 		continue;
 	}
-	brelse(bh);
+	if (bh) brelse(bh);
 	return 0;
 }
 
diff -u --recursive --new-file v2.4.0-test10/linux/fs/isofs/inode.c linux/fs/isofs/inode.c
--- v2.4.0-test10/linux/fs/isofs/inode.c	Tue Jul 18 21:40:47 2000
+++ linux/fs/isofs/inode.c	Fri Nov 17 15:15:07 2000
@@ -972,14 +972,24 @@
 	return 0;
 }
 
+struct buffer_head *isofs_bread(struct inode *inode, unsigned int bufsize, unsigned int block)
+{
+	unsigned int blknr = isofs_bmap(inode, block);
+	if (!blknr)
+		return NULL;
+	return bread(inode->i_dev, blknr, bufsize);
+}
+
 static int isofs_readpage(struct file *file, struct page *page)
 {
 	return block_read_full_page(page,isofs_get_block);
 }
+
 static int _isofs_bmap(struct address_space *mapping, long block)
 {
 	return generic_block_bmap(mapping,block,isofs_get_block);
 }
+
 static struct address_space_operations isofs_aops = {
 	readpage: isofs_readpage,
 	sync_page: block_sync_page,
diff -u --recursive --new-file v2.4.0-test10/linux/fs/isofs/joliet.c linux/fs/isofs/joliet.c
--- v2.4.0-test10/linux/fs/isofs/joliet.c	Tue Jul 18 22:48:32 2000
+++ linux/fs/isofs/joliet.c	Fri Nov 17 15:29:55 2000
@@ -70,8 +70,7 @@
 }
 
 int
-get_joliet_filename(struct iso_directory_record * de, struct inode * inode,
-		    unsigned char *outname)
+get_joliet_filename(struct iso_directory_record * de, unsigned char *outname, struct inode * inode)
 {
 	unsigned char utf8;
 	struct nls_table *nls;
diff -u --recursive --new-file v2.4.0-test10/linux/fs/isofs/namei.c linux/fs/isofs/namei.c
--- v2.4.0-test10/linux/fs/isofs/namei.c	Mon May 10 14:14:28 1999
+++ linux/fs/isofs/namei.c	Fri Nov 17 15:43:19 2000
@@ -57,147 +57,87 @@
  * itself (as an inode number). It does NOT read the inode of the
  * entry - you'll have to do that yourself if you want to.
  */
-static struct buffer_head *
-isofs_find_entry(struct inode *dir, struct dentry *dentry, unsigned long *ino)
+static unsigned long
+isofs_find_entry(struct inode *dir, struct dentry *dentry,
+	char * tmpname, struct iso_directory_record * tmpde)
 {
+	unsigned long inode_number;
 	unsigned long bufsize = ISOFS_BUFFER_SIZE(dir);
 	unsigned char bufbits = ISOFS_BUFFER_BITS(dir);
-	unsigned int block, i, f_pos, offset, 
-		inode_number = 0; /* shut gcc up */
-	struct buffer_head * bh , * retval = NULL;
-	unsigned int old_offset;
-	int dlen, match;
-	char * dpnt;
-	unsigned char *page = NULL;
-	struct iso_directory_record * de = NULL; /* shut gcc up */
-	char de_not_in_buf = 0;	  /* true if de is in kmalloc'd memory */
-	char c;
-
-	*ino = 0;
-	
-	if (!(block = dir->u.isofs_i.i_first_extent)) return NULL;
+	unsigned int block, f_pos, offset;
+	struct buffer_head * bh = NULL;
+
+	if (!dir->u.isofs_i.i_first_extent)
+		return 0;
   
 	f_pos = 0;
+	offset = 0;
+	block = 0;
 
-	offset = f_pos & (bufsize - 1);
-	block = isofs_bmap(dir,f_pos >> bufbits);
+	while (f_pos < dir->i_size) {
+		struct iso_directory_record * de;
+		int de_len, match, i, dlen;
+		char *dpnt;
 
-	if (!block || !(bh = bread(dir->i_dev,block,bufsize))) return NULL;
+		if (!bh) {
+			bh = isofs_bread(dir, bufsize, block);
+			if (!bh)
+				return 0;
+		}
 
-	while (f_pos < dir->i_size) {
+		de = (struct iso_directory_record *) (bh->b_data + offset);
+		inode_number = (bh->b_blocknr << bufbits) + offset;
 
-		/* if de is in kmalloc'd memory, do not point to the
-                   next de, instead we will move to the next sector */
-		if(!de_not_in_buf) {
-			de = (struct iso_directory_record *) 
-				(bh->b_data + offset);
-		}
-		inode_number = (block << bufbits) + (offset & (bufsize - 1));
-
-		/* If byte is zero, or we had to fetch this de past
-		   the end of the buffer, this is the end of file, or
-		   time to move to the next sector. Usually 2048 byte
-		   boundaries. */
-		
-		if (*((unsigned char *) de) == 0 || de_not_in_buf) {
-			if(de_not_in_buf) {
-				/* james@bpgc.com: Since we slopped
-                                   past the end of the last buffer, we
-                                   must start some way into the new
-                                   one */
-				de_not_in_buf = 0;
-				kfree(de);
-				f_pos += offset;
-			}
-			else { 
-				offset = 0;
-				f_pos = ((f_pos & ~(ISOFS_BLOCK_SIZE - 1))
-					 + ISOFS_BLOCK_SIZE);
-			}
+		de_len = *(unsigned char *) de;
+		if (!de_len) {
 			brelse(bh);
 			bh = NULL;
-
-			if (f_pos >= dir->i_size) 
-				break;
-
-			block = isofs_bmap(dir,f_pos>>bufbits);
-			if (!block || !(bh = bread(dir->i_dev,block,bufsize)))
-				break;
-
-			continue; /* Will kick out if past end of directory */
+			f_pos = (f_pos + ISOFS_BLOCK_SIZE) & ~(ISOFS_BLOCK_SIZE - 1);
+			block = f_pos >> bufbits;
+			offset = 0;
+			continue;
 		}
 
-		old_offset = offset;
-		offset += *((unsigned char *) de);
-		f_pos += *((unsigned char *) de);
+		offset += de_len;
+		f_pos += de_len;
 
-		/* james@bpgc.com: new code to handle case where the
-		   directory entry spans two blocks.  Usually 1024
-		   byte boundaries */
+		/* Make sure we have a full directory entry */
 		if (offset >= bufsize) {
-			struct buffer_head *bh_next;
-
-			/* james@bpgc.com: read the next block, and
-                           copy the split de into a newly kmalloc'd
-                           buffer */
-			block = isofs_bmap(dir,f_pos>>bufbits);
-			if (!block || 
-			    !(bh_next = bread(dir->i_dev,block,bufsize)))
-				break;
-			
-			de = (struct iso_directory_record *)
-				kmalloc(offset - old_offset, GFP_KERNEL);
-			memcpy((char *)de, bh->b_data + old_offset, 
-			       bufsize - old_offset);
-			memcpy((char *)de + bufsize - old_offset,
-			       bh_next->b_data, offset - bufsize);
-			brelse(bh_next);
-			de_not_in_buf = 1;
-			offset -= bufsize;
+			int slop = bufsize - offset + de_len;
+			memcpy(tmpde, de, slop);
+			offset &= bufsize - 1;
+			block++;
+			brelse(bh);
+			bh = NULL;
+			if (offset) {
+				bh = isofs_bread(dir, bufsize, block);
+				if (!bh)
+					return 0;
+				memcpy((void *) tmpde + slop, bh->b_data, de_len - slop);
+			}
+			de = tmpde;
 		}
+
 		dlen = de->name_len[0];
 		dpnt = de->name;
 
-		if (dir->i_sb->u.isofs_sb.s_rock ||
-		    dir->i_sb->u.isofs_sb.s_joliet_level || 
-		    dir->i_sb->u.isofs_sb.s_mapping == 'n' ||
-		    dir->i_sb->u.isofs_sb.s_mapping == 'a') {
-			if (! page) {
-				page = (unsigned char *)
-					__get_free_page(GFP_KERNEL);
-				if (!page) break;
-			}
-		}
 		if (dir->i_sb->u.isofs_sb.s_rock &&
-		    ((i = get_rock_ridge_filename(de, page, dir)))) {
+		    ((i = get_rock_ridge_filename(de, tmpname, dir)))) {
 			dlen = i;
-			dpnt = page;
+			dpnt = tmpname;
 #ifdef CONFIG_JOLIET
 		} else if (dir->i_sb->u.isofs_sb.s_joliet_level) {
-			dlen = get_joliet_filename(de, dir, page);
-			dpnt = page;
+			dlen = get_joliet_filename(de, dir, tmpname);
+			dpnt = tmpname;
 #endif
 		} else if (dir->i_sb->u.isofs_sb.s_mapping == 'a') {
-			dlen = get_acorn_filename(de, page, dir);
-			dpnt = page;
+			dlen = get_acorn_filename(de, tmpname, dir);
+			dpnt = tmpname;
 		} else if (dir->i_sb->u.isofs_sb.s_mapping == 'n') {
-			for (i = 0; i < dlen; i++) {
-				c = dpnt[i];
-				/* lower case */
-				if (c >= 'A' && c <= 'Z') c |= 0x20;
-				if (c == ';' && i == dlen-2 && dpnt[i+1] == '1') {
-					dlen -= 2;
-					break;
-				}
-				if (c == ';') c = '.';
-				page[i] = c;
-			}
-			/* This allows us to match with and without
-			 * a trailing period. */
-			if(page[dlen-1] == '.' && dentry->d_name.len == dlen-1)
-				dlen--;
-			dpnt = page;
+			dlen = isofs_name_translate(de, tmpname, dir);
+			dpnt = tmpname;
 		}
+
 		/*
 		 * Skip hidden or associated files unless unhide is set 
 		 */
@@ -208,43 +148,32 @@
 			match = (isofs_cmp(dentry,dpnt,dlen) == 0);
 		}
 		if (match) {
-			if(inode_number == -1) {
-				/* Should only happen for the '..' entry */
-				inode_number = 
-					isofs_lookup_grandparent(dir,
-					   find_rock_ridge_relocation(de,dir));
-			}
-			*ino = inode_number;
-			retval = bh;
-			bh = NULL;
-			break;
+			if (bh) brelse(bh);
+			return inode_number;
 		}
 	}
-	if (page) free_page((unsigned long) page);
 	if (bh) brelse(bh);
-	if(de_not_in_buf) 
-		kfree(de);
-	return retval;
+	return 0;
 }
 
 struct dentry *isofs_lookup(struct inode * dir, struct dentry * dentry)
 {
 	unsigned long ino;
-	struct buffer_head * bh;
 	struct inode *inode;
+	struct page *page;
 
 #ifdef DEBUG
 	printk("lookup: %x %s\n",dir->i_ino, dentry->d_name.name);
 #endif
 	dentry->d_op = dir->i_sb->s_root->d_op;
 
-	bh = isofs_find_entry(dir, dentry, &ino);
+	page = alloc_page(GFP_USER);
+	ino = isofs_find_entry(dir, dentry, page_address(page), 1024 + page_address(page));
+	__free_page(page);
 
 	inode = NULL;
-	if (bh) {
-		brelse(bh);
-
-		inode = iget(dir->i_sb,ino);
+	if (ino) {
+		inode = iget(dir->i_sb, ino);
 		if (!inode)
 			return ERR_PTR(-EACCES);
 	}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Harald Koenig]

On Nov 17, Linus Torvalds wrote:

> 
> 
> On Fri, 17 Nov 2000, Harald Koenig wrote:
> > 
> > Linus:		0.380u 76.850s 1:19.12 97.6%    0+0k 0+0io 113pf+0w
> > Andries:	0.470u 97.220s 1:40.29 97.4%    0+0k 0+0io 112pf+0w
> 
> The biggest difference is just the system times and the fact that it's
> more efficient coding. 
> 
> > BUT: there are some obvious bugs in the output of "du" and "find".
> > some samples (all file names (should) match the format "xe%03d/xe%03d.%c%c"
> > with both %03d being the _same_ number and both %c are in [a-z0-9]).
> 
> Yes. There's a silly bug there, now that I've tested it a bit. Basically
> the test for stuff that traversed a boundary was wrong.
> 
> The whole name conversion code is pretty horrible. It's been written over
> the years, and it was doing the same thing with small modifications in
> both readdir() and lookup(). I've got a cleaned up version that also
> should have the above bug fixed.
> 
> Still ready to test? This time I went over the files rather carefully, and
> while I've not tested the fixed version I'm getting pretty happy with it.

better you'd have tested it;)  while Andries' patch works fine (2 CDs of
data copied and checked a bit, seems to work ok with no obvious problems)
your new patch still shows a number of problems:

I've got a SIGSEGV in "find" and the following kernel messages (through ksymops):

File unit size != 0 for ISO file (403610326).
Interleaved files not (yet) supported.
File unit size != 0 for ISO file (403611545).
Interleaved files not (yet) supported.
File unit size != 0 for ISO file (403612080).
kernel BUG at slab.c:1542!
invalid operand: 0000
CPU:    0
EIP:    0010:[<c0129b20>]
EFLAGS: 00010286
eax: 0000001b   ebx: c04a21dd   ecx: c63b6000   edx: 00000001
esi: c04a2155   edi: c2282000   ebp: 00000024   esp: c63b7e60
ds: 0018   es: 0018   ss: 0018
Process find (pid: 30666, stackpage=c63b7000)
Stack: c01e5828 c01e58a8 00000606 c04a21dd c04a2155 c2282000 00000024 c63b6000 
       c8857c1e d6c9b662 00000007 00000025 c04a2155 c04a2176 00000090 00000000 
       00000000 00000000 00000000 d6c9b662 9006fe25 f9af4843 c04a2201 ffffffe4 
Call Trace: [<c01e5828>] [<c01e58a8>] [<c8857c1e>] [<d6c9b662>] [<d6c9b662>] [<f9af4843>] [<c88552a8>] 
       [<c88553ed>] [<c013b3b6>] [<c013b849>] [<c013b09a>] [<c013c31c>] [<c0138af6>] [<c0130687>] [<c010a69b>] 
       [<c010002b>] 
Code: 0f 0b 83 c4 0c 31 c0 5b 5e 5f 5d 59 c3 8d 76 00 83 ec 04 57 

>>EIP: c0129b20 <kmalloc+100/110>
Trace: c01e5828 <tvecs+1660/d1d8>
Trace: c01e58a8 <tvecs+16e0/d1d8>
Trace: c8857c1e <nlm_procname+2d7a2/30bd4>
Trace: d6c9b662 <END_OF_CODE+e43f1d6/????>
Trace: d6c9b662 <END_OF_CODE+e43f1d6/????>
Trace: f9af4843 <END_OF_CODE+312983b7/????>
Trace: c88552a8 <nlm_procname+2ae2c/30bd4>
Trace: c88553ed <nlm_procname+2af71/30bd4>
Trace: c010002b <startup_32+2b/cc>
Code:  c0129b20 <kmalloc+100/110>              00000000 <_EIP>: <===
Code:  c0129b20 <kmalloc+100/110>                 0:    0f 0b                   ud2a    <===
Code:  c0129b22 <kmalloc+102/110>                 2:    83 c4 0c                addl   $0xc,%esp
Code:  c0129b25 <kmalloc+105/110>                 5:    31 c0                   xorl   %eax,%eax
Code:  c0129b27 <kmalloc+107/110>                 7:    5b                      popl   %ebx
Code:  c0129b28 <kmalloc+108/110>                 8:    5e                      popl   %esi
Code:  c0129b29 <kmalloc+109/110>                 9:    5f                      popl   %edi
Code:  c0129b2a <kmalloc+10a/110>                 a:    5d                      popl   %ebp
Code:  c0129b2b <kmalloc+10b/110>                 b:    59                      popl   %ecx
Code:  c0129b2c <kmalloc+10c/110>                 c:    c3                      ret    
Code:  c0129b2d <kmalloc+10d/110>                 d:    8d 76 00                leal   0x0(%esi),%esi
Code:  c0129b30 <kmem_cache_free+0/80>           10:    83 ec 04                subl   $0x4,%esp
Code:  c0129b33 <kmem_cache_free+3/80>           13:    57                      pushl  %edi



trying to read/copy the data on a CD using tar still gives strange
file/directory names (output of find looks ok though) as "du" output
of the copy shows (all directories should be format "xe%03d", no 
further subdirectories!  xe283.c8 is a _filename_, not subdir...):

105718  xe280
12842   xe281
13738   xe282
0       xe283/xe283.c8
0       xe283/xe283.2c
50243   xe283
0       xe284/xe284.0a
12851   xe284
12418   xe285
0       xe286/xe286.pc
0       xe286/xe286.fg
42651   xe286
10914   xe287
10710   xe288
6890    xe289
36      xe290
0       xe291/xe291.0a
5719    xe291
5586    xe292
5902    xe293
0       xe294/xe294.h6
0       xe294/xe294.dq
0       xe294/xe294.ai
32916   xe294
3542    xe295
5582    xe296
5734    xe297
334     xe298



> 
> I'll merge some more of the name translation logic, but before I do that
> here's the newest patch..
> 
> 		Linus
> 
> -----
> diff -u --recursive --new-file v2.4.0-test10/linux/fs/isofs/dir.c linux/fs/isofs/dir.c
> --- v2.4.0-test10/linux/fs/isofs/dir.c	Fri Aug 11 14:29:01 2000
> +++ linux/fs/isofs/dir.c	Fri Nov 17 15:43:36 2000
> @@ -40,14 +40,17 @@
>  	lookup:		isofs_lookup,
>  };
>  
> -static int isofs_name_translate(char * old, int len, char * new)
> +int isofs_name_translate(struct iso_directory_record *de, char *new, struct inode *inode)
>  {
> -	int i, c;
> +	char * old = de->name;
> +	int len = de->name_len[0];
> +	int i;
>  			
>  	for (i = 0; i < len; i++) {
> -		c = old[i];
> +		unsigned char c = old[i];
>  		if (!c)
>  			break;
> +
>  		if (c >= 'A' && c <= 'Z')
>  			c |= 0x20;	/* lower case */
>  
> @@ -74,8 +77,7 @@
>  {
>  	int std;
>  	unsigned char * chr;
> -	int retnamlen = isofs_name_translate(de->name,
> -				de->name_len[0], retname);
> +	int retnamlen = isofs_name_translate(de, retname, inode);
>  	if (retnamlen == 0) return 0;
>  	std = sizeof(struct iso_directory_record) + de->name_len[0];
>  	if (std & 1) std++;
> @@ -105,7 +107,7 @@
>  	unsigned char bufbits = ISOFS_BUFFER_BITS(inode);
>  	unsigned int block, offset;
>  	int inode_number = 0;	/* Quiet GCC */
> -	struct buffer_head *bh;
> +	struct buffer_head *bh = NULL;
>  	int len;
>  	int map;
>  	int high_sierra;
> @@ -117,46 +119,22 @@
>  		return 0;
>   
>  	offset = filp->f_pos & (bufsize - 1);
> -	block = isofs_bmap(inode, filp->f_pos >> bufbits);
> +	block = filp->f_pos >> bufbits;
>  	high_sierra = inode->i_sb->u.isofs_sb.s_high_sierra;
>  
> -	if (!block)
> -		return 0;
> -
> -	if (!(bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size)))
> -		return 0;
> -
>  	while (filp->f_pos < inode->i_size) {
>  		int de_len;
> -#ifdef DEBUG
> -		printk("Block, offset, f_pos: %x %x %x\n",
> -		       block, offset, filp->f_pos);
> -	        printk("inode->i_size = %x\n",inode->i_size);
> -#endif
> -		/* Next directory_record on next CDROM sector */
> -		if (offset >= bufsize) {
> -#ifdef DEBUG
> -			printk("offset >= bufsize\n");
> -#endif
> -			brelse(bh);
> -			offset = 0;
> -			block = isofs_bmap(inode, (filp->f_pos) >> bufbits);
> -			if (!block)
> -				return 0;
> -			bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size);
> +
> +		if (!bh) {
> +			bh = isofs_bread(inode, bufsize, block);
>  			if (!bh)
>  				return 0;
> -			continue;
>  		}
>  
>  		de = (struct iso_directory_record *) (bh->b_data + offset);
> -		if(first_de) inode_number = (block << bufbits) + (offset & (bufsize - 1));
> +		if (first_de) inode_number = (bh->b_blocknr << bufbits) + offset;
>  
>  		de_len = *(unsigned char *) de;
> -#ifdef DEBUG
> -		printk("de_len = %d\n", de_len);
> -#endif
> -	    
>  
>  		/* If the length byte is zero, we should move on to the next
>  		   CDROM sector.  If we are at the end of the directory, we
> @@ -164,36 +142,33 @@
>  
>  		if (de_len == 0) {
>  			brelse(bh);
> -			filp->f_pos = ((filp->f_pos & ~(ISOFS_BLOCK_SIZE - 1))
> -				       + ISOFS_BLOCK_SIZE);
> +			bh = NULL;
> +			filp->f_pos = ((filp->f_pos & ~(ISOFS_BLOCK_SIZE - 1)) + ISOFS_BLOCK_SIZE);
> +			block = filp->f_pos >> bufbits;
>  			offset = 0;
> -
> -			if (filp->f_pos >= inode->i_size)
> -				return 0;
> -
> -			block = isofs_bmap(inode, (filp->f_pos) >> bufbits);
> -			if (!block)
> -				return 0;
> -			bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size);
> -			if (!bh)
> -				return 0;
>  			continue;
>  		}
>  
> -		offset +=  de_len;
> -		if (offset > bufsize) {
> -			/*
> -			 * This would only normally happen if we had
> -			 * a buggy cdrom image.  All directory
> -			 * entries should terminate with a null size
> -			 * or end exactly at the end of the sector.
> -			 */
> -		        printk("next_offset (%x) > bufsize (%lx)\n",
> -			       offset,bufsize);
> -			break;
> +		offset += de_len;
> +
> +		/* Make sure we have a full directory entry */
> +		if (offset >= bufsize) {
> +			int slop = bufsize - offset + de_len;
> +			memcpy(tmpde, de, slop);
> +			offset &= bufsize - 1;
> +			block++;
> +			brelse(bh);
> +			bh = NULL;
> +			if (offset) {
> +				bh = isofs_bread(inode, bufsize, block);
> +				if (!bh)
> +					return 0;
> +				memcpy((void *) tmpde + slop, bh->b_data, de_len - slop);
> +			}
> +			de = tmpde;				
>  		}
>  
> -		if(de->flags[-high_sierra] & 0x80) {
> +		if (de->flags[-high_sierra] & 0x80) {
>  			first_de = 0;
>  			filp->f_pos += de_len;
>  			continue;
> @@ -240,7 +215,7 @@
>  		if (map) {
>  #ifdef CONFIG_JOLIET
>  			if (inode->i_sb->u.isofs_sb.s_joliet_level) {
> -				len = get_joliet_filename(de, inode, tmpname);
> +				len = get_joliet_filename(de, tmpname, inode);
>  				p = tmpname;
>  			} else
>  #endif
> @@ -249,8 +224,7 @@
>  				p = tmpname;
>  			} else
>  			if (inode->i_sb->u.isofs_sb.s_mapping == 'n') {
> -				len = isofs_name_translate(de->name,
> -					de->name_len[0], tmpname);
> +				len = isofs_name_translate(de, tmpname, inode);
>  				p = tmpname;
>  			} else {
>  				p = de->name;
> @@ -265,7 +239,7 @@
>  
>  		continue;
>  	}
> -	brelse(bh);
> +	if (bh) brelse(bh);
>  	return 0;
>  }
>  
> diff -u --recursive --new-file v2.4.0-test10/linux/fs/isofs/inode.c linux/fs/isofs/inode.c
> --- v2.4.0-test10/linux/fs/isofs/inode.c	Tue Jul 18 21:40:47 2000
> +++ linux/fs/isofs/inode.c	Fri Nov 17 15:15:07 2000
> @@ -972,14 +972,24 @@
>  	return 0;
>  }
>  
> +struct buffer_head *isofs_bread(struct inode *inode, unsigned int bufsize, unsigned int block)
> +{
> +	unsigned int blknr = isofs_bmap(inode, block);
> +	if (!blknr)
> +		return NULL;
> +	return bread(inode->i_dev, blknr, bufsize);
> +}
> +
>  static int isofs_readpage(struct file *file, struct page *page)
>  {
>  	return block_read_full_page(page,isofs_get_block);
>  }
> +
>  static int _isofs_bmap(struct address_space *mapping, long block)
>  {
>  	return generic_block_bmap(mapping,block,isofs_get_block);
>  }
> +
>  static struct address_space_operations isofs_aops = {
>  	readpage: isofs_readpage,
>  	sync_page: block_sync_page,
> diff -u --recursive --new-file v2.4.0-test10/linux/fs/isofs/joliet.c linux/fs/isofs/joliet.c
> --- v2.4.0-test10/linux/fs/isofs/joliet.c	Tue Jul 18 22:48:32 2000
> +++ linux/fs/isofs/joliet.c	Fri Nov 17 15:29:55 2000
> @@ -70,8 +70,7 @@
>  }
>  
>  int
> -get_joliet_filename(struct iso_directory_record * de, struct inode * inode,
> -		    unsigned char *outname)
> +get_joliet_filename(struct iso_directory_record * de, unsigned char *outname, struct inode * inode)
>  {
>  	unsigned char utf8;
>  	struct nls_table *nls;
> diff -u --recursive --new-file v2.4.0-test10/linux/fs/isofs/namei.c linux/fs/isofs/namei.c
> --- v2.4.0-test10/linux/fs/isofs/namei.c	Mon May 10 14:14:28 1999
> +++ linux/fs/isofs/namei.c	Fri Nov 17 15:43:19 2000
> @@ -57,147 +57,87 @@
>   * itself (as an inode number). It does NOT read the inode of the
>   * entry - you'll have to do that yourself if you want to.
>   */
> -static struct buffer_head *
> -isofs_find_entry(struct inode *dir, struct dentry *dentry, unsigned long *ino)
> +static unsigned long
> +isofs_find_entry(struct inode *dir, struct dentry *dentry,
> +	char * tmpname, struct iso_directory_record * tmpde)
>  {
> +	unsigned long inode_number;
>  	unsigned long bufsize = ISOFS_BUFFER_SIZE(dir);
>  	unsigned char bufbits = ISOFS_BUFFER_BITS(dir);
> -	unsigned int block, i, f_pos, offset, 
> -		inode_number = 0; /* shut gcc up */
> -	struct buffer_head * bh , * retval = NULL;
> -	unsigned int old_offset;
> -	int dlen, match;
> -	char * dpnt;
> -	unsigned char *page = NULL;
> -	struct iso_directory_record * de = NULL; /* shut gcc up */
> -	char de_not_in_buf = 0;	  /* true if de is in kmalloc'd memory */
> -	char c;
> -
> -	*ino = 0;
> -	
> -	if (!(block = dir->u.isofs_i.i_first_extent)) return NULL;
> +	unsigned int block, f_pos, offset;
> +	struct buffer_head * bh = NULL;
> +
> +	if (!dir->u.isofs_i.i_first_extent)
> +		return 0;
>    
>  	f_pos = 0;
> +	offset = 0;
> +	block = 0;
>  
> -	offset = f_pos & (bufsize - 1);
> -	block = isofs_bmap(dir,f_pos >> bufbits);
> +	while (f_pos < dir->i_size) {
> +		struct iso_directory_record * de;
> +		int de_len, match, i, dlen;
> +		char *dpnt;
>  
> -	if (!block || !(bh = bread(dir->i_dev,block,bufsize))) return NULL;
> +		if (!bh) {
> +			bh = isofs_bread(dir, bufsize, block);
> +			if (!bh)
> +				return 0;
> +		}
>  
> -	while (f_pos < dir->i_size) {
> +		de = (struct iso_directory_record *) (bh->b_data + offset);
> +		inode_number = (bh->b_blocknr << bufbits) + offset;
>  
> -		/* if de is in kmalloc'd memory, do not point to the
> -                   next de, instead we will move to the next sector */
> -		if(!de_not_in_buf) {
> -			de = (struct iso_directory_record *) 
> -				(bh->b_data + offset);
> -		}
> -		inode_number = (block << bufbits) + (offset & (bufsize - 1));
> -
> -		/* If byte is zero, or we had to fetch this de past
> -		   the end of the buffer, this is the end of file, or
> -		   time to move to the next sector. Usually 2048 byte
> -		   boundaries. */
> -		
> -		if (*((unsigned char *) de) == 0 || de_not_in_buf) {
> -			if(de_not_in_buf) {
> -				/* james@bpgc.com: Since we slopped
> -                                   past the end of the last buffer, we
> -                                   must start some way into the new
> -                                   one */
> -				de_not_in_buf = 0;
> -				kfree(de);
> -				f_pos += offset;
> -			}
> -			else { 
> -				offset = 0;
> -				f_pos = ((f_pos & ~(ISOFS_BLOCK_SIZE - 1))
> -					 + ISOFS_BLOCK_SIZE);
> -			}
> +		de_len = *(unsigned char *) de;
> +		if (!de_len) {
>  			brelse(bh);
>  			bh = NULL;
> -
> -			if (f_pos >= dir->i_size) 
> -				break;
> -
> -			block = isofs_bmap(dir,f_pos>>bufbits);
> -			if (!block || !(bh = bread(dir->i_dev,block,bufsize)))
> -				break;
> -
> -			continue; /* Will kick out if past end of directory */
> +			f_pos = (f_pos + ISOFS_BLOCK_SIZE) & ~(ISOFS_BLOCK_SIZE - 1);
> +			block = f_pos >> bufbits;
> +			offset = 0;
> +			continue;
>  		}
>  
> -		old_offset = offset;
> -		offset += *((unsigned char *) de);
> -		f_pos += *((unsigned char *) de);
> +		offset += de_len;
> +		f_pos += de_len;
>  
> -		/* james@bpgc.com: new code to handle case where the
> -		   directory entry spans two blocks.  Usually 1024
> -		   byte boundaries */
> +		/* Make sure we have a full directory entry */
>  		if (offset >= bufsize) {
> -			struct buffer_head *bh_next;
> -
> -			/* james@bpgc.com: read the next block, and
> -                           copy the split de into a newly kmalloc'd
> -                           buffer */
> -			block = isofs_bmap(dir,f_pos>>bufbits);
> -			if (!block || 
> -			    !(bh_next = bread(dir->i_dev,block,bufsize)))
> -				break;
> -			
> -			de = (struct iso_directory_record *)
> -				kmalloc(offset - old_offset, GFP_KERNEL);
> -			memcpy((char *)de, bh->b_data + old_offset, 
> -			       bufsize - old_offset);
> -			memcpy((char *)de + bufsize - old_offset,
> -			       bh_next->b_data, offset - bufsize);
> -			brelse(bh_next);
> -			de_not_in_buf = 1;
> -			offset -= bufsize;
> +			int slop = bufsize - offset + de_len;
> +			memcpy(tmpde, de, slop);
> +			offset &= bufsize - 1;
> +			block++;
> +			brelse(bh);
> +			bh = NULL;
> +			if (offset) {
> +				bh = isofs_bread(dir, bufsize, block);
> +				if (!bh)
> +					return 0;
> +				memcpy((void *) tmpde + slop, bh->b_data, de_len - slop);
> +			}
> +			de = tmpde;
>  		}
> +
>  		dlen = de->name_len[0];
>  		dpnt = de->name;
>  
> -		if (dir->i_sb->u.isofs_sb.s_rock ||
> -		    dir->i_sb->u.isofs_sb.s_joliet_level || 
> -		    dir->i_sb->u.isofs_sb.s_mapping == 'n' ||
> -		    dir->i_sb->u.isofs_sb.s_mapping == 'a') {
> -			if (! page) {
> -				page = (unsigned char *)
> -					__get_free_page(GFP_KERNEL);
> -				if (!page) break;
> -			}
> -		}
>  		if (dir->i_sb->u.isofs_sb.s_rock &&
> -		    ((i = get_rock_ridge_filename(de, page, dir)))) {
> +		    ((i = get_rock_ridge_filename(de, tmpname, dir)))) {
>  			dlen = i;
> -			dpnt = page;
> +			dpnt = tmpname;
>  #ifdef CONFIG_JOLIET
>  		} else if (dir->i_sb->u.isofs_sb.s_joliet_level) {
> -			dlen = get_joliet_filename(de, dir, page);
> -			dpnt = page;
> +			dlen = get_joliet_filename(de, dir, tmpname);
> +			dpnt = tmpname;
>  #endif
>  		} else if (dir->i_sb->u.isofs_sb.s_mapping == 'a') {
> -			dlen = get_acorn_filename(de, page, dir);
> -			dpnt = page;
> +			dlen = get_acorn_filename(de, tmpname, dir);
> +			dpnt = tmpname;
>  		} else if (dir->i_sb->u.isofs_sb.s_mapping == 'n') {
> -			for (i = 0; i < dlen; i++) {
> -				c = dpnt[i];
> -				/* lower case */
> -				if (c >= 'A' && c <= 'Z') c |= 0x20;
> -				if (c == ';' && i == dlen-2 && dpnt[i+1] == '1') {
> -					dlen -= 2;
> -					break;
> -				}
> -				if (c == ';') c = '.';
> -				page[i] = c;
> -			}
> -			/* This allows us to match with and without
> -			 * a trailing period. */
> -			if(page[dlen-1] == '.' && dentry->d_name.len == dlen-1)
> -				dlen--;
> -			dpnt = page;
> +			dlen = isofs_name_translate(de, tmpname, dir);
> +			dpnt = tmpname;
>  		}
> +
>  		/*
>  		 * Skip hidden or associated files unless unhide is set 
>  		 */
> @@ -208,43 +148,32 @@
>  			match = (isofs_cmp(dentry,dpnt,dlen) == 0);
>  		}
>  		if (match) {
> -			if(inode_number == -1) {
> -				/* Should only happen for the '..' entry */
> -				inode_number = 
> -					isofs_lookup_grandparent(dir,
> -					   find_rock_ridge_relocation(de,dir));
> -			}
> -			*ino = inode_number;
> -			retval = bh;
> -			bh = NULL;
> -			break;
> +			if (bh) brelse(bh);
> +			return inode_number;
>  		}
>  	}
> -	if (page) free_page((unsigned long) page);
>  	if (bh) brelse(bh);
> -	if(de_not_in_buf) 
> -		kfree(de);
> -	return retval;
> +	return 0;
>  }
>  
>  struct dentry *isofs_lookup(struct inode * dir, struct dentry * dentry)
>  {
>  	unsigned long ino;
> -	struct buffer_head * bh;
>  	struct inode *inode;
> +	struct page *page;
>  
>  #ifdef DEBUG
>  	printk("lookup: %x %s\n",dir->i_ino, dentry->d_name.name);
>  #endif
>  	dentry->d_op = dir->i_sb->s_root->d_op;
>  
> -	bh = isofs_find_entry(dir, dentry, &ino);
> +	page = alloc_page(GFP_USER);
> +	ino = isofs_find_entry(dir, dentry, page_address(page), 1024 + page_address(page));
> +	__free_page(page);
>  
>  	inode = NULL;
> -	if (bh) {
> -		brelse(bh);
> -
> -		inode = iget(dir->i_sb,ino);
> +	if (ino) {
> +		inode = iget(dir->i_sb, ino);
>  		if (!inode)
>  			return ERR_PTR(-EACCES);
>  	}
> 


Harald
-- 
All SCSI disks will from now on                     ___       _____
be required to send an email notice                0--,|    /OOOOOOO\
24 hours prior to complete hardware failure!      <_/  /  /OOOOOOOOOOO\
                                                    \  \/OOOOOOOOOOOOOOO\
                                                      \ OOOOOOOOOOOOOOOOO|//
Harald Koenig,                                         \/\/\/\/\/\/\/\/\/
Inst.f.Theoret.Astrophysik                              //  /     \\  \
koenig@tat.physik.uni-tuebingen.de                     ^^^^^       ^^^^^
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Linus Torvalds]

Oh, and sorry - the last patch doesn't contain the (obvious) fixes to the
header files to take some of the calling convention changes into account.


		Linus

---
--- v2.4.0-test10/linux/include/linux/iso_fs.h	Fri Sep  8 12:52:56 2000
+++ linux/include/linux/iso_fs.h	Fri Nov 17 15:52:03 2000
@@ -177,16 +177,17 @@
 
 extern int parse_rock_ridge_inode(struct iso_directory_record *, struct inode *);
 extern int get_rock_ridge_filename(struct iso_directory_record *, char *, struct inode *);
+extern int isofs_name_translate(struct iso_directory_record *, char *, struct inode *);
 
 extern int find_rock_ridge_relocation(struct iso_directory_record *, struct inode *);
 
-int get_joliet_filename(struct iso_directory_record *, struct inode *, unsigned char *);
+int get_joliet_filename(struct iso_directory_record *, unsigned char *, struct inode *);
 int get_acorn_filename(struct iso_directory_record *, char *, struct inode *);
 
 extern struct dentry *isofs_lookup(struct inode *, struct dentry *);
 extern int isofs_get_block(struct inode *, long, struct buffer_head *, int);
 extern int isofs_bmap(struct inode *, int);
-extern int isofs_lookup_grandparent(struct inode *, int);
+extern struct buffer_head *isofs_bread(struct inode *, unsigned int, unsigned int);
 
 extern struct inode_operations isofs_dir_inode_operations;
 extern struct file_operations isofs_dir_operations;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Andries.Brouwer]

> both 2.2.x and 2.4.x kernels can't read `real sky' CDs

Yes. 2.0.38 is OK. I just made a patch that seems to work.

Harald, could you try
	ftp.xx.kernel.org/.../people/aeb/linux-2.4.0test9-isofs-patch
and report?

Linus, Alan - I made patches for 2.2 and 2.4 but want to
polish and check them a bit more before submitting.
There also seem to be a lot of bug reports in newsgroups
and mailing lists - must check whether people complain
about the same thing or whether there are more problems.

Andries

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Harald Koenig]

[-- Attachment #1: Type: text/plain, Size: 1806 bytes --]

On Nov 17, Andries.Brouwer@cwi.nl wrote:

> > both 2.2.x and 2.4.x kernels can't read `real sky' CDs
> 
> Yes. 2.0.38 is OK. I just made a patch that seems to work.
> 
> Harald, could you try
> 	ftp.xx.kernel.org/.../people/aeb/linux-2.4.0test9-isofs-patch
> and report?

works -- sort of:(   I've tried both test9 and test10 with your patch
on a PPro200 with 128MB ram and I get the same effects both times:

directory listing seems to be possible (haven't checked data contents yet)
BUT if I try "du /cdrom/"  (either using real cdrom device or loopback mount
of my sample isofs image) there seems to be a huge memory leak !

first observation is that "du" is awfuly slow -- it takes ~90 secs real time
and ~60 system cpu secs to "du" through the first ~70 of 106 direcories, 
then the 128MB memory are almost used up and the systems starts to
swap heavily.  this meory doesn't get freed up even after umount/losetup -d
or whatever -- only reboot "helps"...


I'll attach log files showing output of "free", /proc/meminfo and
output of ALT-SYSREQ-M plus full "ps" output  for both -test9 and 
-test10 with your patch  in the situation when almost all memory 
is "gone" (du already killed).  hope this helps...



Harald
-- 
All SCSI disks will from now on                     ___       _____
be required to send an email notice                0--,|    /OOOOOOO\
24 hours prior to complete hardware failure!      <_/  /  /OOOOOOOOOOO\
                                                    \  \/OOOOOOOOOOOOOOO\
                                                      \ OOOOOOOOOOOOOOOOO|//
Harald Koenig,                                         \/\/\/\/\/\/\/\/\/
Inst.f.Theoret.Astrophysik                              //  /     \\  \
koenig@tat.physik.uni-tuebingen.de                     ^^^^^       ^^^^^

[-- Attachment #2: log-test9.gz --]
[-- Type: application/x-gunzip, Size: 4175 bytes --]

[-- Attachment #3: log-test10.gz --]
[-- Type: application/x-gunzip, Size: 3274 bytes --]

Re: BUG: isofs broken (2.2 and 2.4)

[Andries.Brouwer]

> Anybody else willing to finish this one off?

If noone else does, I suppose I can.

(> .. gets ENOENT ..
and that is not because it only is a partial image?)

Andries


PS - Yesterday I complained that 2.4.0test9 was fine
but 2.4.0test11pre5 dies as soon as it has to forward a ping.
The effect is reproducible, and 2.4.0test10 is also fine.
I see no changes in the netfilter code.
Will look some more into this tomorrow.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Linus Torvalds]

On Thu, 16 Nov 2000 Andries.Brouwer@cwi.nl wrote:
> 
> If noone else does, I suppose I can.

Thanks.

> 
> (> .. gets ENOENT ..
> and that is not because it only is a partial image?)

I don't think so, but I obviously have no way of actually confirming my
suspicion.

If the stat information was wrong due to the partial image, the lookup
should still have succeeded (the directory entries certainly were there -
otherwise they'd not have shown up in readdir), and we would just have
gotten garbage inode information etc. I think.

			Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

BUG: isofs broken (2.2 and 2.4)

[Harald Koenig]

Hi,

both 2.2.x and 2.4.x kernels can't read `real sky' CDs from the
Space Telescope Science Institute containing lotsof directories (~100) 
which each contain lots of small files (~700 files/dir).  only ~10 directories
with ~10 files each are displayed, all the other files/diretories can't be 
accessed. the kernel gives the following message:

	next_offset (212) > bufsize (200)

and with 2.2.x kernels I additionally get

	Invalid session number or type of track

at mount time (that's the 2nd instance of this message, i == -22 (RTFS)).



you can find an isofs image for testing (only directory part, no real data,
compressed ~620kb) on

	http://www.tat.physik.uni-tuebingen.de/~koenig/buggy_fs.iso.gz



any idea/patch/fix ?

thanks,


Harald

PS:  I'm not subscribed to linux-kernel right now, so please 
reply directly using Cc:.   thanks!
--
All SCSI disks will from now on                     ___       _____
be required to send an email notice                0--,|    /OOOOOOO\
24 hours prior to complete hardware failure!      <_/  /  /OOOOOOOOOOO\
                                                    \  \/OOOOOOOOOOOOOOO\
                                                      \ OOOOOOOOOOOOOOOOO|//
Harald Koenig,                                         \/\/\/\/\/\/\/\/\/
Inst.f.Theoret.Astrophysik                              //  /     \\  \
koenig@tat.physik.uni-tuebingen.de                     ^^^^^       ^^^^^
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Andries Brouwer]

On Wed, Nov 15, 2000 at 08:23:44PM +0100, Harald Koenig wrote:

> both 2.2.x and 2.4.x kernels can't read `real sky' CDs from the
> Space Telescope Science Institute containing lotsof directories (~100) 
> which each contain lots of small files (~700 files/dir).
> only ~10 directories with ~10 files each are displayed,
> all the other files/diretories can't be accessed.
> the kernel gives the following message:
> 
> 	next_offset (212) > bufsize (200)

Has there been a kernel version that could read these?
It looks like it proclaims blocksize 512 and uses blocksize 2048 or so.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Linus Torvalds]

On Thu, 16 Nov 2000, Andries Brouwer wrote:
> 
> Has there been a kernel version that could read these?
> It looks like it proclaims blocksize 512 and uses blocksize 2048 or so.

The (de_len == 0) check in do_isofs_readdir() seems to imply that the
blocksize is always 2048. So at the very least something is inconsistent.
We use ISOFS_BUFFER_SIZE(inode) (512 in this case) for some sector sizes,
and then ISOFS_BLOCK_SIZE (2048) for others. 

But the way isofs_bmap() works, we need to work with
ISOFS_BUFFER_SIZE(inode). And I don't know if directories are always
_aligned_ at 2048 bytes even if they should be blocked at 2k.

Looking at the isofs lookup() logic, it will actually handle split
entries, instead of complaining about them. And I suspect readdir() did
too at some point, and the code was just removed (probably due to
excessive confusion) when one of the many readdir() reorganizations was
done. 

readdir() probably worked a long time ago.

Is the thing documented somewhere? It looks like we should just allow
entries that are split and not complain about them. We have the temporary
buffer for it already..

		Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Linus Torvalds]

Does this patch fix it for you?

Warning: TOTALLY UNTESTED!!! Please test carefully.

Also, I'd be interested to know whether somebody really knows if the zero
length handling is correct. Should we really round up to 2048, or should
we perhaps round up only to the next bufsize?

		Linus

-----
--- v2.4.0-test10/linux/fs/isofs/dir.c	Fri Aug 11 14:29:01 2000
+++ linux/fs/isofs/dir.c	Wed Nov 15 17:14:26 2000
@@ -94,6 +94,14 @@
 	return retnamlen;
 }
 
+static struct buffer_head *isofs_bread(struct inode *inode, unsigned int bufsize, unsigned int block)
+{
+	unsigned int blknr = isofs_bmap(inode, block);
+	if (!blknr)
+		return NULL;
+	return bread(inode->i_dev, blknr, bufsize);
+}
+
 /*
  * This should _really_ be cleaned up some day..
  */
@@ -105,7 +113,7 @@
 	unsigned char bufbits = ISOFS_BUFFER_BITS(inode);
 	unsigned int block, offset;
 	int inode_number = 0;	/* Quiet GCC */
-	struct buffer_head *bh;
+	struct buffer_head *bh = NULL;
 	int len;
 	int map;
 	int high_sierra;
@@ -117,46 +125,25 @@
 		return 0;
  
 	offset = filp->f_pos & (bufsize - 1);
-	block = isofs_bmap(inode, filp->f_pos >> bufbits);
+	block = filp->f_pos >> bufbits;
 	high_sierra = inode->i_sb->u.isofs_sb.s_high_sierra;
 
-	if (!block)
-		return 0;
-
-	if (!(bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size)))
-		return 0;
-
 	while (filp->f_pos < inode->i_size) {
 		int de_len;
-#ifdef DEBUG
-		printk("Block, offset, f_pos: %x %x %x\n",
-		       block, offset, filp->f_pos);
-	        printk("inode->i_size = %x\n",inode->i_size);
-#endif
-		/* Next directory_record on next CDROM sector */
-		if (offset >= bufsize) {
-#ifdef DEBUG
-			printk("offset >= bufsize\n");
-#endif
-			brelse(bh);
-			offset = 0;
-			block = isofs_bmap(inode, (filp->f_pos) >> bufbits);
-			if (!block)
-				return 0;
-			bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size);
+
+		if (!bh) {
+			bh = isofs_bread(inode, bufsize, block);
 			if (!bh)
 				return 0;
-			continue;
 		}
 
 		de = (struct iso_directory_record *) (bh->b_data + offset);
-		if(first_de) inode_number = (block << bufbits) + (offset & (bufsize - 1));
+		if (first_de) inode_number = (block << bufbits) + (offset & (bufsize - 1));
 
 		de_len = *(unsigned char *) de;
 #ifdef DEBUG
 		printk("de_len = %d\n", de_len);
-#endif
-	    
+#endif	    
 
 		/* If the length byte is zero, we should move on to the next
 		   CDROM sector.  If we are at the end of the directory, we
@@ -164,36 +151,36 @@
 
 		if (de_len == 0) {
 			brelse(bh);
-			filp->f_pos = ((filp->f_pos & ~(ISOFS_BLOCK_SIZE - 1))
-				       + ISOFS_BLOCK_SIZE);
+			bh = NULL;
+			filp->f_pos = ((filp->f_pos & ~(ISOFS_BLOCK_SIZE - 1)) + ISOFS_BLOCK_SIZE);
+			block = filp->f_pos >> bufbits;
 			offset = 0;
-
-			if (filp->f_pos >= inode->i_size)
-				return 0;
-
-			block = isofs_bmap(inode, (filp->f_pos) >> bufbits);
-			if (!block)
-				return 0;
-			bh = breada(inode->i_dev, block, bufsize, filp->f_pos, inode->i_size);
-			if (!bh)
-				return 0;
 			continue;
 		}
 
-		offset +=  de_len;
+		offset += de_len;
+		if (offset == bufsize) {
+			offset = 0;
+			block++;
+			brelse(bh);
+			bh = NULL;
+		}
+
+		/* Make sure we have a full directory entry */
 		if (offset > bufsize) {
-			/*
-			 * This would only normally happen if we had
-			 * a buggy cdrom image.  All directory
-			 * entries should terminate with a null size
-			 * or end exactly at the end of the sector.
-			 */
-		        printk("next_offset (%x) > bufsize (%lx)\n",
-			       offset,bufsize);
-			break;
+			int slop = bufsize - offset + de_len;
+			memcpy(tmpde, de, slop);
+			offset &= bufsize - 1;
+			block++;
+			brelse(bh);
+			bh = isofs_bread(inode, bufsize, block);
+			if (!bh)
+				return 0;
+			memcpy((void *) tmpde + slop, bh->b_data, de_len - slop);
+			de = tmpde;
 		}
 
-		if(de->flags[-high_sierra] & 0x80) {
+		if (de->flags[-high_sierra] & 0x80) {
 			first_de = 0;
 			filp->f_pos += de_len;
 			continue;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: BUG: isofs broken (2.2 and 2.4)

[Linus Torvalds]

On Wed, 15 Nov 2000, Linus Torvalds wrote:
> 
> Does this patch fix it for you?
> 
> Warning: TOTALLY UNTESTED!!! Please test carefully.

Ok, I tested it with the broken image.

It looks like "readdir()" is ok now (but not really knowing what the right
output should be I cannot guarantee that). HOWEVER, doing an "ls -l" on
some of the files gets ENOENT, implying that "lookup()" still has some
problems with the image.

I suspect the code to handle split entries in isofs_find_entry() has some
simple bug, but I'm too lazy to check it out right now. Anybody else
willing to finish this one off?

		Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/