On Mon, Apr 15, 2019 at 02:44:19PM +0000, Filipe Manana wrote:
> On Mon, Apr 15, 2019 at 3:33 PM David Sterba <dsterba@xxxxxxx> wrote:
> >
> > On Mon, Apr 15, 2019 at 09:29:10AM +0100, fdmanana@xxxxxxxxxx wrote:
> > > From: Filipe Manana <fdmanana@xxxxxxxx>
> > >
> > > As of commit bb739cf08e8f32 ("btrfs: btrfs_check_shared should manage its
> > > own transaction") btrfs_check_shared() manages its own transaction, it
> > > no longer receives a transaction handle as parameter. Since it is used
> > > only in the fiemap path, the ulists it needs to use are allocated before
> > > it grabs a transaction handle and we are not holding any locks that could
> > > cause a deadlock in case reclaim happens during a memory allocation, we
> > > can use the GFP_KERNEL flag to allocate the ulists instead of GFP_NOFS.
> > > So do that switch.
> > >
> > > Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
> >
> > Reviewed-by: David Sterba <dsterba@xxxxxxxx>
>
> Actually no, this isn't safe, drop it please.
> An inode's range is locked, the allocations can trigger writeback for
> that inode, and writepages() needs to lock the range as well.
Ah, right, lock_extent_bits. I think the ulist allocations can be moved
outside of the section to extent_fiemap, each call of btrfs_check_shared
allocates and frees.
