On Thu, Oct 28, 2010 at 02:28:23AM +0200, Jan Kara wrote:
> reiserfs_quota_on() unpacks a tail of quota file in case it has one. But after
> BKL conversion, reiserfs_unpack() expects to be called with write_lock held.
> So acquire the lock before calling reiserfs_unpack() to avoid assertion
> failures.
>
> Reported-by: markus.gapp@xxxxxxx
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> fs/reiserfs/super.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> Frederic, would you merge this patch or should I merge it?
>
> diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
> index 6e85cfd..73c000f 100644
> --- a/fs/reiserfs/super.c
> +++ b/fs/reiserfs/super.c
> @@ -2059,7 +2059,9 @@ static int reiserfs_quota_on(struct super_block *sb, int type, int format_id,
> inode = path->dentry->d_inode;
> /* We must not pack tails for quota files on reiserfs for quota IO to work */
> if (!(REISERFS_I(inode)->i_flags & i_nopack_mask)) {
> + reiserfs_write_lock(sb);
> err = reiserfs_unpack(inode, NULL);
> + reiserfs_write_unlock(sb);
> if (err) {
> reiserfs_warning(sb, "super-6520",
> "Unpacking tail of quota file failed"
> --
> 1.7.1
>
Yeah. This is due to a recent fix in reiserfs_unpack().
And in this fix I assumed reiserfs_unpack() was always called under the
reiserfs lock.
I was wrong, I thought that reiserfs_quota_on() was ok because it can
call joural_begin() which appears to have the same requirements.
But no that's probably another bug, journal_begin() should also
be called under the reiserfs lock.
Anyway that must be another patch.
For this specific problem, it might be slightly more proper to do the
below. It lowers a bit the reiserfs lock coverage and also fixes
a weird lock-unlock ordering in reiserfs_unpack() that was doing:
Lock A - Lock B - Unlock A - Unlock B
Hmm?
diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c
index 5cbb81e..af2a58f 100644
--- a/fs/reiserfs/ioctl.c
+++ b/fs/reiserfs/ioctl.c
@@ -189,8 +189,8 @@ int reiserfs_unpack(struct inode *inode, struct file *filp)
/* we need to make sure nobody is changing the file size beneath
** us
*/
- reiserfs_mutex_lock_safe(&inode->i_mutex, inode->i_sb);
depth = reiserfs_write_lock_once(inode->i_sb);
+ reiserfs_mutex_lock_safe(&inode->i_mutex, inode->i_sb);
write_from = inode->i_size & (blocksize - 1);
/* if we are on a block boundary, we are already unpacked. */
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
