Re: 2.5.64-mm8

From: Roman Zippel (zippel@linux-m68k.org)
Date: Sun Mar 16 2003 - 07:21:34 EST


Hi,

On Sun, 16 Mar 2003, Andrew Morton wrote:

> +affs-lock_kernel-fix.patch
>
> Missing an unlock_kernel(). (Why didn't any of the checkers notice this?)

Could you replace this the patch below?
It removes the kernel lock completely and also fixes a bitmap corruption.

bye, Roman

--- linux/fs/affs/Changes 18 May 2002 12:58:27 -0000 1.1.1.2
+++ linux/fs/affs/Changes 16 Mar 2003 00:35:30 -0000
@@ -28,6 +28,11 @@ Known bugs:
 
 Please direct bug reports to: zippel@linux-m68k.org
 
+Version 3.20
+------------
+- kill kernel lock
+- fix for a possible bitmap corruption
+
 Version 3.19
 ------------
 
--- linux/fs/affs/bitmap.c 25 May 2002 16:20:39 -0000 1.1.1.8
+++ linux/fs/affs/bitmap.c 16 Mar 2003 00:35:31 -0000
@@ -185,6 +185,8 @@ find_bmap:
         /* search for the next bmap buffer with free bits */
         i = sbi->s_bmap_count;
         do {
+ if (--i < 0)
+ goto err_full;
                 bmap++;
                 bm++;
                 if (bmap < sbi->s_bmap_count)
@@ -192,8 +194,6 @@ find_bmap:
                 /* restart search at zero */
                 bmap = 0;
                 bm = sbi->s_bitmap;
- if (--i <= 0)
- goto err_full;
         } while (!bm->bm_free);
         blk = bmap * sbi->s_bmap_bits;
 
@@ -216,8 +216,8 @@ find_bmap_bit:
         mask = ~0UL << (bit & 31);
         blk &= ~31UL;
 
- tmp = be32_to_cpu(*data) & mask;
- if (tmp)
+ tmp = be32_to_cpu(*data);
+ if (tmp & mask)
                 goto find_bit;
 
         /* scan the rest of the buffer */
@@ -230,10 +230,11 @@ find_bmap_bit:
                         goto find_bmap;
         } while (!(tmp = *data));
         tmp = be32_to_cpu(tmp);
+ mask = ~0;
 
 find_bit:
         /* finally look for a free bit in the word */
- bit = ffs(tmp) - 1;
+ bit = ffs(tmp & mask) - 1;
         blk += bit + sbi->s_reserved;
         mask2 = mask = 1 << (bit & 31);
         AFFS_I(inode)->i_lastalloc = blk;
@@ -266,8 +267,8 @@ err_bh_read:
         sbi->s_bmap_bh = NULL;
         sbi->s_last_bmap = ~0;
 err_full:
- pr_debug("failed\n");
         up(&sbi->s_bmlock);
+ pr_debug("failed\n");
         return 0;
 }
 
--- linux/fs/affs/dir.c 11 Nov 2002 18:56:16 -0000 1.1.1.6
+++ linux/fs/affs/dir.c 16 Mar 2003 00:35:31 -0000
@@ -65,8 +65,6 @@ affs_readdir(struct file *filp, void *di
         int stored;
         int res;
 
- lock_kernel();
-
         pr_debug("AFFS: readdir(ino=%lu,f_pos=%lx)\n",inode->i_ino,(unsigned long)filp->f_pos);
 
         stored = 0;
@@ -162,7 +160,6 @@ readdir_out:
         affs_brelse(dir_bh);
         affs_brelse(fh_bh);
         affs_unlock_dir(inode);
- unlock_kernel();
         pr_debug("AFFS: readdir()=%d\n", stored);
         return res;
 }
