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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
