Skip to content

feat: add short idle and TLP mode support#1111

Open
xionglinlin wants to merge 1 commit into
linuxdeepin:masterfrom
xionglinlin:feat/short-idle-tlp
Open

feat: add short idle and TLP mode support#1111
xionglinlin wants to merge 1 commit into
linuxdeepin:masterfrom
xionglinlin:feat/short-idle-tlp

Conversation

@xionglinlin
Copy link
Copy Markdown
Contributor

  1. Add short idle state management with kernel file writing via dde- system-daemon
  2. Add TLP mode setting interface in system power1
  3. Implement short idle delay configuration and power saving plan
  4. Add third-party application detection to control short idle entry
  5. Add screen state from DPMS off/on events
  6. Add DSG configuration for idle paths, delays, and blacklists

Log: Added short idle state management and TLP mode configuration for power saving optimization

Influence:

  1. Test short idle entry and exit with various delay configurations
  2. Verify TLP mode switching via D-Bus interface
  3. Test third-party application detection prevents short idle entry
  4. Verify short idle blacklist and whitelist application behavior
  5. Test screen state synchronization with DPMS events
  6. Verify kernel file writing for idle and screen states
  7. Test DSG configuration changes are properly applied

feat: 添加短idle和TLP模式支持

  1. 通过dde-system-daemon添加短idle状态管理及内核文件写入功能
  2. 在系统电源管理中新增TLP模式设置接口
  3. 实现短idle延迟配置和节能计划管理
  4. 添加第三方应用检测机制以控制短idle进入
  5. 从DPMS关闭/打开事件同步屏幕状态
  6. 新增DSG配置项:空闲路径、延迟时间和黑名单

Log: 新增短idle状态管理和TLP模式配置,优化节能策略

Influence:

  1. 测试不同延迟配置下的短idle进入和退出
  2. 通过D-Bus接口验证TLP模式切换功能
  3. 测试第三方应用运行阻止短idle进入的逻辑
  4. 验证短idle黑名单和白名单应用的行为
  5. 测试屏幕状态与DPMS事件的同步
  6. 验证空闲和屏幕状态的内核文件写入
  7. 测试DSG配置变更的正确应用

PMS: TASK-389737
Change-Id: Ia3572bf438ff45f8c67e7f354f991fd6535f7775

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @xionglinlin, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: xionglinlin

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Copy Markdown
Contributor

@mhduiy mhduiy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: PR #1111

整体功能完整,涵盖了短 idle 管理、TLP 模式、第三方应用检测、DSG 配置等。以下按严重程度分级列出问题和建议:


🔴 需要修复的问题

1. system/power1/manager.go - setTlpMode 未更新 m.TlpMode 字段

func (m *Manager) setTlpMode(mode string) error {
    if m.TlpMode == mode {
        return errors.New("repeat set tlp mode")
    }
    // ... 设置 DSPCState
    return nil
}

问题m.TlpMode 从未被赋值为新的 mode,导致"重复设置"的保护逻辑完全无效,每次都会重新执行 setDSPCState
建议:在设置成功后更新 m.TlpMode = mode

2. session/power1/power_save_plan.go - Map 并发读写竞态

systemApplicationsMapshortIdleBlackListApplicationsMap 在以下场景存在竞态:

  • initDsgConfigValueChanged 回调中写 map(在 signalLoop goroutine 中)
  • isThirdPartyAppRunning 中读 map(在 metaTask 执行中)
  • 没有任何 mutex 保护,可能触发 concurrent map read and map write panic。
    建议:使用 sync.RWMutex 保护这两个 map 的读写,或者使用 sync.Map

3. bin/dde-system-daemon/main.go - _daemon.service 重复赋值

_daemon = &Daemon{
    service:             service,
    // ...
}
_daemon.getDsgValue()  // getDsgValue 中创建了自己的 configManager 连接
_daemon.service = service  // ← 重复赋值,无意义

建议:删除第 121 行的 _daemon.service = service

4. system/power1/manager.go - doSetMode 中的延时 hack

time.AfterFunc(500*time.Millisecond, func() {
    err := m.setTlpMode(ddePowerSave)
    // ...
})

