lkml.org 
[lkml]   [2010]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: locking problems: Btrfs: be more selective in the defrag ioctl
On Mon, Mar 22, 2010 at 09:47:21AM -0400, Josef Bacik wrote:
> On Sat, Mar 20, 2010 at 02:38:51PM +0300, Dan Carpenter wrote:
> > Hi Chris,
> >
> > There is a locking problem in
> > 940100a4a7b78 "Btrfs: be more selective in the defrag ioctl"
> >
> > There are two places where we break out of the while loop under the
> > lock.
> >
> > fs/btrfs/ioctl.c +708 btrfs_defrag_file(159) error: double lock 'mutex:&inode->i_mutex'
> > 600 mutex_lock(&inode->i_mutex);
> > 601 if (range->flags & BTRFS_DEFRAG_RANGE_COMPRESS)
> > 602 BTRFS_I(inode)->force_compress = 1;
> > 603
> > 604 ret = btrfs_check_data_free_space(root, inode, PAGE_CACHE_SIZE);
> > 605 if (ret) {
> > 606 ret = -ENOSPC;
> > 607 break;
> >
> > Here.
> >
> > 608 }
> > 609
> > 610 ret = btrfs_reserve_metadata_for_delalloc(root, inode, 1);
> > 611 if (ret) {
> > 612 btrfs_free_reserved_data_space(root, inode,
> > 613 PAGE_CACHE_SIZE);
> > 614 ret = -ENOSPC;
> > 615 break;
> >
> > And here.
> >
> > 616 }
> >
> > Maybe we should have "goto err_reservations;" instead of break? I
> > don't know the code well enough to say.
>
> No, everything is accounted for correctly. If the metadata reservation fails,
> we free the data space reservation and break. If the data space reservation
> fails, we're good to go and can just exit. Thanks,
>

What about the lock on line 606?

> > 600 mutex_lock(&inode->i_mutex);

If we break on line 615 or 607 that means that we return with the lock
held, or if (range->flags & BTRFS_DEFRAG_RANGE_COMPRESS) is true then we
dead lock.

regards,
dan carpenter

> Josef


\
 
 \ /
  Last update: 2010-03-22 15:05    [W:0.061 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site