lkml.org 
[lkml]   [2000]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: 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.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/

\
 
 \ /
  Last update: 2005-03-22 12:47    [W:0.145 / U:0.588 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site