From: Christoph Hellwig Yes, me too. generic_shutdown_super() takes lock_super(). And udf uses lock_super for protecting its block allocation data strutures. Trivial deadlock on unmount. Below is a fix to switch udf to it's own private locking. It's safe because it doesn't intefere with VFS lock_super usage anywhere. udf_free_inode has some more updates than simply switching the used lock: - clear_inode() call moved outside locked section to avoid another deadlock - unused variable ino killed - is_directory moved into the conditional it's actually used in Signed-off-by: Christoph Hellwig (note that I see memory corruption in UDF_I_DATA(inode), but I've reproduced that with a kernel without all recent udf changes. I'll debug that one further) Signed-off-by: Andrew Morton --- 25-akpm/fs/udf/balloc.c | 45 ++++++++++++++++++++------------------ 25-akpm/fs/udf/ialloc.c | 29 +++++++++--------------- 25-akpm/fs/udf/super.c | 2 + 25-akpm/include/linux/udf_fs_sb.h | 4 +++ 4 files changed, 41 insertions(+), 39 deletions(-) diff -puN fs/udf/balloc.c~udf-deadlock-fix fs/udf/balloc.c --- 25/fs/udf/balloc.c~udf-deadlock-fix 2005-01-29 11:29:16.166926016 -0800 +++ 25-akpm/fs/udf/balloc.c 2005-01-29 11:29:16.176924496 -0800 @@ -148,6 +148,7 @@ static void udf_bitmap_free_blocks(struc struct udf_bitmap *bitmap, kernel_lb_addr bloc, uint32_t offset, uint32_t count) { + struct udf_sb_info *sbi = UDF_SB(sb); struct buffer_head * bh = NULL; unsigned long block; unsigned long block_group; @@ -156,7 +157,7 @@ static void udf_bitmap_free_blocks(struc int bitmap_nr; unsigned long overflow; - lock_super(sb); + down(&sbi->s_alloc_sem); if (bloc.logicalBlockNum < 0 || (bloc.logicalBlockNum + count) > UDF_SB_PARTLEN(sb, bloc.partitionReferenceNum)) { @@ -215,7 +216,7 @@ error_return: sb->s_dirt = 1; if (UDF_SB_LVIDBH(sb)) mark_buffer_dirty(UDF_SB_LVIDBH(sb)); - unlock_super(sb); + up(&sbi->s_alloc_sem); return; } @@ -224,13 +225,13 @@ static int udf_bitmap_prealloc_blocks(st struct udf_bitmap *bitmap, uint16_t partition, uint32_t first_block, uint32_t block_count) { + struct udf_sb_info *sbi = UDF_SB(sb); int alloc_count = 0; int bit, block, block_group, group_start; int nr_groups, bitmap_nr; struct buffer_head *bh; - lock_super(sb); - + down(&sbi->s_alloc_sem); if (first_block < 0 || first_block >= UDF_SB_PARTLEN(sb, partition)) goto out; @@ -279,7 +280,7 @@ out: mark_buffer_dirty(UDF_SB_LVIDBH(sb)); } sb->s_dirt = 1; - unlock_super(sb); + up(&sbi->s_alloc_sem); return alloc_count; } @@ -287,6 +288,7 @@ static int udf_bitmap_new_block(struct s struct inode * inode, struct udf_bitmap *bitmap, uint16_t partition, uint32_t goal, int *err) { + struct udf_sb_info *sbi = UDF_SB(sb); int newbit, bit=0, block, block_group, group_start; int end_goal, nr_groups, bitmap_nr, i; struct buffer_head *bh = NULL; @@ -294,7 +296,7 @@ static int udf_bitmap_new_block(struct s int newblock = 0; *err = -ENOSPC; - lock_super(sb); + down(&sbi->s_alloc_sem); repeat: if (goal < 0 || goal >= UDF_SB_PARTLEN(sb, partition)) @@ -367,7 +369,7 @@ repeat: } if (i >= (nr_groups*2)) { - unlock_super(sb); + up(&sbi->s_alloc_sem); return newblock; } if (bit < sb->s_blocksize << 3) @@ -376,7 +378,7 @@ repeat: bit = udf_find_next_one_bit(bh->b_data, sb->s_blocksize << 3, group_start << 3); if (bit >= sb->s_blocksize << 3) { - unlock_super(sb); + up(&sbi->s_alloc_sem); return 0; } @@ -390,7 +392,7 @@ got_block: */ if (inode && DQUOT_ALLOC_BLOCK(inode, 1)) { - unlock_super(sb); + up(&sbi->s_alloc_sem); *err = -EDQUOT; return 0; } @@ -413,13 +415,13 @@ got_block: mark_buffer_dirty(UDF_SB_LVIDBH(sb)); } sb->s_dirt = 1; - unlock_super(sb); + up(&sbi->s_alloc_sem); *err = 0; return newblock; error_return: *err = -EIO; - unlock_super(sb); + up(&sbi->s_alloc_sem); return 0; } @@ -428,6 +430,7 @@ static void udf_table_free_blocks(struct struct inode * table, kernel_lb_addr bloc, uint32_t offset, uint32_t count) { + struct udf_sb_info *sbi = UDF_SB(sb); uint32_t start, end; uint32_t nextoffset, oextoffset, elen; kernel_lb_addr nbloc, obloc, eloc; @@ -435,7 +438,7 @@ static void udf_table_free_blocks(struct int8_t etype; int i; - lock_super(sb); + down(&sbi->s_alloc_sem); if (bloc.logicalBlockNum < 0 || (bloc.logicalBlockNum + count) > UDF_SB_PARTLEN(sb, bloc.partitionReferenceNum)) { @@ -669,7 +672,7 @@ static void udf_table_free_blocks(struct error_return: sb->s_dirt = 1; - unlock_super(sb); + up(&sbi->s_alloc_sem); return; } @@ -678,6 +681,7 @@ static int udf_table_prealloc_blocks(str struct inode *table, uint16_t partition, uint32_t first_block, uint32_t block_count) { + struct udf_sb_info *sbi = UDF_SB(sb); int alloc_count = 0; uint32_t extoffset, elen, adsize; kernel_lb_addr bloc, eloc; @@ -694,8 +698,7 @@ static int udf_table_prealloc_blocks(str else return 0; - lock_super(sb); - + down(&sbi->s_alloc_sem); extoffset = sizeof(struct unallocSpaceEntry); bloc = UDF_I_LOCATION(table); @@ -739,7 +742,7 @@ static int udf_table_prealloc_blocks(str mark_buffer_dirty(UDF_SB_LVIDBH(sb)); sb->s_dirt = 1; } - unlock_super(sb); + up(&sbi->s_alloc_sem); return alloc_count; } @@ -747,6 +750,7 @@ static int udf_table_new_block(struct su struct inode * inode, struct inode *table, uint16_t partition, uint32_t goal, int *err) { + struct udf_sb_info *sbi = UDF_SB(sb); uint32_t spread = 0xFFFFFFFF, nspread = 0xFFFFFFFF; uint32_t newblock = 0, adsize; uint32_t extoffset, goal_extoffset, elen, goal_elen = 0; @@ -763,8 +767,7 @@ static int udf_table_new_block(struct su else return newblock; - lock_super(sb); - + down(&sbi->s_alloc_sem); if (goal < 0 || goal >= UDF_SB_PARTLEN(sb, partition)) goal = 0; @@ -814,7 +817,7 @@ static int udf_table_new_block(struct su if (spread == 0xFFFFFFFF) { udf_release_data(goal_bh); - unlock_super(sb); + up(&sbi->s_alloc_sem); return 0; } @@ -830,7 +833,7 @@ static int udf_table_new_block(struct su if (inode && DQUOT_ALLOC_BLOCK(inode, 1)) { udf_release_data(goal_bh); - unlock_super(sb); + up(&sbi->s_alloc_sem); *err = -EDQUOT; return 0; } @@ -849,7 +852,7 @@ static int udf_table_new_block(struct su } sb->s_dirt = 1; - unlock_super(sb); + up(&sbi->s_alloc_sem); *err = 0; return newblock; } diff -puN fs/udf/ialloc.c~udf-deadlock-fix fs/udf/ialloc.c --- 25/fs/udf/ialloc.c~udf-deadlock-fix 2005-01-29 11:29:16.168925712 -0800 +++ 25-akpm/fs/udf/ialloc.c 2005-01-29 11:29:16.177924344 -0800 @@ -35,11 +35,8 @@ void udf_free_inode(struct inode * inode) { - struct super_block * sb = inode->i_sb; - int is_directory; - unsigned long ino; - - ino = inode->i_ino; + struct super_block *sb = inode->i_sb; + struct udf_sb_info *sbi = UDF_SB(sb); /* * Note: we must free any quota before locking the superblock, @@ -48,36 +45,32 @@ void udf_free_inode(struct inode * inode DQUOT_FREE_INODE(inode); DQUOT_DROP(inode); - lock_super(sb); - - is_directory = S_ISDIR(inode->i_mode); - clear_inode(inode); - if (UDF_SB_LVIDBH(sb)) - { - if (is_directory) + down(&sbi->s_alloc_sem); + if (sbi->s_lvidbh) { + if (S_ISDIR(inode->i_mode)) UDF_SB_LVIDIU(sb)->numDirs = cpu_to_le32(le32_to_cpu(UDF_SB_LVIDIU(sb)->numDirs) - 1); else UDF_SB_LVIDIU(sb)->numFiles = cpu_to_le32(le32_to_cpu(UDF_SB_LVIDIU(sb)->numFiles) - 1); - mark_buffer_dirty(UDF_SB_LVIDBH(sb)); + mark_buffer_dirty(sbi->s_lvidbh); } - unlock_super(sb); + up(&sbi->s_alloc_sem); udf_free_blocks(sb, NULL, UDF_I_LOCATION(inode), 0, 1); } struct inode * udf_new_inode (struct inode *dir, int mode, int * err) { - struct super_block *sb; + struct super_block *sb = dir->i_sb; + struct udf_sb_info *sbi = UDF_SB(sb); struct inode * inode; int block; uint32_t start = UDF_I_LOCATION(dir).logicalBlockNum; - sb = dir->i_sb; inode = new_inode(sb); if (!inode) @@ -94,8 +87,8 @@ struct inode * udf_new_inode (struct ino iput(inode); return NULL; } - lock_super(sb); + down(&sbi->s_alloc_sem); UDF_I_UNIQUE(inode) = 0; UDF_I_LENEXTENTS(inode) = 0; UDF_I_NEXT_ALLOC_BLOCK(inode) = 0; @@ -160,8 +153,8 @@ struct inode * udf_new_inode (struct ino UDF_I_CRTIME(inode) = current_fs_time(inode->i_sb); insert_inode_hash(inode); mark_inode_dirty(inode); + up(&sbi->s_alloc_sem); - unlock_super(sb); if (DQUOT_ALLOC_INODE(inode)) { DQUOT_DROP(inode); diff -puN fs/udf/super.c~udf-deadlock-fix fs/udf/super.c --- 25/fs/udf/super.c~udf-deadlock-fix 2005-01-29 11:29:16.169925560 -0800 +++ 25-akpm/fs/udf/super.c 2005-01-29 11:29:16.178924192 -0800 @@ -1504,6 +1504,8 @@ static int udf_fill_super(struct super_b sb->s_fs_info = sbi; memset(UDF_SB(sb), 0x00, sizeof(struct udf_sb_info)); + init_MUTEX(&sbi->s_alloc_sem); + if (!udf_parse_options((char *)options, &uopt)) goto error_out; diff -puN include/linux/udf_fs_sb.h~udf-deadlock-fix include/linux/udf_fs_sb.h --- 25/include/linux/udf_fs_sb.h~udf-deadlock-fix 2005-01-29 11:29:16.171925256 -0800 +++ 25-akpm/include/linux/udf_fs_sb.h 2005-01-29 11:29:16.179924040 -0800 @@ -18,6 +18,8 @@ #ifndef _UDF_FS_SB_H #define _UDF_FS_SB_H 1 +#include + #pragma pack(1) #define UDF_MAX_BLOCK_LOADED 8 @@ -113,6 +115,8 @@ struct udf_sb_info /* VAT inode */ struct inode *s_vat; + + struct semaphore s_alloc_sem; }; #endif /* _UDF_FS_SB_H */ _