| Message ID | 20200610161226.424337-1-nitesh@redhat.com |
|---|---|
| Headers | show |
| Series | Preventing job distribution to isolated CPUs | expand |
Hi Nitesh, On Wed, Jun 10, 2020 at 12:12:23PM -0400, Nitesh Narayan Lal wrote: > This patch-set is originated from one of the patches that have been > posted earlier as a part of "Task_isolation" mode [1] patch series > by Alex Belits <abelits@marvell.com>. There are only a couple of > changes that I am proposing in this patch-set compared to what Alex > has posted earlier. > > > Context > ======= > On a broad level, all three patches that are included in this patch > set are meant to improve the driver/library to respect isolated > CPUs by not pinning any job on it. Not doing so could impact > the latency values in RT use-cases. > > > Patches > ======= > * Patch1: > The first patch is meant to make cpumask_local_spread() > aware of the isolated CPUs. It ensures that the CPUs that > are returned by this API only includes housekeeping CPUs. > > * Patch2: > This patch ensures that a probe function that is called > using work_on_cpu() doesn't run any task on an isolated CPU. > > * Patch3: > This patch makes store_rps_map() aware of the isolated > CPUs so that rps don't queue any jobs on an isolated CPU. > > > Changes > ======= > To fix the above-mentioned issues Alex has used housekeeping_cpumask(). > The only changes that I am proposing here are: > - Removing the dependency on CONFIG_TASK_ISOLATION that was proposed by Alex. > As it should be safe to rely on housekeeping_cpumask() > even when we don't have any isolated CPUs and we want > to fall back to using all available CPUs in any of the above scenarios. > - Using both HK_FLAG_DOMAIN and HK_FLAG_WQ in all three patches, this is > because we would want the above fixes not only when we have isolcpus but > also with something like systemd's CPU affinity. > > > Testing > ======= > * Patch 1: > Fix for cpumask_local_spread() is tested by creating VFs, loading > iavf module and by adding a tracepoint to confirm that only housekeeping > CPUs are picked when an appropriate profile is set up and all remaining CPUs > when no CPU isolation is required/configured. > > * Patch 2: > To test the PCI fix, I hotplugged a virtio-net-pci from qemu console > and forced its addition to a specific node to trigger the code path that > includes the proposed fix and verified that only housekeeping CPUs > are included via tracepoint. I understand that this may not be the > best way to test it, hence, I am open to any suggestion to test this > fix in a better way if required. > > * Patch 3: > To test the fix in store_rps_map(), I tried configuring an isolated > CPU by writing to /sys/class/net/en*/queues/rx*/rps_cpus which > resulted in 'write error: Invalid argument' error. For the case > where a non-isolated CPU is writing in rps_cpus the above operation > succeeded without any error. > > [1] https://patchwork.ozlabs.org/project/netdev/patch/51102eebe62336c6a4e584c7a503553b9f90e01c.camel@marvell.com/ > > Alex Belits (3): > lib: restricting cpumask_local_spread to only houskeeping CPUs > PCI: prevent work_on_cpu's probe to execute on isolated CPUs > net: restrict queuing of receive packets to housekeeping CPUs > > drivers/pci/pci-driver.c | 5 ++++- > lib/cpumask.c | 43 +++++++++++++++++++++++----------------- > net/core/net-sysfs.c | 10 +++++++++- > 3 files changed, 38 insertions(+), 20 deletions(-) > > -- > Looks good to me. The flags mechanism is not well organized: this is using HK_FLAG_WQ to infer nohz_full is being set (while HK_FLAG_WQ should indicate that non-affined workqueue threads should not run on certain CPUs). But this is a problem of the flags (which apparently Frederic wants to fix by exposing a limited number of options to users), and not of this patch.