On Mon, Mar 25, 2019 at 06:43:49PM +0200, Nikolay Borisov wrote:
> >> -static int __must_check
> >> +int __must_check
> >> __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
> >> unsigned bits, unsigned exclusive_bits,
> >> u64 *failed_start, struct extent_state **cached_state,
> >
> > Does this really need to be exported again? There are helpers that can
> > wrap specific combinations of parameters.
>
> This is exported so that __set_extent_bit could be called with
> GFP_NOWAIT parameter otherwise I was getting lockdep splats since
> extent_map_device_set_bits (called from add_extent_mapping) is called
> under a write_lock hence we can't sleep.
So then we should add a helper for that, like
int set_exentent_bit_nowait(...)
similar to set_record_extent_bits.
> >> @@ -335,6 +335,8 @@ void btrfs_free_device(struct btrfs_device *device)
> >> {
> >> WARN_ON(!list_empty(&device->post_commit_list));
> >> rcu_string_free(device->name);
> >> + if (!in_softirq())
> >> + extent_io_tree_release(&device->alloc_state);
>
> This is used to distinguish between btrfs_free_device being called from
> btrfs_close_devices in close_ctree i.e non rcu (hence no softirq )
> context or any of the error handlers and from free_device_rcu. In the
> latter case the extent tree is already freed in btrfs_close_one_device,
> hence there is no need to do it in the RCU callback.
Ok, I can cook a comment from that, thanks.
> Furthermore, there is also a comment that the extent io tree cannot be
> destroyed in RCU context because extent_io_tree_release calls
> cond_resched_lock which in turn could sleep, but this is forbidden in
> RCU context.
Yeah, comment in the right place.
