lkml.org 
[lkml]   [1997]   [Jan]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectAgain - inode races [PATCH] Re: Corrupted inode list?
Steven, I hope this patch cures your problem...

Ok folks, I think I have seen the light on this one, and It really
is simple: give up on requiring put_inode not to sleep! I just
discovered that clear_inode can end up sleeping before it is
finished, plus any block writes and.... ick! This should even
fix the problems some people were having with disk quotas not
being properly updated (the drop in clear_inode).
The interesting portion of this patch:
a) make sure inode is locked when calling clear_inode
b) lock inode on calls to put_inode
c) various reordering of inode operations to avoid
even more races

The patch is against 2.0.28, but goes cleanly into 2.1.21. Perhaps
we can test it in 2.1.22 and integrate it into 2.0.29 if all is
well?

-b (blah@dot.superaje.com)

On 5 Jan 1997, Steven L Baur wrote:

> I am getting this error in spades on an IDE disk (2.0.27, no patches
> 200MHz Pentium with 64MB of RAM).
>
> ...
> Dec 31 03:03:50 altair kernel: EXT2-fs warning (device 16:02): ext2_free_inode: bit already cleared for inode 102442
> Dec 31 03:03:50 altair kernel: EXT2-fs warning (device 16:02): ext2_free_inode: bit already cleared for inode 102443
> Dec 31 03:03:50 altair kernel: EXT2-fs warning (device 16:02): ext2_free_inode: bit already cleared for inode 102444
> Dec 31 03:04:01 altair kernel: EXT2-fs warning (device 16:02): ext2_free_inode: bit already cleared for inode 102441
> ...
> Jan 3 12:20:42 altair kernel: EXT2-fs warning (device 16:02): ext2_free_inode: bit already cleared for inode 170081
>
> I've only detected it occuring on the machine's third disk, and it
> seems to bite 4 consecutive inodes at a time when it strikes.

----snip here people ;)----
diff -u --recursive dist/drivers/block/md.c linux/drivers/block/md.c
--- dist/drivers/block/md.c Sat Jun 29 17:04:00 1996
+++ linux/drivers/block/md.c Mon Jan 20 23:26:59 1997
@@ -215,9 +215,12 @@
}

/* Remove locks. */
- for (i=0; i<md_dev[minor].nb_dev; i++)
+ for (i=0; i<md_dev[minor].nb_dev; i++) {
+ lock_inode (md_dev[minor].devices[i].inode);
clear_inode (md_dev[minor].devices[i].inode);
-
+ unlock_inode (md_dev[minor].devices[i].inode);
+ }
+
md_dev[minor].nb_dev=md_size[minor]=0;
md_hd_struct[minor].nr_sects=0;
md_dev[minor].pers=NULL;
diff -u --recursive dist/fs/exec.c linux/fs/exec.c
--- dist/fs/exec.c Mon Sep 9 14:04:57 1996
+++ linux/fs/exec.c Mon Jan 20 23:26:59 1997
@@ -131,16 +131,17 @@
f->f_pos = 0;
f->f_reada = 0;
f->f_op = inode->i_op->default_file_ops;
+ inode->i_count++;
if (f->f_op->open) {
int error = f->f_op->open(inode,f);
if (error) {
f->f_count--;
put_unused_fd(fd);
+ iput(inode);
return error;
}
}
current->files->fd[fd] = f;
- inode->i_count++;
}
return fd;
}
diff -u --recursive dist/fs/inode.c linux/fs/inode.c
--- dist/fs/inode.c Wed Jul 24 00:03:07 1996
+++ linux/fs/inode.c Mon Jan 20 23:26:59 1997
@@ -145,13 +145,13 @@
__wait_on_inode(inode);
}

-static inline void lock_inode(struct inode * inode)
+void lock_inode(struct inode * inode)
{
wait_on_inode(inode);
inode->i_lock = 1;
}

