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.