I've attached a patch to correct an ext2 race problem with disk quota
for a deleted inode, following a report sent by David Miller.
Dropping disk quota for a deleted inode requires writing to the quota
file, and this may lead to attempting to lock the super block for one of
the ext2 block allocations. But since ext2_free_inode already holds the
superblock lock, a deadlock can result.
The attached patch avoids the problem by reordering operations to drop
the disk quota before locking the superblock. Since there are no other
references to the inode at this time, the reordering shouldn't cause any
problems. (The subsequent DQUOT_DROP() in clear_inode will be a nop.)
I've tested the changes with intensive cp and rm commands to a directory
with disk quotas, and all seems to be working OK. But I'm not sure this
setup would always triggger the race, and so would appreciate further
testing by anyone who has seen processes stuck in wait_on_super when
using disk quotas.
Regards,
Bill
--------------F9D65CD3B5D0032C3177AF8A
Content-Type: text/plain; charset=us-ascii; name="ext2_123-patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="ext2_123-patch"
--- linux-2.1.123/fs/ext2/ialloc.c.old Wed Aug 26 13:20:32 1998
+++ linux-2.1.123/fs/ext2/ialloc.c Fri Oct 2 20:09:59 1998
@@ -179,9 +179,9 @@
*/
void ext2_free_inode (struct inode * inode)
{
+ struct super_block * sb = inode->i_sb;
int is_directory;
unsigned long ino;
- struct super_block * sb;
struct buffer_head * bh;
struct buffer_head * bh2;
unsigned long block_group;
@@ -190,8 +190,6 @@
struct ext2_group_desc * gdp;
struct ext2_super_block * es;
- if (!inode)
- return;
if (!inode->i_dev) {
printk ("ext2_free_inode: inode has no device\n");
return;
@@ -205,7 +203,7 @@
inode->i_nlink);
return;
}
- if (!inode->i_sb) {
+ if (!sb) {
printk("ext2_free_inode: inode on nonexistent device\n");
return;
}
@@ -213,15 +211,21 @@
ino = inode->i_ino;
ext2_debug ("freeing inode %lu\n", ino);
- sb = inode->i_sb;
+ /*
+ * Note: we must free any quota before locking the superblock,
+ * as writing the quota to disk may need the lock as well.
+ */
+ DQUOT_FREE_INODE(sb, inode);
+ DQUOT_DROP(inode);
+
lock_super (sb);
- if (ino < EXT2_FIRST_INO(sb) ||
- ino > le32_to_cpu(sb->u.ext2_sb.s_es->s_inodes_count)) {
+ es = sb->u.ext2_sb.s_es;
+ if (ino < EXT2_FIRST_INO(sb) ||
+ ino > le32_to_cpu(es->s_inodes_count)) {
ext2_error (sb, "free_inode",
"reserved inode or nonexistent inode");
goto error_return;
}
- es = sb->u.ext2_sb.s_es;
block_group = (ino - 1) / EXT2_INODES_PER_GROUP(sb);
bit = (ino - 1) % EXT2_INODES_PER_GROUP(sb);
bitmap_nr = load_inode_bitmap (sb, block_group);
@@ -233,7 +237,6 @@
is_directory = S_ISDIR(inode->i_mode);
/* Do this BEFORE marking the inode not in use */
- DQUOT_FREE_INODE(sb, inode);
clear_inode (inode);
/* Ok, now we can actually update the inode bitmaps.. */
--------------F9D65CD3B5D0032C3177AF8A--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/