From: Kefu Chai Date: Thu, 3 Nov 2022 06:04:30 +0000 (+0800) Subject: crimson/os/alienstore: parse crimson_alien_thread_cpu_cores as a cpuset(7) X-Git-Tag: v18.1.0~928^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f1ef17edf818e8ab0ca53a3f215dcbd9e41bb0f7;p=ceph-ci.git crimson/os/alienstore: parse crimson_alien_thread_cpu_cores as a cpuset(7) the "List format" listed in cpuset(7) allows us to specify a range of CPU cores in a comma-separated list. and the upper bound of the range is optional. before this change, the upper bound is not optional. before this change, the upper bound of the range is not optional. and the upper bound is not inclusive. so we don't support the list format of cpuset(7). Take cores "1,2,3,5,7,8" for example, we need to set the option to "1-4,5-6,7-9" to represent this cpuset. after this change, the upper bound is optional, so we can properly support the list format defined by cpuset(7). and the upper bound is inclusive. so we can use "1-3,5,7-8", which is compatible with notation defined by cpuset(7). in this change, the cpuset option is parsed using a seastar helper, which is implemented using a regex. so we don't need to manually parse it. as Seastar's parser returns an `std::optional>`. if the string does not match with the regex of comma-separated list, the returned cpuset does not have a value. this design is more explicit. so in this change, instead of using `std::vector`, `std::optional` is used. Signed-off-by: Jianxin Li Signed-off-by: Kefu Chai --- diff --git a/src/common/options/crimson.yaml.in b/src/common/options/crimson.yaml.in index 09a652788ec..9a83b1fd0ec 100644 --- a/src/common/options/crimson.yaml.in +++ b/src/common/options/crimson.yaml.in @@ -22,7 +22,7 @@ options: - name: crimson_alien_thread_cpu_cores type: str level: advanced - desc: CPU cores on which alienstore threads will run + desc: CPU cores on which alienstore threads will run in cpuset(7) format - name: seastore_segment_size type: size desc: Segment size to use for SegmentManager diff --git a/src/crimson/os/alienstore/alien_store.cc b/src/crimson/os/alienstore/alien_store.cc index dd5b5408411..68d268408cc 100644 --- a/src/crimson/os/alienstore/alien_store.cc +++ b/src/crimson/os/alienstore/alien_store.cc @@ -14,6 +14,7 @@ #include #include #include +#include #include "common/ceph_context.h" #include "global/global_context.h" @@ -96,17 +97,20 @@ seastar::future<> AlienStore::start() if (!store) { ceph_abort_msgf("unsupported objectstore type: %s", type.c_str()); } - std::vector cpu_cores = _parse_cpu_cores(); + auto cpu_cores = seastar::resource::parse_cpuset( + get_conf("crimson_alien_thread_cpu_cores")); // cores except the first "N_CORES_FOR_SEASTAR" ones will // be used for alien threads scheduling: // [0, N_CORES_FOR_SEASTAR) are reserved for seastar reactors // [N_CORES_FOR_SEASTAR, ..] are assigned to alien threads. - if (cpu_cores.empty()) { + if (!cpu_cores.has_value()) { if (long nr_cpus = sysconf(_SC_NPROCESSORS_ONLN); nr_cpus > N_CORES_FOR_SEASTAR ) { - for (int i = N_CORES_FOR_SEASTAR; i < nr_cpus; i++) { - cpu_cores.push_back(i); + seastar::resource::cpuset cpuset; + for (unsigned i = N_CORES_FOR_SEASTAR; i < nr_cpus; i++) { + cpuset.insert(i); } + cpu_cores = cpuset; } else { logger().error("{}: unable to get nproc: {}", __func__, errno); } @@ -606,26 +610,4 @@ AlienStore::read_errorator::future> AlienStore::fie }); } -std::vector AlienStore::_parse_cpu_cores() -{ - std::vector cpu_cores; - auto cpu_string = - get_conf("crimson_alien_thread_cpu_cores"); - - std::string token; - std::istringstream token_stream(cpu_string); - while (std::getline(token_stream, token, ',')) { - std::istringstream cpu_stream(token); - std::string cpu; - std::getline(cpu_stream, cpu, '-'); - uint64_t start_cpu = std::stoull(cpu); - std::getline(cpu_stream, cpu, '-'); - uint64_t end_cpu = std::stoull(cpu); - for (uint64_t i = start_cpu; i < end_cpu; i++) { - cpu_cores.push_back(i); - } - } - return cpu_cores; -} - } diff --git a/src/crimson/os/alienstore/alien_store.h b/src/crimson/os/alienstore/alien_store.h index 1b6a4276479..9209dff828b 100644 --- a/src/crimson/os/alienstore/alien_store.h +++ b/src/crimson/os/alienstore/alien_store.h @@ -123,6 +123,5 @@ private: std::unique_ptr cct; mutable seastar::gate op_gate; std::unordered_map coll_map; - std::vector _parse_cpu_cores(); }; } diff --git a/src/crimson/os/alienstore/thread_pool.cc b/src/crimson/os/alienstore/thread_pool.cc index 747d6714e36..5cf9590e61e 100644 --- a/src/crimson/os/alienstore/thread_pool.cc +++ b/src/crimson/os/alienstore/thread_pool.cc @@ -15,7 +15,7 @@ namespace crimson::os { ThreadPool::ThreadPool(size_t n_threads, size_t queue_sz, - std::vector cpus) + const std::optional& cpus) : n_threads(n_threads), queue_size{round_up_to(queue_sz, seastar::smp::count)}, pending_queues(n_threads) @@ -23,8 +23,8 @@ ThreadPool::ThreadPool(size_t n_threads, auto queue_max_wait = std::chrono::seconds(local_conf()->threadpool_empty_queue_max_wait); for (size_t i = 0; i < n_threads; i++) { threads.emplace_back([this, cpus, queue_max_wait, i] { - if (!cpus.empty()) { - pin(cpus); + if (cpus.has_value()) { + pin(*cpus); } block_sighup(); (void) pthread_setname_np(pthread_self(), "alien-store-tp"); @@ -40,7 +40,7 @@ ThreadPool::~ThreadPool() } } -void ThreadPool::pin(const std::vector& cpus) +void ThreadPool::pin(const seastar::resource::cpuset& cpus) { cpu_set_t cs; CPU_ZERO(&cs); diff --git a/src/crimson/os/alienstore/thread_pool.h b/src/crimson/os/alienstore/thread_pool.h index 8f3069af3a5..78e18219a88 100644 --- a/src/crimson/os/alienstore/thread_pool.h +++ b/src/crimson/os/alienstore/thread_pool.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -125,7 +126,7 @@ public: * @note each @c Task has its own crimson::thread::Condition, which possesses * an fd, so we should keep the size of queue under a reasonable limit. */ - ThreadPool(size_t n_threads, size_t queue_sz, std::vector cpus); + ThreadPool(size_t n_threads, size_t queue_sz, const std::optional& cpus); ~ThreadPool(); seastar::future<> start(); seastar::future<> stop(); @@ -163,7 +164,7 @@ private: bool is_stopping() const { return stopping.load(std::memory_order_relaxed); } - static void pin(const std::vector& cpus); + static void pin(const seastar::resource::cpuset& cpus); static void block_sighup(); seastar::semaphore& local_free_slots() { return submit_queue.local().free_slots; diff --git a/src/test/crimson/test_alienstore_thread_pool.cc b/src/test/crimson/test_alienstore_thread_pool.cc index 7cfffec757b..dbeed26cd7d 100644 --- a/src/test/crimson/test_alienstore_thread_pool.cc +++ b/src/test/crimson/test_alienstore_thread_pool.cc @@ -50,7 +50,7 @@ int main(int argc, char** argv) .then([conf_file_list] { return local_conf().parse_config_files(conf_file_list); }).then([] { - return seastar::do_with(std::make_unique(2, 128, (std::vector){0}), + return seastar::do_with(std::make_unique(2, 128, seastar::resource::cpuset{0}), [](auto& tp) { return tp->start().then([&tp] { return test_accumulate(*tp);