Re: BUG: isofs broken (2.2 and 2.4)
From [EMAIL PROTECTED] 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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
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:
CPU:0
EIP:0010:[]
EFLAGS: 00010286
eax: 001b ebx: c04a21dd ecx: c63b6000 edx: 0001
esi: c04a2155 edi: c2282000 ebp: 0024 esp: c63b7e60
ds: 0018 es: 0018 ss: 0018
Process find (pid: 30666, stackpage=c63b7000)
Stack: c01e5828 c01e58a8 0606 c04a21dd c04a2155 c2282000 0024 c63b6000
c8857c1e d6c9b662 0007 0025 c04a2155 c04a2176 0090
d6c9b662 9006fe25 f9af4843 c04a2201 ffe4
Call Trace: [] [] [] [] []
[] []
[] [] [] [] [] []
[] []
[]
Code: 0f 0b 83 c4 0c 31 c0 5b 5e 5f 5d 59 c3 8d 76 00 83 ec 04 57
>>EIP: c0129b20
Trace: c01e5828
Trace: c01e58a8
Trace: c8857c1e
Trace: d6c9b662
Trace: d6c9b662
Trace: f9af4843
Trace: c88552a8
Trace: c88553ed
Trace: c010002b
Code: c0129b20 <_EIP>: <===
Code: c0129b20 0:0f 0b ud2a
<===
Code: c0129b22 2:83 c4 0caddl
$0xc,%esp
Code: c0129b25 5:31 c0 xorl
%eax,%eax
Code: c0129b27 7:5b popl
%ebx
Code: c0129b28 8:5e popl
%esi
Code: c0129b29 9:5f popl
%edi
Code: c0129b2a a:5d popl
%ebp
Code: c0129b2b b:59 popl
%ecx
Code: c0129b2c c:c3 ret
Code: c0129b2d d:8d 76 00leal
0x0(%esi),%esi
Code: c0129b3010:83 ec 04subl
$0x4,%esp
Code: c0129b3313: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
6890xe289
36 xe290
0 xe291/xe291.0a
5719xe291
5586xe292
5902xe293
0 xe294/xe294.h6
0 xe294/xe294.dq
0 xe294/xe294.ai
32916 xe294
3542xe295
5582xe296
5734xe297
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.cFri 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 c
Re: BUG: isofs broken (2.2 and 2.4)
On Sat, 18 Nov 2000, Keith Owens wrote: > On Fri, 17 Nov 2000 17:21:53 -0800 (PST), > Linus Torvalds <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
On Fri, 17 Nov 2000 17:21:53 -0800 (PST), Linus Torvalds <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
>> 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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
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.hFri 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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
On Sat, 18 Nov 2000 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
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.cFri 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);
+
Re: BUG: isofs broken (2.2 and 2.4)
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 [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
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/¶^¶ ¶¶p¶$¶^¶¶^¶^¶ ¶¶¶{}: 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 notice0--,|/OOO\
24 hours prior to complete hardware failure! <_/ / /OOO\
\ \/OOO\
\ O|//
Harald Koenig, \/\/\/\/\/\/\/\/\/
Inst.f.Theoret.Astrophysik // / \\ \
[EMAIL PROTECTED] ^ ^
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
On Nov 17, [EMAIL PROTECTED] 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 notice0--,|/OOO\
24 hours prior to complete hardware failure! <_/ / /OOO\
\ \/OOO\
\ O|//
Harald Koenig, \/\/\/\/\/\/\/\/\/
Inst.f.Theoret.Astrophysik // / \\ \
[EMAIL PROTECTED] ^ ^
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
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.cFri 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 */
+ i
Re: BUG: isofs broken (2.2 and 2.4)
On Nov 17, [EMAIL PROTECTED] 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 notice0--,|/OOO\ 24 hours prior to complete hardware failure! <_/ / /OOO\ \ \/OOO\ \ O|// Harald Koenig, \/\/\/\/\/\/\/\/\/ Inst.f.Theoret.Astrophysik // / \\ \ [EMAIL PROTECTED] ^ ^ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
> 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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
On Nov 17, [EMAIL PROTECTED] 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 notice0--,|/OOO\ 24 hours prior to complete hardware failure! <_/ / /OOO\ \ \/OOO\ \ O|// Harald Koenig, \/\/\/\/\/\/\/\/\/ Inst.f.Theoret.Astrophysik // / \\ \ [EMAIL PROTECTED] ^ ^ log-test9.gz log-test10.gz
Re: BUG: isofs broken (2.2 and 2.4)
> 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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
On Thu, 16 Nov 2000 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
> 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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
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.cWed 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);
+
Re: BUG: isofs broken (2.2 and 2.4)
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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: BUG: isofs broken (2.2 and 2.4)
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 [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
