[PATCH] fix reiserfs bad path release panic
authorSuzuki K P <suzuki@in.ibm.com>
Thu, 7 Dec 2006 04:36:10 +0000 (20:36 -0800)
committerLinus Torvalds <torvalds@woody.osdl.org>
Thu, 7 Dec 2006 16:39:32 +0000 (08:39 -0800)
One of our test team hit a reiserfs_panic while running fsstress tests on
2.6.19-rc1.  The message looks like :

  REISERFS: panic(device Null superblock):
  reiserfs[5676]: assertion !(p->path_length != 1 ) failed at
  fs/reiserfs/stree.c:397:reiserfs_check_path: path not properly relsed.

The backtrace looked :

  kernel BUG in reiserfs_panic at fs/reiserfs/prints.c:361!
.reiserfs_check_path+0x58/0x74
.reiserfs_get_block+0x1444/0x1508
.__block_prepare_write+0x1c8/0x558
.block_prepare_write+0x34/0x64
.reiserfs_prepare_write+0x118/0x1d0
.generic_file_buffered_write+0x314/0x82c
.__generic_file_aio_write_nolock+0x350/0x3e0
.__generic_file_write_nolock+0x78/0xb0
.generic_file_write+0x60/0xf0
.reiserfs_file_write+0x198/0x2038
.vfs_write+0xd0/0x1b4
.sys_write+0x4c/0x8c
syscall_exit+0x0/0x4

Upon debugging I found that the restart_transaction was not releasing
the path if the th->refcount was > 1.

/*static*/
int restart_transaction(struct reiserfs_transaction_handle *th,
                            struct inode *inode, struct path *path)
{
[...]

         /* we cannot restart while nested */
         if (th->t_refcount > 1) { <<- Path is not released in this case!
                 return 0;
         }

         pathrelse(path); <<- Path released here.
[...]

This could happen in such a situation :

In reiserfs/inode.c: reiserfs_get_block() ::

      if (repeat == NO_DISK_SPACE || repeat == QUOTA_EXCEEDED) {
          /* restart the transaction to give the journal a chance to free
           ** some blocks.  releases the path, so we have to go back to
           ** research if we succeed on the second try
           */
          SB_JOURNAL(inode->i_sb)->j_next_async_flush = 1;

        -->>  retval = restart_transaction(th, inode, &path); <<--

  We are supposed to release the path, no matter we succeed or fail. But
if the th->refcount is > 1, the path is still valid. And,

          if (retval)
                   goto failure;
          repeat =
              _allocate_block(th, block, inode,
                             &allocated_block_nr, NULL, create);

If the above allocate_block fails with NO_DISK_SPACE or QUOTA_EXCEEDED,
we would have path which is not released.

         if (repeat != NO_DISK_SPACE && repeat != QUOTA_EXCEEDED) {
                   goto research;
         }
         if (repeat == QUOTA_EXCEEDED)
                   retval = -EDQUOT;
         else
                   retval = -ENOSPC;
         goto failure;
[...]

       failure:
[...]
         reiserfs_check_path(&path); << Panics here !

Attached here is a patch which could fix the issue.

fix reiserfs/inode.c : restart_transaction() to release the path in all
cases.

The restart_transaction() doesn't release the path when the the journal
handle has a refcount > 1.  This would trigger a reiserfs_panic() if we
encounter an -ENOSPC / -EDQUOT in reiserfs_get_block().

Signed-off-by: Suzuki K P <suzuki@in.ibm.com>
Cc: "Vladimir V. Saveliev" <vs@namesys.com>
Cc: <reiserfs-dev@namesys.com>
Cc: Jeff Mahoney <jeffm@suse.com>
Acked-by: Jan Kara <jack@suse.cz>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
fs/reiserfs/inode.c

index 9c69bca..a625688 100644 (file)
@@ -216,11 +216,12 @@ static int file_capable(struct inode *inode, long block)
        BUG_ON(!th->t_trans_id);
        BUG_ON(!th->t_refcount);
 
+       pathrelse(path);
+
        /* we cannot restart while nested */
        if (th->t_refcount > 1) {
                return 0;
        }
-       pathrelse(path);
        reiserfs_update_sd(th, inode);
        err = journal_end(th, s, len);
        if (!err) {