--- linux/fs/affs/inode.c 18 Nov 2002 18:46:35 -0000 1.1.1.10
+++ linux/fs/affs/inode.c 16 Mar 2003 00:35:31 -0000
@@ -195,11 +195,9 @@ affs_write_inode(struct inode *inode, in
         if (!inode->i_nlink)
                 // possibly free block
                 return;
- lock_kernel();
         bh = affs_bread(sb, inode->i_ino);
         if (!bh) {
                 affs_error(sb,"write_inode","Cannot read block %lu",inode->i_ino);
- unlock_kernel();
                 return;
         }
         tail = AFFS_TAIL(sb, bh);
@@ -227,7 +225,7 @@ affs_write_inode(struct inode *inode, in
         affs_fix_checksum(sb, bh);
         mark_buffer_dirty_inode(bh, inode);
         affs_brelse(bh);
- unlock_kernel();
+ affs_free_prealloc(inode);
 }
 
 int
@@ -236,8 +234,6 @@ affs_notify_change(struct dentry *dentry
         struct inode *inode = dentry->d_inode;
         int error;
 
- lock_kernel();
-
         pr_debug("AFFS: notify_change(%lu,0x%x)\n",inode->i_ino,attr->ia_valid);
 
         error = inode_change_ok(inode,attr);
@@ -257,7 +253,6 @@ affs_notify_change(struct dentry *dentry
         if (!error && (attr->ia_valid & ATTR_MODE))
                 mode_to_prot(inode);
 out:
- unlock_kernel();
         return error;
 }
 
@@ -265,15 +260,13 @@ void
 affs_put_inode(struct inode *inode)
 {
         pr_debug("AFFS: put_inode(ino=%lu, nlink=%u)\n", inode->i_ino, inode->i_nlink);
- lock_kernel();
         affs_free_prealloc(inode);
         if (atomic_read(&inode->i_count) == 1) {
+ down(&inode->i_sem);
                 if (inode->i_size != AFFS_I(inode)->mmu_private)
                         affs_truncate(inode);
- //if (inode->i_nlink)
- // affs_clear_inode(inode);
+ up(&inode->i_sem);
         }
- unlock_kernel();
 }
 
 void
@@ -284,9 +277,7 @@ affs_delete_inode(struct inode *inode)
         if (S_ISREG(inode->i_mode))
                 affs_truncate(inode);
         clear_inode(inode);
- lock_kernel();
         affs_free_block(inode->i_sb, inode->i_ino);
- unlock_kernel();
 }
 
 void
--- linux/fs/affs/namei.c 11 Nov 2002 18:56:17 -0000 1.1.1.8
+++ linux/fs/affs/namei.c 16 Mar 2003 00:35:31 -0000
@@ -218,12 +218,10 @@ affs_lookup(struct inode *dir, struct de
 
         pr_debug("AFFS: lookup(\"%.*s\")\n",(int)dentry->d_name.len,dentry->d_name.name);
 
- lock_kernel();
         affs_lock_dir(dir);
         bh = affs_find_entry(dir, dentry);
         affs_unlock_dir(dir);
         if (IS_ERR(bh)) {
- unlock_kernel();
                 return ERR_PTR(PTR_ERR(bh));
         }
         if (bh) {
@@ -240,12 +238,10 @@ affs_lookup(struct inode *dir, struct de
                 affs_brelse(bh);
                 inode = iget(sb, ino);
                 if (!inode) {
- unlock_kernel();
                         return ERR_PTR(-EACCES);
                 }
         }
         dentry->d_op = AFFS_SB(sb)->s_flags & SF_INTL ? &affs_intl_dentry_operations : &affs_dentry_operations;
- unlock_kernel();
         d_add(dentry, inode);
         return NULL;
 }
@@ -253,17 +249,10 @@ affs_lookup(struct inode *dir, struct de
 int
 affs_unlink(struct inode *dir, struct dentry *dentry)
 {
- int res;
         pr_debug("AFFS: unlink(dir=%d, \"%.*s\")\n", (u32)dir->i_ino,
                  (int)dentry->d_name.len, dentry->d_name.name);
 
- if (!dentry->d_inode)
- return -ENOENT;
-
- lock_kernel();
- res = affs_remove_header(dentry);
- unlock_kernel();
- return res;
+ return affs_remove_header(dentry);
 }
 
 int
@@ -276,12 +265,9 @@ affs_create(struct inode *dir, struct de
         pr_debug("AFFS: create(%lu,\"%.*s\",0%o)\n",dir->i_ino,(int)dentry->d_name.len,
                  dentry->d_name.name,mode);
 
- lock_kernel();
         inode = affs_new_inode(dir);
- if (!inode) {
- unlock_kernel();
+ if (!inode)
                 return -ENOSPC;
- }
 
         inode->i_mode = mode;
         mode_to_prot(inode);
@@ -294,10 +280,8 @@ affs_create(struct inode *dir, struct de
         if (error) {
                 inode->i_nlink = 0;
                 iput(inode);
- unlock_kernel();
                 return error;
         }
- unlock_kernel();
         return 0;
 }
 
@@ -310,12 +294,9 @@ affs_mkdir(struct inode *dir, struct den
         pr_debug("AFFS: mkdir(%lu,\"%.*s\",0%o)\n",dir->i_ino,
                  (int)dentry->d_name.len,dentry->d_name.name,mode);
 
- lock_kernel();
         inode = affs_new_inode(dir);
- if (!inode) {
- unlock_kernel();
+ if (!inode)
                 return -ENOSPC;
- }
 
         inode->i_mode = S_IFDIR | mode;
         mode_to_prot(inode);
@@ -328,10 +309,8 @@ affs_mkdir(struct inode *dir, struct den
                 inode->i_nlink = 0;
                 mark_inode_dirty(inode);
                 iput(inode);
- unlock_kernel();
                 return error;
         }
- unlock_kernel();
         return 0;
 }
 
@@ -357,14 +336,10 @@ affs_symlink(struct inode *dir, struct d
         pr_debug("AFFS: symlink(%lu,\"%.*s\" -> \"%s\")\n",dir->i_ino,
                  (int)dentry->d_name.len,dentry->d_name.name,symname);
 
- lock_kernel();
         maxlen = AFFS_SB(sb)->s_hashsize * sizeof(u32) - 1;
- error = -ENOSPC;
         inode = affs_new_inode(dir);
- if (!inode) {
- unlock_kernel();
+ if (!inode)
                 return -ENOSPC;
- }
 
         inode->i_op = &affs_symlink_inode_operations;
         inode->i_data.a_ops = &affs_symlink_aops;
@@ -410,7 +385,6 @@ affs_symlink(struct inode *dir, struct d
         error = affs_add_entry(dir, inode, dentry, ST_SOFTLINK);
         if (error)
                 goto err;
- unlock_kernel();
 
         return 0;
 
@@ -418,7 +392,6 @@ err:
         inode->i_nlink = 0;
         mark_inode_dirty(inode);
         iput(inode);
- unlock_kernel();
         return error;
 }
 
@@ -426,23 +399,11 @@ int
 affs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry)
 {
         struct inode *inode = old_dentry->d_inode;
- int error;
 
         pr_debug("AFFS: link(%u, %u, \"%.*s\")\n", (u32)inode->i_ino, (u32)dir->i_ino,
                  (int)dentry->d_name.len,dentry->d_name.name);
 
- lock_kernel();
- error = affs_add_entry(dir, inode, dentry, ST_LINKFILE);
- if (error) {
- /* WTF??? */
- inode->i_nlink = 0;
- mark_inode_dirty(inode);
- iput(inode);
- unlock_kernel();
- return error;
- }
- unlock_kernel();
- return 0;
+ return affs_add_entry(dir, inode, dentry, ST_LINKFILE);
 }
 
 int
@@ -453,21 +414,19 @@ affs_rename(struct inode *old_dir, struc
         struct buffer_head *bh = NULL;
         int retval;
 
- lock_kernel();
         pr_debug("AFFS: rename(old=%u,\"%*s\" to new=%u,\"%*s\")\n",
                  (u32)old_dir->i_ino, (int)old_dentry->d_name.len, old_dentry->d_name.name,
                  (u32)new_dir->i_ino, (int)new_dentry->d_name.len, new_dentry->d_name.name);
 
- if ((retval = affs_check_name(new_dentry->d_name.name,new_dentry->d_name.len)))
- goto done;
+ retval = affs_check_name(new_dentry->d_name.name,new_dentry->d_name.len);
+ if (retval)
+ return retval;
 
         /* Unlink destination if it already exists */
         if (new_dentry->d_inode) {
                 retval = affs_remove_header(new_dentry);
- if (retval) {
- unlock_kernel();
+ if (retval)
                         return retval;
- }
         }
 
         retval = -EIO;
@@ -493,6 +452,5 @@ affs_rename(struct inode *old_dir, struc
 done:
         mark_buffer_dirty_inode(bh, retval ? old_dir : new_dir);
         affs_brelse(bh);
- unlock_kernel();
         return retval;
 }
--- linux/fs/affs/super.c 27 Jan 2003 21:03:20 -0000 1.1.1.15
+++ linux/fs/affs/super.c 16 Mar 2003 00:35:31 -0000
@@ -40,7 +40,6 @@ static void
 affs_put_super(struct super_block *sb)
 {
         struct affs_sb_info *sbi = AFFS_SB(sb);
- lock_kernel();
         pr_debug("AFFS: put_super()\n");
 
         if (!(sb->s_flags & MS_RDONLY)) {
@@ -58,7 +57,6 @@ affs_put_super(struct super_block *sb)
         affs_brelse(sbi->s_root_bh);
         kfree(sbi);
         sb->s_fs_info = NULL;
- unlock_kernel();
         return;
 }
 
@@ -67,7 +65,7 @@ affs_write_super(struct super_block *sb)
 {
         int clean = 2;
         struct affs_sb_info *sbi = AFFS_SB(sb);
- lock_kernel();
+
         if (!(sb->s_flags & MS_RDONLY)) {
                 // if (sbi->s_bitmap[i].bm_bh) {
                 // if (buffer_dirty(sbi->s_bitmap[i].bm_bh)) {
@@ -81,7 +79,7 @@ affs_write_super(struct super_block *sb)
         } else
                 sb->s_dirt = 0;
 
- unlock_kernel();
+ pr_debug("AFFS: write_super() at %lu, clean=%d\n", get_seconds(), clean);
 }
 
 static kmem_cache_t * affs_inode_cachep;
--- linux/fs/affs/symlink.c 11 Nov 2002 18:56:17 -0000 1.1.1.5
+++ linux/fs/affs/symlink.c 16 Mar 2003 00:35:31 -0000
@@ -32,9 +32,7 @@ static int affs_symlink_readpage(struct
         pr_debug("AFFS: follow_link(ino=%lu)\n",inode->i_ino);
 
         err = -EIO;
- lock_kernel();
         bh = affs_bread(inode->i_sb, inode->i_ino);
- unlock_kernel();
         if (!bh)
                 goto fail;
         i = 0;
@@ -63,9 +61,7 @@ static int affs_symlink_readpage(struct
                 j++;
         }
         link[i] = '\0';
- lock_kernel();
         affs_brelse(bh);
- unlock_kernel();
         SetPageUptodate(page);
         kunmap(page);
         unlock_page(page);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sun Mar 23 2003 - 22:00:18 EST