-static inline void unlock_inode(struct inode * inode)
+void unlock_inode(struct inode * inode)
{
inode->i_lock = 0;
wake_up(&inode->i_wait);
@@ -159,7 +159,8 @@

/*
* Note that we don't want to disturb any wait-queues when we discard
- * an inode.
+ * an inode. Note also that the inode must be locked when clear_inode
+ * is called.
*
* Argghh. Got bitten by a gcc problem with inlining: no way to tell
* the compiler that the inline asm function 'memset' changes 'inode'.
@@ -168,13 +169,16 @@
*
* The solution is the weird use of 'volatile'. Ho humm. Have to report
* it to the gcc lists, and hope we can do this more cleanly some day..
+ *
+ * Doh!!! This has been another one of those places where races are
+ * bound to occur. Just remember that the inode must remain locked
+ * whenever we can sleep... Please!!! --blah
*/
void clear_inode(struct inode * inode)
{
struct wait_queue * wait;

- truncate_inode_pages(inode, 0);
- wait_on_inode(inode);
+ truncate_inode_pages(inode, 0); /* may sleep */
if (IS_WRITABLE(inode)) {
if (inode->i_sb && inode->i_sb->dq_op)
inode->i_sb->dq_op->drop(inode);
@@ -202,7 +206,9 @@
continue;
if (inode->i_count || inode->i_dirt || inode->i_lock)
return 0;
+ lock_inode(inode);
clear_inode(inode);
+ unlock_inode(inode);
}
return 1;
}
@@ -395,7 +401,9 @@
kdevname(dev));
continue;
}
+ lock_inode(inode);
clear_inode(inode);
+ unlock_inode(inode);
}
}

@@ -441,7 +449,9 @@
}

if (inode->i_sb && inode->i_sb->s_op && inode->i_sb->s_op->put_inode) {
- inode->i_sb->s_op->put_inode(inode);
+ inode->i_lock = 1;
+ inode->i_sb->s_op->put_inode(inode); /* sleeps here were once bad */
+ unlock_inode(inode);
if (!inode->i_nlink)
return;
}
@@ -523,6 +533,7 @@
if (best->i_count)
goto repeat;
found_good:
+ lock_inode(best);
clear_inode(best);
best->i_count = 1;
best->i_nlink = 1;
@@ -530,6 +541,7 @@
best->i_sem.count = 1;
best->i_ino = ++ino;
best->i_dev = 0;
+ unlock_inode(best);
nr_free_inodes--;
if (nr_free_inodes < 0) {
printk ("VFS: get_empty_inode: bad free inode count.\n");
diff -u --recursive dist/fs/namei.c linux/fs/namei.c
--- dist/fs/namei.c Mon Jan 20 23:27:22 1997
+++ linux/fs/namei.c Mon Jan 20 23:26:59 1997
@@ -169,11 +169,12 @@
*result = dir;
return 0;
} else if ((sb = dir->i_sb) && (dir == sb->s_mounted)) {
+ if (sb->s_covered)
+ sb->s_covered->i_count++; /* avoid the race */
iput(dir);
dir = sb->s_covered;
if (!dir)
return -ENOENT;
- dir->i_count++;
}
}
if (!dir->i_op || !dir->i_op->lookup) {
diff -u --recursive dist/fs/open.c linux/fs/open.c
--- dist/fs/open.c Tue Aug 20 04:59:19 1996
+++ linux/fs/open.c Mon Jan 20 23:26:59 1997
@@ -267,7 +267,7 @@

asmlinkage int sys_chdir(const char * filename)
{
- struct inode * inode;
+ struct inode * inode, *tmp;
int error;

error = namei(filename,&inode);
@@ -281,14 +281,15 @@
iput(inode);
return error;
}
- iput(current->fs->pwd);
+ tmp = current->fs->pwd;
current->fs->pwd = inode;
+ iput(tmp);
return (0);
}

asmlinkage int sys_fchdir(unsigned int fd)
{
- struct inode * inode;
+ struct inode * inode, *tmp;
struct file * file;
int error;

@@ -300,9 +301,10 @@
return -ENOTDIR;
if ((error = permission(inode,MAY_EXEC)) != 0)
return error;
- iput(current->fs->pwd);
+ tmp = current->fs->pwd;
current->fs->pwd = inode;
inode->i_count++;
+ iput(tmp);
return (0);
}

diff -u --recursive dist/include/linux/fs.h linux/include/linux/fs.h
--- dist/include/linux/fs.h Mon Jan 20 23:27:57 1997
+++ linux/include/linux/fs.h Mon Jan 20 23:27:00 1997
@@ -618,6 +618,8 @@
struct inode ** res_inode, struct inode * base);
extern int do_mknod(const char * filename, int mode, dev_t dev);
extern int do_pipe(int *);
+extern void lock_inode(struct inode * inode);
+extern void unlock_inode(struct inode * inode);
extern void iput(struct inode * inode);
extern struct inode * __iget(struct super_block * sb,int nr,int crsmnt);
extern struct inode * get_empty_inode(void);

\
 
 \ /
  Last update: 2005-03-22 13:38    [from the cache]
©2003-2011 Jasper Spaans