Re: [BUG] Reiserfs panic while running fsstress due to multipletruncate "safe links" for a file.

From: Andrew Morton
Date: Mon Nov 27 2006 - 20:27:40 EST


On Mon, 27 Nov 2006 15:37:49 -0800
Suzuki <suzuki@xxxxxxxxxx> wrote:

> * Do not add save links for O_DIRECT writes.
>
> We add a save link for O_DIRECT writes to protect the i_size against the crashes before we actually finish the I/O. If we hit an -ENOSPC in aops->prepare_write(), we would do a truncate() to release the blocks which might have got initialized. Now the truncate would add another save link for the same inode causing a reiserfs panic for having multiple save links for the same inode.
>
>

OK...

But how does this patch fix it? It removes a lot of code - how come we
don't need it any more?

>
>
> Index: linux-2.6.19-rc1/fs/reiserfs/file.c
> ===================================================================
> --- linux-2.6.19-rc1.orig/fs/reiserfs/file.c 2006-10-10 05:54:30.000000000 -0700
> +++ linux-2.6.19-rc1/fs/reiserfs/file.c 2006-11-21 17:17:36.000000000 -0800
> @@ -1306,56 +1306,8 @@
> count = MAX_NON_LFS - (unsigned long)*ppos;
> }
>
> - if (file->f_flags & O_DIRECT) { // Direct IO needs treatment
> - ssize_t result, after_file_end = 0;
> - if ((*ppos + count >= inode->i_size)
> - || (file->f_flags & O_APPEND)) {
> - /* If we are appending a file, we need to put this savelink in here.
> - If we will crash while doing direct io, finish_unfinished will
> - cut the garbage from the file end. */
> - reiserfs_write_lock(inode->i_sb);
> - err =
> - journal_begin(&th, inode->i_sb,
> - JOURNAL_PER_BALANCE_CNT);
> - if (err) {
> - reiserfs_write_unlock(inode->i_sb);
> - return err;
> - }
> - reiserfs_update_inode_transaction(inode);
> - add_save_link(&th, inode, 1 /* Truncate */ );
> - after_file_end = 1;
> - err =
> - journal_end(&th, inode->i_sb,
> - JOURNAL_PER_BALANCE_CNT);
> - reiserfs_write_unlock(inode->i_sb);
> - if (err)
> - return err;
> - }
> - result = do_sync_write(file, buf, count, ppos);
> -
> - if (after_file_end) { /* Now update i_size and remove the savelink */
> - struct reiserfs_transaction_handle th;
> - reiserfs_write_lock(inode->i_sb);
> - err = journal_begin(&th, inode->i_sb, 1);
> - if (err) {
> - reiserfs_write_unlock(inode->i_sb);
> - return err;
> - }
> - reiserfs_update_inode_transaction(inode);
> - mark_inode_dirty(inode);
> - err = journal_end(&th, inode->i_sb, 1);
> - if (err) {
> - reiserfs_write_unlock(inode->i_sb);
> - return err;
> - }
> - err = remove_save_link(inode, 1 /* truncate */ );
> - reiserfs_write_unlock(inode->i_sb);
> - if (err)
> - return err;
> - }
> -
> - return result;
> - }
> + if (file->f_flags & O_DIRECT)
> + return do_sync_write(file, buf, count, ppos);
>
> if (unlikely((ssize_t) count < 0))
> return -EINVAL;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/