On 1.11.18 г. 18:44 ч., David Sterba wrote:
> On Thu, Nov 01, 2018 at 09:00:56AM -0700, Omar Sandoval wrote:
>>> That was an assumption for the demonstration purposes, the wording was
>>> confusing sorry.
>>
>> Oh, well in that case, that's exactly what kthread_park() is ;) Stop the
>> thread and make wake_up a noop, and then we don't need to add special
>> cases everywhere else.
>
> The end result is equivalent and should be, I'm now weighing both
> approaches. Explicit state tracking outside of the thread structures
> seems more clear and in case we want to query the status, we can't after
> kthread_stop is called. My current version below.
>
<snip>
> +static inline void btrfs_wake_up_transaction(struct btrfs_fs_info *fs_info)
> +{
> + if (!test_bit(BTRFS_FS_CLOSING_SINGLE, &fs_info->flags)) {
Why do we need a separate flag for a predicate? If we use
FS_CLOSING_START then waking up becomes a noop from the moment
close_ctree is called.
> + wake_up_process(fs_info->transaction_kthread);
> + } else {
> + btrfs_debug(fs_info,
> + "attempt to wake up transaction kthread during
> shutdown");
> + }
> +}
> +
> /*
> * Try to commit transaction asynchronously, so this is safe to call
> * even holding a spinlock.
> @@ -210,7 +229,7 @@ static inline void btrfs_commit_transaction_locksafe(
> struct btrfs_fs_info *fs_info)
> {
> set_bit(BTRFS_FS_NEED_ASYNC_COMMIT, &fs_info->flags);
> - wake_up_process(fs_info->transaction_kthread);
> + btrfs_wake_up_transaction(fs_info);
> }
> int btrfs_end_transaction_throttle(struct btrfs_trans_handle *trans);
> int btrfs_should_end_transaction(struct btrfs_trans_handle *trans);
>
