Re: [PATCH] Btrfs: remove unnecessary delalloc mutex for inodes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 25, 2019 at 10:52:42AM +0100, fdmanana@xxxxxxxxxx wrote:
> From: Filipe Manana <fdmanana@xxxxxxxx>
> 
> The inode delalloc mutex was added a long time ago by commit f248679e86fea
> ("Btrfs: add a delalloc mutex to inodes for delalloc reservations"), and
> the reason for its introduction is not very clear from the change log. It
> claims it solves bogus warnings from lockdep, however it lacks an example
> report/warning from lockdep, or any explanation.
> 
> Since we have enough concurrentcy protection from the locks of the space
> info and block reserve objects, and such lockdep warnings don't seem to
> exist anymore (at least on a 5.3 kernel I couldn't get them with fstests,
> ltp, fs_mark, etc), remove it, simplifying things a bit and decreasing
> the size of the btrfs_inode structure. With some quick fio tests doing
> direct IO and mmap writes I couldn't observe any significant performance
> increase either (direct IO writes that don't increase the file's size
> don't hold the inode's lock for their entire duration and mmap writes
> don't hold the inode's lock at all), which are the only type of writes
> that could see any performance gain due to less serialization.
> 
> Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>

The problem was taking the i_mutex in mmap, which is how I was protecting
delalloc reservations originally.  The delalloc mutex didn't come with all of
the other dependencies.  That's what the lockdep messages were about, removing
the lock isn't going to make them appear again.

We _had_ to lock around this because we used to do tricks to keep from
over-reserving, and if we didn't serialize delalloc reservations we'd end up
with ugly accounting problems when we tried to clean things up.

However with my recentish changes this isn't the case anymore.  Every operation
is responsible for reserving its space, and then adding it to the inode.  Then
cleaning up is straightforward and can't be mucked up by other users.  So we no
longer need the delalloc mutex to safe us from ourselves.

Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx>

Thanks,

Josef



[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux