[PATCH] Fix of quota deadlock on pagelock: reiserfs

Implement quota journaling and quota reading and writing functions for
reiserfs.  Solves also several other deadlocks possible for reiserfs due to
the lock inversion on journal_begin and quota locks.

From: Vladimir Saveliev <vs@namesys.com>

When CONFIG_QUOTA is defined reiserfs's finish_unfinished sets and clears
MS_ACTIVE bit in s_flags field of super block.  If that bit was set already
it should not be set.
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

fs/reiserfs/file.c

@@ -54,7 +54,7 @@ static int reiserfs_file_release (struct inode * inode, struct file * filp)
     /* freeing preallocation only involves relogging blocks that
      * are already in the current transaction.  preallocation gets
      * freed at the end of each transaction, so it is impossible for
-     * us to log any additional blocks
+     * us to log any additional blocks (including quota blocks)
      */
     err = journal_begin(&th, inode->i_sb, 1);
     if (err) {
@@ -201,7 +201,7 @@ int reiserfs_allocate_blocks_for_region(
     /* If we came here, it means we absolutely need to open a transaction,
        since we need to allocate some blocks */
     reiserfs_write_lock(inode->i_sb); // Journaling stuff and we need that.
-    res = journal_begin(th, inode->i_sb, JOURNAL_PER_BALANCE_CNT * 3 + 1); // Wish I know if this number enough
+    res = journal_begin(th, inode->i_sb, JOURNAL_PER_BALANCE_CNT * 3 + 1 + 2 * REISERFS_QUOTA_TRANS_BLOCKS); // Wish I know if this number enough
     if (res)
         goto error_exit;
     reiserfs_update_inode_transaction(inode) ;
@@ -576,7 +576,7 @@ int reiserfs_allocate_blocks_for_region(
         int err;
         // update any changes we made to blk count
         reiserfs_update_sd(th, inode);
-        err = journal_end(th, inode->i_sb, JOURNAL_PER_BALANCE_CNT * 3 + 1);
+        err = journal_end(th, inode->i_sb, JOURNAL_PER_BALANCE_CNT * 3 + 1 + 2 * REISERFS_QUOTA_TRANS_BLOCKS);
         if (err)
             res = err;
     }

fs/reiserfs/inode.c

@@ -20,27 +20,17 @@
 
 extern int reiserfs_default_io_size; /* default io size devuned in super.c */
 
-/* args for the create parameter of reiserfs_get_block */
-#define GET_BLOCK_NO_CREATE 0 /* don't create new blocks or convert tails */
-#define GET_BLOCK_CREATE 1    /* add anything you need to find block */
-#define GET_BLOCK_NO_HOLE 2   /* return -ENOENT for file holes */
-#define GET_BLOCK_READ_DIRECT 4  /* read the tail if indirect item not found */
-#define GET_BLOCK_NO_ISEM     8 /* i_sem is not held, don't preallocate */
-#define GET_BLOCK_NO_DANGLE   16 /* don't leave any transactions running */
-
-static int reiserfs_get_block (struct inode * inode, sector_t block,
-			       struct buffer_head * bh_result, int create);
 static int reiserfs_commit_write(struct file *f, struct page *page,
                                  unsigned from, unsigned to);
 
 void reiserfs_delete_inode (struct inode * inode)
 {
-    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 2; 
+    /* We need blocks for transaction + (user+group) quota update (possibly delete) */
+    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 2 + 2 * REISERFS_QUOTA_INIT_BLOCKS;
     struct reiserfs_transaction_handle th ;
   
     reiserfs_write_lock(inode->i_sb);
 
-    DQUOT_FREE_INODE(inode);
     /* The = 0 happens when we abort creating a new inode for some reason like lack of space.. */
     if (!(inode->i_state & I_NEW) && INODE_PKEY(inode)->k_objectid != 0) { /* also handles bad_inode case */
 	down (&inode->i_sem); 
@@ -58,6 +48,11 @@ void reiserfs_delete_inode (struct inode * inode)
 	    goto out;
 	}
 
+	/* Do quota update inside a transaction for journaled quotas. We must do that
+	 * after delete_object so that quota updates go into the same transaction as
+	 * stat data deletion */
+	DQUOT_FREE_INODE(inode);
+
 	if (journal_end(&th, inode->i_sb, jbegin_count)) {
 	    up (&inode->i_sem);
 	    goto out;
@@ -592,8 +587,9 @@ int reiserfs_get_block (struct inode * inode, sector_t block,
         . 3 balancings in direct->indirect conversion
         . 1 block involved into reiserfs_update_sd()
        XXX in practically impossible worst case direct2indirect()
-       can incur (much) more that 3 balancings. */
-    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 3 + 1;
+       can incur (much) more than 3 balancings.
+       quota update for user, group */
+    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 3 + 1 + 2 * REISERFS_QUOTA_TRANS_BLOCKS;
     int version;
     int dangle = 1;
     loff_t new_offset = (((loff_t)block) << inode->i_sb->s_blocksize_bits) + 1 ;
@@ -1699,6 +1695,10 @@ int reiserfs_new_inode (struct reiserfs_transaction_handle *th,
 
     BUG_ON (!th->t_trans_id);
   
+    if (DQUOT_ALLOC_INODE(inode)) {
+	err = -EDQUOT;
+	goto out_end_trans;
+    }
     if (!dir || !dir->i_nlink) {
 	err = -EPERM;
 	goto out_bad_inode;
@@ -1866,9 +1866,12 @@ int reiserfs_new_inode (struct reiserfs_transaction_handle *th,
     /* Invalidate the object, nothing was inserted yet */
     INODE_PKEY(inode)->k_objectid = 0;
 
-    /* dquot_drop must be done outside a transaction */
-    journal_end(th, th->t_super, th->t_blocks_allocated) ;
+    /* Quota change must be inside a transaction for journaling */
     DQUOT_FREE_INODE(inode);
+
+out_end_trans:
+    journal_end(th, th->t_super, th->t_blocks_allocated) ;
+    /* Drop can be outside and it needs more credits so it's better to have it outside */
     DQUOT_DROP(inode);
     inode->i_flags |= S_NOQUOTA;
     make_bad_inode(inode);
@@ -2796,8 +2799,25 @@ int reiserfs_setattr(struct dentry *dentry, struct iattr *attr) {
 	    (ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid)) {
                 error = reiserfs_chown_xattrs (inode, attr);
 
-                if (!error)
+                if (!error) {
+		    struct reiserfs_transaction_handle th;
+
+		    /* (user+group)*(old+new) structure - we count quota info and , inode write (sb, inode) */
+		    journal_begin(&th, inode->i_sb, 4*REISERFS_QUOTA_INIT_BLOCKS+2);
                     error = DQUOT_TRANSFER(inode, attr) ? -EDQUOT : 0;
+		    if (error) {
+			journal_end(&th, inode->i_sb, 4*REISERFS_QUOTA_INIT_BLOCKS+2);
+			goto out;
+		    }
+		    /* Update corresponding info in inode so that everything is in
+		     * one transaction */
+		    if (attr->ia_valid & ATTR_UID)
+			inode->i_uid = attr->ia_uid;
+		    if (attr->ia_valid & ATTR_GID)
+			inode->i_gid = attr->ia_gid;
+		    mark_inode_dirty(inode);
+		    journal_end(&th, inode->i_sb, 4*REISERFS_QUOTA_INIT_BLOCKS+2);
+		}
         }
         if (!error)
             error = inode_setattr(inode, attr) ;

fs/reiserfs/namei.c

@@ -545,7 +545,7 @@ static int reiserfs_add_entry (struct reiserfs_transaction_handle *th, struct in
 
 /* quota utility function, call if you've had to abort after calling
 ** new_inode_init, and have not called reiserfs_new_inode yet.
-** This should only be called on inodes that do not hav stat data
+** This should only be called on inodes that do not have stat data
 ** inserted into the tree yet.
 */
 static int drop_new_inode(struct inode *inode) {
@@ -557,10 +557,9 @@ static int drop_new_inode(struct inode *inode) {
 }
 
 /* utility function that does setup for reiserfs_new_inode.  
-** DQUOT_ALLOC_INODE cannot be called inside a transaction, so we had
-** to pull some bits of reiserfs_new_inode out into this func.
-** Yes, the actual quota calls are missing, they are part of the quota
-** patch.
+** DQUOT_INIT needs lots of credits so it's better to have it
+** outside of a transaction, so we had to pull some bits of
+** reiserfs_new_inode out into this func.
 */
 static int new_inode_init(struct inode *inode, struct inode *dir, int mode) {
 
@@ -578,10 +577,6 @@ static int new_inode_init(struct inode *inode, struct inode *dir, int mode) {
         inode->i_gid = current->fsgid;
     }
     DQUOT_INIT(inode);
-    if (DQUOT_ALLOC_INODE(inode)) {
-        drop_new_inode(inode);
-	return -EDQUOT;
-    }
     return 0 ;
 }
 
@@ -590,16 +585,15 @@ static int reiserfs_create (struct inode * dir, struct dentry *dentry, int mode,
 {
     int retval;
     struct inode * inode;
-    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 2 ;
+    /* We need blocks for transaction + (user+group)*(quotas for new inode + update of quota for directory owner) */
+    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 2 + 2 * (REISERFS_QUOTA_INIT_BLOCKS+REISERFS_QUOTA_TRANS_BLOCKS);
     struct reiserfs_transaction_handle th ;
     int locked;
 
     if (!(inode = new_inode(dir->i_sb))) {
 	return -ENOMEM ;
     }
-    retval = new_inode_init(inode, dir, mode);
-    if (retval)
-        return retval;
+    new_inode_init(inode, dir, mode);
 
     locked = reiserfs_cache_default_acl (dir);
 
@@ -658,7 +652,8 @@ static int reiserfs_mknod (struct inode * dir, struct dentry *dentry, int mode,
     int retval;
     struct inode * inode;
     struct reiserfs_transaction_handle th ;
-    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 3; 
+    /* We need blocks for transaction + (user+group)*(quotas for new inode + update of quota for directory owner) */
+    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 3 + 2 * (REISERFS_QUOTA_INIT_BLOCKS+REISERFS_QUOTA_TRANS_BLOCKS);
     int locked;
 
     if (!new_valid_dev(rdev))
@@ -667,9 +662,7 @@ static int reiserfs_mknod (struct inode * dir, struct dentry *dentry, int mode,
     if (!(inode = new_inode(dir->i_sb))) {
 	return -ENOMEM ;
     }
-    retval = new_inode_init(inode, dir, mode);
-    if (retval)
-        return retval;
+    new_inode_init(inode, dir, mode);
 
     locked = reiserfs_cache_default_acl (dir);
 
@@ -733,7 +726,8 @@ static int reiserfs_mkdir (struct inode * dir, struct dentry *dentry, int mode)
     int retval;
     struct inode * inode;
     struct reiserfs_transaction_handle th ;
-    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 3; 
+    /* We need blocks for transaction + (user+group)*(quotas for new inode + update of quota for directory owner) */
+    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 3 + 2 * (REISERFS_QUOTA_INIT_BLOCKS+REISERFS_QUOTA_TRANS_BLOCKS);
     int locked;
 
 #ifdef DISPLACE_NEW_PACKING_LOCALITIES
@@ -744,9 +738,7 @@ static int reiserfs_mkdir (struct inode * dir, struct dentry *dentry, int mode)
     if (!(inode = new_inode(dir->i_sb))) {
 	return -ENOMEM ;
     }
-    retval = new_inode_init(inode, dir, mode);
-    if (retval)
-        return retval;
+    new_inode_init(inode, dir, mode);
 
     locked = reiserfs_cache_default_acl (dir);
 
@@ -836,8 +828,9 @@ static int reiserfs_rmdir (struct inode * dir, struct dentry *dentry)
     struct reiserfs_dir_entry de;
 
 
-    /* we will be doing 2 balancings and update 2 stat data */
-    jbegin_count = JOURNAL_PER_BALANCE_CNT * 2 + 2;
+    /* we will be doing 2 balancings and update 2 stat data, we change quotas
+     * of the owner of the directory and of the owner of the parent directory */
+    jbegin_count = JOURNAL_PER_BALANCE_CNT * 2 + 2 + 2 * (REISERFS_QUOTA_INIT_BLOCKS+REISERFS_QUOTA_TRANS_BLOCKS);
 
     reiserfs_write_lock(dir->i_sb);
     retval = journal_begin(&th, dir->i_sb, jbegin_count) ;
@@ -920,8 +913,9 @@ static int reiserfs_unlink (struct inode * dir, struct dentry *dentry)
     inode = dentry->d_inode;
 
     /* in this transaction we can be doing at max two balancings and update
-       two stat datas */
-    jbegin_count = JOURNAL_PER_BALANCE_CNT * 2 + 2;
+       two stat datas, we change quotas of the owner of the directory and of
+       the owner of the parent directory */
+    jbegin_count = JOURNAL_PER_BALANCE_CNT * 2 + 2 + 2 * (REISERFS_QUOTA_INIT_BLOCKS+REISERFS_QUOTA_TRANS_BLOCKS);
 
     reiserfs_write_lock(dir->i_sb);
     retval = journal_begin(&th, dir->i_sb, jbegin_count) ;
@@ -1005,15 +999,13 @@ static int reiserfs_symlink (struct inode * parent_dir,
     int item_len;
     struct reiserfs_transaction_handle th ;
     int mode = S_IFLNK | S_IRWXUGO;
-    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 3; 
+    /* We need blocks for transaction + (user+group)*(quotas for new inode + update of quota for directory owner) */
+    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 3 + 2 * (REISERFS_QUOTA_INIT_BLOCKS+REISERFS_QUOTA_TRANS_BLOCKS);
 
     if (!(inode = new_inode(parent_dir->i_sb))) {
 	return -ENOMEM ;
     }
-    retval = new_inode_init(inode, parent_dir, mode);
-    if (retval) {
-        return retval;
-    }
+    new_inode_init(inode, parent_dir, mode);
 
     reiserfs_write_lock(parent_dir->i_sb);
     item_len = ROUND_UP (strlen (symname));
@@ -1083,7 +1075,8 @@ static int reiserfs_link (struct dentry * old_dentry, struct inode * dir, struct
     int retval;
     struct inode *inode = old_dentry->d_inode;
     struct reiserfs_transaction_handle th ;
-    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 3; 
+    /* We need blocks for transaction + update of quotas for the owners of the directory */
+    int jbegin_count = JOURNAL_PER_BALANCE_CNT * 3 + 2 * REISERFS_QUOTA_TRANS_BLOCKS;
 
     reiserfs_write_lock(dir->i_sb);
     if (inode->i_nlink >= REISERFS_LINK_MAX) {
@@ -1201,8 +1194,9 @@ static int reiserfs_rename (struct inode * old_dir, struct dentry *old_dentry,
        (2) new directory and (3) maybe old object stat data (when it is
        directory) and (4) maybe stat data of object to which new entry
        pointed initially and (5) maybe block containing ".." of
-       renamed directory */
-    jbegin_count = JOURNAL_PER_BALANCE_CNT * 3 + 5;
+       renamed directory
+       quota updates: two parent directories */
+    jbegin_count = JOURNAL_PER_BALANCE_CNT * 3 + 5 + 4 * REISERFS_QUOTA_TRANS_BLOCKS;
 
     old_inode = old_dentry->d_inode;
     new_dentry_inode = new_dentry->d_inode;

include/linux/reiserfs_fs.h

@@ -1688,6 +1688,13 @@ struct reiserfs_journal_header {
 #define JOURNAL_MAX_COMMIT_AGE 30 
 #define JOURNAL_MAX_TRANS_AGE 30
 #define JOURNAL_PER_BALANCE_CNT (3 * (MAX_HEIGHT-2) + 9)
+#ifdef CONFIG_QUOTA
+#define REISERFS_QUOTA_TRANS_BLOCKS 2	/* We need to update data and inode (atime) */
+#define REISERFS_QUOTA_INIT_BLOCKS (DQUOT_MAX_WRITES*(JOURNAL_PER_BALANCE_CNT+2)+1)	/* 1 balancing, 1 bitmap, 1 data per write + stat data update */
+#else
+#define REISERFS_QUOTA_TRANS_BLOCKS 0
+#define REISERFS_QUOTA_INIT_BLOCKS 0
+#endif
 
 /* both of these can be as low as 1, or as high as you want.  The min is the
 ** number of 4k bitmap nodes preallocated on mount. New nodes are allocated
@@ -1930,12 +1937,21 @@ int reiserfs_do_truncate (struct reiserfs_transaction_handle *th,
 void padd_item (char * item, int total_length, int length);
 
 /* inode.c */
+/* args for the create parameter of reiserfs_get_block */
+#define GET_BLOCK_NO_CREATE 0 /* don't create new blocks or convert tails */
+#define GET_BLOCK_CREATE 1    /* add anything you need to find block */
+#define GET_BLOCK_NO_HOLE 2   /* return -ENOENT for file holes */
+#define GET_BLOCK_READ_DIRECT 4  /* read the tail if indirect item not found */
+#define GET_BLOCK_NO_ISEM     8 /* i_sem is not held, don't preallocate */
+#define GET_BLOCK_NO_DANGLE   16 /* don't leave any transactions running */
+
 int restart_transaction(struct reiserfs_transaction_handle *th, struct inode *inode, struct path *path);
 void reiserfs_read_locked_inode(struct inode * inode, struct reiserfs_iget_args *args) ;
 int reiserfs_find_actor(struct inode * inode, void *p) ;
 int reiserfs_init_locked_inode(struct inode * inode, void *p) ;
 void reiserfs_delete_inode (struct inode * inode);
 int reiserfs_write_inode (struct inode * inode, int) ;
+int reiserfs_get_block (struct inode * inode, sector_t block, struct buffer_head * bh_result, int create);
 struct dentry *reiserfs_get_dentry(struct super_block *, void *) ;
 struct dentry *reiserfs_decode_fh(struct super_block *sb, __u32 *data,
                                      int len, int fhtype,

include/linux/reiserfs_fs_sb.h

@@ -410,6 +410,10 @@ struct reiserfs_sb_info
     struct rw_semaphore xattr_dir_sem;
 
     int j_errno;
+#ifdef CONFIG_QUOTA
+    char *s_qf_names[MAXQUOTAS];
+    int s_jquota_fmt;
+#endif
 };
 
 /* Definitions of reiserfs on-disk properties: */