Messages in this thread | | | Date | Wed, 29 Nov 2023 11:01:04 -0500 | Subject | Re: [PATCH-cgroup 2/2] cgroup/cpuset: Include isolated cpuset CPUs in cpu_is_isolated() check | From | Waiman Long <> |
| |
On 11/28/23 17:12, Tejun Heo wrote: > Hello, > > On Tue, Nov 28, 2023 at 01:32:53PM -0500, Waiman Long wrote: >> On 11/28/23 11:56, Tejun Heo wrote: >>> Hello, >>> >>> On Sun, Nov 26, 2023 at 11:19:56PM -0500, Waiman Long wrote: >>>> +bool cpuset_cpu_is_isolated(int cpu) >>>> +{ >>>> + unsigned int seq; >>>> + bool ret; >>>> + >>>> + do { >>>> + seq = read_seqcount_begin(&isolcpus_seq); >>>> + ret = cpumask_test_cpu(cpu, isolated_cpus); >>>> + } while (read_seqcount_retry(&isolcpus_seq, seq)); >>>> + return ret; >>>> +} >>>> +EXPORT_SYMBOL_GPL(cpuset_cpu_is_isolated); >>> We're testing a bit in a bitmask. I don't think we need to worry about value >>> integrity from RMW updates being broken up. ie. We can just test the bit >>> without seqlock and drop the first patch? >> My concern is that if we have an isolated partition with a set of isolated >> CPUs (say 2-4), I don't want any addition, deletion of changes made to >> another isolated partition affects the test of the pre-existing one. Testing >> result of the partition being change is fair game. >> >> Depending on how the cpumask operators are implemented, we may not have a >> guarantee that testing CPU 2, for instance, will always return true. That is > Can you please elaborate this part a bit? I'm having a difficult time > imagining the sequence of operations where this would matter but that could > easily be me not being familiar with the details.
I may be a bit paranoid about incorrect result due to racing as I had been burned before. Just testing a bit in the bitmask may probably be OK. I don't think it will be a problem for x86, but I am less certain about other more exotic architectures like arm64 or PPC which I am less familiar about. I add a seqcount for synchronization just for the peace of mind. I can take the seqcount out if you don't it is necessary.
I have also been thinking about an alternative helper that returns the whole isolated cpumask since in both cases where cpu_is_isolated() is used, we will have to iterate all the possible CPUs anyway, it will be more efficient to have the whole cpumask available. In that case, we may want to have a seqcount to avoid returning an intermediate result. Anyway, this is just a thought for now, I am not planning to do that at the moment.
Cheers, Longman
| |