问题:用固定 500ms 延时来规避 deepin-power-control 连续调用失败的问题,这是治标不治本的方案。在高负载或低性能设备上 500ms 可能不够,在正常设备上又浪费时间。
建议:修复 deepin-power-control 连续调用的根本问题,或至少增加重试逻辑 + 上限超时。


🟡 建议改进

5. bin/dde-system-daemon/power.go - 使用已废弃的 ioutil

content, err := ioutil.ReadFile(file)
err = ioutil.WriteFile(file, []byte(newContent), 0644)

建议:替换为 os.ReadFileos.WriteFileioutil 自 Go 1.16 起已废弃)。

6. bin/dde-system-daemon/power.go - isStrInList 重复造轮子

func isStrInList(item string, items []string) bool {

建议:如果项目 Go 版本 ≥ 1.21,使用 slices.Contains(item, items) 替代。

7. bin/dde-system-daemon/power.go - setStateshortIdleState 零值问题

shortIdleState, err := d.systemPower.ShortIdleState().Get(0)
if err != nil {
    logger.Warning("Get systemPower.ShortIdleState err :", err)
}
// 如果出错,shortIdleState 为零值 false
if shortIdleState == state {
    return errors.New("Short idle state not exchange.")
}

问题:D-Bus 调用失败时 shortIdleState 默认为 false,如果传入的 state 也是 false,会误判为"状态未变化"并返回错误。
建议:D-Bus 调用失败时应该直接返回错误,而不是继续判断。

8. session/power1/power_save_plan.go - changeShortIdleState 阻塞 sleep

func (psp *powerSavePlan) changeShortIdleState(state bool) {
    // ...
    psp.setDsg(dsettingsShortIdleState, state)
    time.Sleep(300 * time.Millisecond)  // ← 阻塞 300ms
    callSetIdleState(state)
}

问题:在 metaTask 执行路径中阻塞 300ms,可能影响其他电源状态转换的响应时间。
建议:使用 time.AfterFunc 或在 goroutine 中处理,避免阻塞主流程。

9. session/power1/power_save_plan.go - getLaunchedApplications 性能问题

func getLaunchedApplications() []string {
    // 同步 D-Bus 调用,获取所有应用实例
}

问题:每次检查是否进入短 idle 时,都会同步查询 ApplicationManager 获取所有运行中的应用。在系统运行大量应用时,这可能导致明显的延迟。
建议:考虑缓存应用列表,或通过信号增量更新,而非每次全量查询。

10. session/power1/power_save_plan.go - isThirdPartyAppRunning 的启发式判断

if strings.Contains(desktop, "deepin") || strings.Contains(desktop, "dde") || strings.Contains(desktop, "uos") {
    logger.Warning("Need add systemApplicationsMap, Running app : ", app, desktop)
    continue
}

问题:仅通过文件名包含 deepin/dde/uos 来判断是否为系统应用过于宽泛,且日志级别为 Warning 但实际是正常业务逻辑。第三方应用也可能包含这些字符串。
建议:严格依赖 systemApplications 配置列表,如果发现缺失项应补充到配置中,而非运行时猜测。日志级别改为 Debug

11. system/power1/manager.go - setTlpModesetShortIdleState 中 goroutine 无错误反馈

go m.setDSPCState(_powerConfigMap[mode].DSPCConfig)
// ...
go func() {
    m.setDSPCState(_powerConfigMap[powerState].DSPCConfig)
    m.setDPCWifiState(wifiState)
}()

问题:异步执行但无任何错误处理或完成确认。如果设置失败,调用方完全无感知。
建议:至少添加错误日志;如果调用方需要知道结果,考虑使用 channel 或 callback。

12. 硬编码 LoongArch 路径缺乏架构保护

const (
    IdleFile       = "/sys/devices/system/loongarch/relax_state"
    IdleScreenFile = "/sys/devices/system/loongarch/idle_state"
)

问题:这些路径仅适用于 LoongArch 架构,在 x86/ARM 设备上文件不存在。
建议:添加架构检查(runtime.GOARCH),或在 getDsgValue 中通过 DSG 配置覆盖路径(当前已有此机制,但默认值仍会初始化为 LoongArch 路径)。

13. misc/dsg-configs/org.deepin.dde.daemon.power.json - shortIdleState 权限

"shortIdleState": {
    "permissions": "readwrite",
    "visibility": "private"
}

问题shortIdleState 被 session 侧和 system 侧同时写入,谁是权威来源?建议明确文档说明,或改为仅由一侧写入。


✅ 做得好的地方

  • DSG 配置结构清晰,配置项命名规范
  • systemApplications 白名单机制设计合理
  • 空闲延迟配置支持插电/电池分别设置
  • DPMS 屏幕状态同步到内核节点的逻辑清晰
  • exported_methods_auto.go 遵循项目约定

"dde-lock.desktop",
"dde-clipboard.desktop",
"dde-clipboard-daemon.desktop",
"dde-launcher.desktop",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1

"dde-file-manager.desktop",
"dde-computer.desktop",
"dde-trash.desktop",
"dde-control-center.desktop",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1

"deepin-diskmanager.desktop",
"deepin-camera.desktop",
"deepin-data-transfer.desktop",
"deepin-ab-recovery.desktop",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1

"gnome-keyring-ssh.desktop",
"gnome-keyring-pkcs11.desktop",
"gnome-keyring-secrets.desktop",
"fcitx-helper.desktop",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1

Comment thread misc/dsg-configs/org.deepin.dde.daemon.power.json
@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 20, 2026

TAG Bot

New tag: 6.1.90
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1117

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 22, 2026

TAG Bot

New tag: 6.1.91
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1121

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 25, 2026

TAG Bot

New tag: 6.1.92
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1127

1. Add short idle state management with kernel file writing via dde-
system-daemon
2. Add TLP mode setting interface in system power1
3. Implement short idle delay configuration and power saving plan
4. Add third-party application detection to control short idle entry
5. Add screen state from DPMS off/on events
6. Add DSG configuration for idle paths, delays, and blacklists

Log: Added short idle state management and TLP mode configuration for
power saving optimization

Influence:
1. Test short idle entry and exit with various delay configurations
2. Verify TLP mode switching via D-Bus interface
3. Test third-party application detection prevents short idle entry
4. Verify short idle blacklist and whitelist application behavior
5. Test screen state synchronization with DPMS events
6. Verify kernel file writing for idle and screen states
7. Test DSG configuration changes are properly applied

feat: 添加短idle和TLP模式支持

1. 通过dde-system-daemon添加短idle状态管理及内核文件写入功能
2. 在系统电源管理中新增TLP模式设置接口
3. 实现短idle延迟配置和节能计划管理
4. 添加第三方应用检测机制以控制短idle进入
5. 从DPMS关闭/打开事件同步屏幕状态
6. 新增DSG配置项:空闲路径、延迟时间和黑名单

Log: 新增短idle状态管理和TLP模式配置,优化节能策略

Influence:
1. 测试不同延迟配置下的短idle进入和退出
2. 通过D-Bus接口验证TLP模式切换功能
3. 测试第三方应用运行阻止短idle进入的逻辑
4. 验证短idle黑名单和白名单应用的行为
5. 测试屏幕状态与DPMS事件的同步
6. 验证空闲和屏幕状态的内核文件写入
7. 测试DSG配置变更的正确应用

PMS: TASK-389737
Change-Id: Ia3572bf438ff45f8c67e7f354f991fd6535f7775
@xionglinlin xionglinlin force-pushed the feat/short-idle-tlp branch from 77befed to 7d12dee Compare May 26, 2026 09:20
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份代码主要实现了“短idle (Short Idle)”的电源管理功能,涉及内核节点的读写、DConfig配置的获取与监听、DBus接口的暴露以及应用黑名单/白名单的检测。整体逻辑较为完整,但在代码质量、性能、并发安全以及代码安全方面存在一些需要改进的地方。

以下是详细的审查意见:

1. 语法与逻辑

  • Map 初始化与清空逻辑错误
    session/power1/power_save_plan.goinitDsgConfig 中:

    if len(psp.manager.systemApplicationsMap) != 0 {
        psp.manager.systemApplicationsMap = make(map[string]string)
    }

    问题:这里的逻辑是“如果Map长度不为0,则重新Make一个Map”。这虽然能达到清空的目的,但判断条件是多余的,且容易让人误解为“如果为空就不初始化”。
    建议:直接清空或重新初始化即可,无需判断长度:

    psp.manager.systemApplicationsMap = make(map[string]string)
    // 或者使用 clear(psp.manager.systemApplicationsMap) // Go 1.21+
  • DBus 方法参数类型不匹配
    session/power1/power_save_plan.gokeybinding1/utils.go 中调用系统DBus方法时:

    err = systemConn.Object(...).Call("org.deepin.dde.Daemon1.SetIdleState", 0, dbus.MakeVariant(state)).Err

    问题:在 exported_methods_auto.go 中,SetIdleStateSetScreenStateInArgs 定义为 []string{"state"},根据 dbusutil 的生成规则,Go端接收的参数类型通常是 bool。但调用端使用了 dbus.MakeVariant(state),这会导致DBus底层传递的类型为 Variant 而非 Boolean,可能引发DBus类型签名不匹配的运行时错误。
    建议:直接传递 state 变量,让 godbus 自动进行类型推断和签名:

    err = systemConn.Object("org.deepin.dde.Daemon1", "/org/deepin/dde/Daemon1").Call("org.deepin.dde.Daemon1.SetIdleState", 0, state).Err
  • setState 中的状态判断逻辑存在漏洞
    bin/dde-system-daemon/power.go 中:

    shortIdleState, err := d.systemPower.ShortIdleState().Get(0)
    // ...
    if shortIdleState == state {
        logger.Info("shortIdleState is same with state : ", state)
        return errors.New("Short idle state not exchange.")
    }

    问题:如果获取 shortIdleState 失败(err != nil),代码仅仅打印了Warning,然后继续往下执行。此时 shortIdleState 为零值,如果 state 也为 false,就会错误地返回 Short idle state not exchange. 错误,导致后续写内核节点的逻辑被阻断。
    建议:获取状态失败时,不应使用该零值进行判断,应当允许继续执行写入或者直接返回更明确的错误:

    shortIdleState, err := d.systemPower.ShortIdleState().Get(0)
    if err != nil {
        logger.Warning("Get systemPower.ShortIdleState err :", err)
        // 获取失败时,跳过状态相同的判断,强制写入,或者直接返回 err
    } else if shortIdleState == state {
        logger.Info("shortIdleState is same with state : ", state)
        return errors.New("Short idle state not exchange.")
    }

2. 代码性能

  • 频繁建立 DBus 连接
    callSetIdleStatecallSetScreenStategetLaunchedApplications 每次调用都会通过 dbus.SystemBus()dbus.SessionBus() 建立新的连接。
    问题:在电源管理这种需要频繁调用的场景下,频繁创建和销毁DBus连接是非常消耗系统资源的。
    建议:在 ManagerDaemon 结构体初始化时建立全局的 DBus 连接并复用,或者利用已有的 service.Conn() / systemSigLoop.Conn()

  • isThirdPartyAppRunning 性能瓶颈
    问题:每次进入短idle检测时,都会通过 GetManagedObjects 拉取所有应用信息,并在本地做字符串匹配(且包含 strings.ToLowerstrings.Contains)。这在应用数量较多时会产生明显的CPU和内存开销。
    建议

    1. 缓存 getLaunchedApplications 的结果,监听 ApplicationManager 的 InterfacesAdded / InterfacesRemoved 信号来增量更新缓存。
    2. 黑白名单的匹配逻辑可以优化:将 strings.ToLower 操作提前到构建 Map 时完成(当前代码在构建Map时已经做了 strings.ToLower,但在匹配时又对 app 做了 ToLower,这是对的,但 strings.Contains 是多余的消耗)。
  • time.Sleep 阻塞
    changeShortIdleState 中使用了 time.Sleep(300 * time.Millisecond)
    问题:阻塞当前 goroutine 300ms 是一种反模式,特别是在持有锁或处理关键逻辑路径时。如果是为了等待 DConfig 生效或状态同步,应当使用事件驱动或回调机制。
    建议:如果必须等待,考虑使用 time.AfterFunc 或 channel 机制;如果可以,去掉 Sleep 并依赖底层状态变更的信号回调。

3. 代码质量

  • 废弃的 ioutil
    bin/dde-system-daemon/power.go 中使用了 ioutil.ReadFileioutil.WriteFile
    问题:自 Go 1.16 起,io/ioutil 包已被废弃。
    建议:替换为 os.ReadFileos.WriteFile

  • 硬编码的架构路径

    const (
        IdleFile       = "/sys/devices/system/loongarch/relax_state"
        IdleScreenFile = "/sys/devices/system/loongarch/idle_state"
    )

    问题:路径中包含了 loongarch(龙芯架构),这使得代码在 x86 或 ARM 等其他架构上运行时,由于文件不存在会直接报错返回。
    建议:既然已经通过 DConfig (idleStatePath / idleScreenStatePath) 配置了路径,就不应该在代码中硬编码特定架构的默认值。默认值可以设为空,或者在 DConfig 的 JSON 配置中根据架构分发不同的默认值,Go代码中仅做判空保护即可。

  • 冗余的第三方应用判断逻辑

    if strings.Contains(desktop, "deepin") || strings.Contains(desktop, "dde") || strings.Contains(desktop, "uos") {
        logger.Warning("Need add systemApplicationsMap, Running app : ", app, desktop)
        continue
    }

    问题:这种基于字符串包含关系的判断非常脆弱。如果第三方应用名字叫 "my-deepin-app.desktop",就会被误判为系统应用。
    建议:彻底移除这种启发式判断。系统应用白名单应该在 DConfig 中维护完整,而不是在代码中做模糊匹配。

  • interfaceToArrayString 的健壮性
    DConfig 返回数组时,底层类型往往是 []dbus.Variant,该函数对此做了处理,这是好的。但返回的 d 在未命中分支时为 nil,调用方需要做好 nil 判断。

4. 代码安全

  • Sysfs 文件写入权限过大

    err = ioutil.WriteFile(file, []byte(newContent), 0644)

    问题:对于 /sys/devices/system/... 下的内核参数节点,给予 0644 权限(即其他用户可读)可能存在信息泄露风险,而内核节点通常只允许 root 写入。
    建议:由于该 Daemon 是以 root 权限运行的,写入时无需关心文件的权限位(内核 sysfs 节点的权限由内核 sysfs_attrs 决定,用户态传入的 mode 往往被忽略)。但为了代码语义严谨,建议改为 06000200

  • TOCTOU (Time-of-Check to Time-of-Use) 竞态问题

    if !utils.IsFileExist(file) { ... }
    content, err := ioutil.ReadFile(file)

    问题:在检查文件存在和读取文件之间,文件可能被删除或替换(符号链接攻击),导致程序行为异常。
    建议:直接调用 os.ReadFile,如果文件不存在会返回错误,通过判断 os.IsNotExist(err) 来处理即可,无需提前 IsFileExist

  • 并发安全隐患
    Manager.ShortIdleStatesystem/power1/manager.go 中被多个方法读写,且在 setShortIdleState 的异步 goroutine 中也有读取:

    go func() {
        // ...
        if m.ShortIdleState { ... } // 异步读取
    }()

    问题ShortIdleState 字段缺乏互斥锁保护,在并发读写时可能产生数据竞争。
    建议:为涉及异步读写的状态字段增加 sync.Mutex 保护,或者使用 atomic.Bool 来替代:

    // 定义
    shortIdleState atomic.Bool
    
    // 读取
    if m.shortIdleState.Load() { ... }
    
    // 写入
    m.shortIdleState.Store(state)

总结与核心修改建议

  1. 移除 dbus.MakeVariant,直接传递基本类型给 DBus Call。
  2. 替换 ioutilos.ReadFile / os.WriteFile,并移除 IsFileExist 的提前检查。
  3. 修复 Map 清空逻辑ShortIdleState 获取失败后的判断逻辑
  4. 复用 DBus 连接,避免在关键路径上频繁建连。
  5. 增加并发安全保护,对 ShortIdleState 使用 atomic 或加锁。
  6. 移除硬编码的 loongarch 路径模糊字符串匹配逻辑,依赖 DConfig 和精确白名单。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants