Re: Re: [Bug] possible circular locking in reiserfs_unpack

[Frederic Weisbecker <fweisbec@gmail.com>]

On Thu, Sep 09, 2010 at 04:55:34PM +0200, Jarek Poplawski wrote:
> Frederic Weisbecker wrote, On 12/23/-28158 08:59 PM:
> 
> > On Wed, Sep 08, 2010 at 03:37:30PM -0700, Andrew Morton wrote:
> >> On Sun, 5 Sep 2010 13:31:21 +0200
> >> Jarek Poplawski <jarkao2@gmail.com> wrote:
> >>
> >>> Hi,
> >>> I get this warning on every lilo write with 2.6.35.4 and a bit/git
> >>> later too.
> >>>
> >> Can you tell us the latest kernel version which did *not* have this
> >> bug?  That way we can narrow the problem down a bit.
> >>
> >> Thanks.
> > 
> > 
> > 
> > Ah, when you see &REISERFS_SB(s)->lock in a bug report, don't hesitate to blame me :-)
> > 
> > This is a problem resulting from the bkl conversion to a mutex that introduced
> > a lot of new locking dependencies. Most of them have been fixed, but for less
> > tested paths like ioctl, we hear about it later.
> > 
> > Does the following patch fixes the issue?
> > If so, I'll make a proper changelog and put the appropriate 2.6.33-35 stable
> > tags for the backport.
> > 
> > Thnaks!
> > 
> > 
> > diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c
> > index f53505d..679d502 100644
> > --- a/fs/reiserfs/ioctl.c
> > +++ b/fs/reiserfs/ioctl.c
> > @@ -188,7 +188,7 @@ int reiserfs_unpack(struct inode *inode, struct file *filp)
> >  	/* we need to make sure nobody is changing the file size beneath
> >  	 ** us
> >  	 */
> > -	mutex_lock(&inode->i_mutex);
> > +	reiserfs_mutex_lock_safe(&inode->i_mutex, inode->i_sb);
> >  	reiserfs_write_lock(inode->i_sb);
> >  
> >  	write_from = inode->i_size & (blocksize - 1);
> > 
> 
> 
> So, there is still a warning but a bit different now.
> 
> Jarek P.


I can reproduce your first case, but not this one.

So, I hope you can give a try to the following fix,

Thanks!


diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c
index f53505d..90a757b 100644
--- a/fs/reiserfs/ioctl.c
+++ b/fs/reiserfs/ioctl.c
@@ -170,7 +170,7 @@ int reiserfs_prepare_write(struct file *f, struct page *page,
 int reiserfs_unpack(struct inode *inode, struct file *filp)
 {
 	int retval = 0;
-	int index;
+	int index, depth;
 	struct page *page;
 	struct address_space *mapping;
 	unsigned long write_from;
@@ -185,11 +185,12 @@ int reiserfs_unpack(struct inode *inode, struct file *filp)
 		return 0;
 	}
 
+	depth = reiserfs_write_lock_once(inode->i_sb);
+
 	/* we need to make sure nobody is changing the file size beneath
 	 ** us
 	 */
-	mutex_lock(&inode->i_mutex);
-	reiserfs_write_lock(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.  */
@@ -224,6 +225,6 @@ int reiserfs_unpack(struct inode *inode, struct file *filp)
 
       out:
 	mutex_unlock(&inode->i_mutex);
-	reiserfs_write_unlock(inode->i_sb);
+	reiserfs_write_unlock_once(inode->i_sb, depth);
 	return retval;
 }