Skip to content

Commit

Permalink
iocost: adding strict mode
Browse files Browse the repository at this point in the history
adding a strict mode to prevent a potential kernel deadlock bug

Signed-off-by: Robin Lu <robin.lu@bytedance.com>
  • Loading branch information
lubinszARM committed Jul 9, 2024
1 parent 6085f49 commit 135eee0
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 0 deletions.
5 changes: 5 additions & 0 deletions cmd/katalyst-agent/app/options/qrm/io_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type WritebackThrottlingOption struct {

type IOCostOption struct {
EnableSettingIOCost bool
CheckWBTDisabled bool
IOCostQoSConfigFile string
IOCostModelConfigFile string
}
Expand All @@ -62,6 +63,7 @@ func NewIOOptions() *IOOptions {
},
IOCostOption: IOCostOption{
EnableSettingIOCost: false,
CheckWBTDisabled: true,
IOCostQoSConfigFile: "",
IOCostModelConfigFile: "",
},
Expand All @@ -88,6 +90,8 @@ func (o *IOOptions) AddFlags(fss *cliflag.NamedFlagSets) {
o.WBTValueNVME, "writeback throttling value for NVME")
fs.BoolVar(&o.EnableSettingIOCost, "enable-io-cost",
o.EnableSettingIOCost, "if set it to true, io.cost setting will be executed")
fs.BoolVar(&o.CheckWBTDisabled, "check-wbt-disabled",
o.CheckWBTDisabled, "if set it to true, wbt should be disabled")
fs.StringVar(&o.IOCostQoSConfigFile, "io-cost-qos-config-file",
o.IOCostQoSConfigFile, "the absolute path of io.cost.qos qos config file")
fs.StringVar(&o.IOCostModelConfigFile, "io-cost-model-config-file",
Expand All @@ -107,6 +111,7 @@ func (o *IOOptions) ApplyTo(conf *qrmconfig.IOQRMPluginConfig) error {
conf.WBTValueSSD = o.WBTValueSSD
conf.WBTValueNVME = o.WBTValueNVME
conf.EnableSettingIOCost = o.EnableSettingIOCost
conf.CheckWBTDisabled = o.CheckWBTDisabled
conf.IOCostQoSConfigFile = o.IOCostQoSConfigFile
conf.IOCostModelConfigFile = o.IOCostModelConfigFile
conf.EnableSettingIOWeight = o.EnableSettingIOWeight
Expand Down
2 changes: 2 additions & 0 deletions pkg/agent/qrm-plugins/io/handlers/iocost/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ const (
DevModelBcache DevModel = "bcache"
DevModelDeviceMapper DevModel = "dm"

sysDiskPrefix = "/sys/block"

IOStatMetricCostVrate = "cost.vrate"

MetricNameIOCostVrate = "iocost_vrate"
Expand Down
39 changes: 39 additions & 0 deletions pkg/agent/qrm-plugins/io/handlers/iocost/iocost_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@ package iocost

import (
"fmt"
"io/ioutil"
"path/filepath"
"strconv"
"sync"

"github.com/kubewharf/katalyst-core/pkg/config"
coreconfig "github.com/kubewharf/katalyst-core/pkg/config"
dynamicconfig "github.com/kubewharf/katalyst-core/pkg/config/agent/dynamic"
coreconsts "github.com/kubewharf/katalyst-core/pkg/consts"
"github.com/kubewharf/katalyst-core/pkg/metaserver"
"github.com/kubewharf/katalyst-core/pkg/metaserver/agent/metric/helper"
"github.com/kubewharf/katalyst-core/pkg/metrics"
"github.com/kubewharf/katalyst-core/pkg/util/cgroup/common"
cgcommon "github.com/kubewharf/katalyst-core/pkg/util/cgroup/common"
Expand Down Expand Up @@ -236,6 +239,33 @@ func applyIOCostConfig(conf *config.Configuration, emitter metrics.MetricEmitter
applyIOCostModelWithDefault(ioCostModelConfigs, devsIDToModel)
}

func checkWBTDisabled(targetDiskType float64, diskPath string, emitter metrics.MetricEmitter, metaServer *metaserver.MetaServer) (bool, error) {
dir, err := ioutil.ReadDir(diskPath)
if err != nil {
general.Errorf("failed to readdir:%v, err:%v", sysDiskPrefix, err)
return false, err
}
for _, entry := range dir {
diskType, err := helper.GetDeviceMetric(metaServer.MetricsFetcher, emitter, coreconsts.MetricIODiskType, entry.Name())
if err != nil {
general.Errorf("failed to read MetricIODiskType, err:%v", err)
return false, err
}

if diskType == targetDiskType {
WBTValue, err := helper.GetDeviceMetric(metaServer.MetricsFetcher, emitter, coreconsts.MetricIODiskWBTValue, entry.Name())
if err != nil {
general.Errorf("failed to read MetricIODiskWBTValue, err:%v", err)
return false, err
}
if WBTValue != 0 {
return false, nil
}
}
}
return true, nil
}

func SetIOCost(conf *coreconfig.Configuration,
_ interface{},
_ *dynamicconfig.DynamicAgentConfiguration,
Expand Down Expand Up @@ -265,6 +295,15 @@ func SetIOCost(conf *coreconfig.Configuration,
return
}

// Strict mode: checking wbt file.
if conf.CheckWBTDisabled {
disabled, err := checkWBTDisabled(coreconsts.DiskTypeHDD, sysDiskPrefix, emitter, metaServer)
if !disabled {
general.Infof("wbt for HDD disks should be disabled, err=%v", err)
return
}
}

if !cgcommon.CheckCgroup2UnifiedMode() {
general.Infof("not in cgv2 environment, skip IOAsyncTaskFunc")
return
Expand Down
13 changes: 13 additions & 0 deletions pkg/agent/qrm-plugins/io/handlers/iocost/iocost_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ func TestSetIOCost(t *testing.T) {
IOQRMPluginConfig: &qrm.IOQRMPluginConfig{
IOCostOption: qrm.IOCostOption{
EnableSettingIOCost: true,
CheckWBTDisabled: true,
},
},
},
Expand Down Expand Up @@ -411,3 +412,15 @@ func TestApplyIOCostQoSWithDefault(t *testing.T) {
// Call the function under test
applyIOCostQoSWithDefault(ioCostQoSConfigs, devsIDToModel)
}

func TestCheckWBTDisabled(t *testing.T) {
t.Parallel()
metaServer, err := makeMetaServer()
assert.NoError(t, err)

result, _ := checkWBTDisabled(1.0, sysDiskPrefix, metrics.DummyMetrics{}, metaServer)
assert.False(t, result)

result, _ = checkWBTDisabled(1.0, "notExist", metrics.DummyMetrics{}, metaServer)
assert.False(t, result)
}
1 change: 1 addition & 0 deletions pkg/config/agent/qrm/io_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type WritebackThrottlingOption struct {

type IOCostOption struct {
EnableSettingIOCost bool
CheckWBTDisabled bool
IOCostQoSConfigFile string
IOCostModelConfigFile string
}
Expand Down

0 comments on commit 135eee0

Please sign in to comment.