Skip to content

feat: add short idle and TLP mode support#1109

Open
xionglinlin wants to merge 1 commit into
linuxdeepin:masterfrom
xionglinlin:master
Open

feat: add short idle and TLP mode support#1109
xionglinlin wants to merge 1 commit into
linuxdeepin:masterfrom
xionglinlin:master

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

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

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份代码主要是为了在 LoongArch 架构的设备上引入“短 Idle (Short Idle)”的电源管理机制,涉及系统守护进程(写内核节点)、会话电源管理(定时器、黑名单应用检测、状态同步)以及 DConfig 配置项的读取与同步。

整体来看,代码逻辑较为清晰,但存在一些安全隐患、性能问题、逻辑漏洞及代码规范上的改进空间。以下是详细的审查意见:

一、 代码安全

  1. 内核文件路径注入风险
    bin/dde-system-daemon/power.go 中,d.idleStatePathd.idleScreenStatePath 是通过 DConfig 动态获取的。虽然 DConfig 默认是 readonlyprivate,但一旦被篡改(例如通过恶意修改本地 json 文件),setState 函数将会把数据写入攻击者指定的任意系统文件(如 /etc/passwd 或其他内核参数文件)。
    改进意见:在 setState 写入文件前,必须对路径进行严格的白名单校验,确保路径只能以 /sys/devices/system/loongarch/ 开头。

    func isValidSysPath(path string) bool {
        return strings.HasPrefix(path, "/sys/devices/system/loongarch/")
    }
    
    // 在 setState 开头加入校验
    if !isValidSysPath(file) {
        return fmt.Errorf("invalid sysfs path: %s", file)
    }
  2. 文件权限问题
    ioutil.WriteFile(file, []byte(newContent), 0644) 中使用了 0644 权限。对于 sysfs 内核节点,通常不需要可读权限给其他用户,建议使用 06000444(根据内核节点要求),以避免信息泄露或被低权限用户误改。

  3. 硬编码系统总线调用
    keybinding1/utils.gosession/power1/power_save_plan.go 中硬编码了 DBus 服务名和路径:
    systemConn.Object("org.deepin.dde.Daemon1", "/org/deepin/dde/Daemon1")
    如果服务名或路径未来发生变更,容易漏改。建议提取为常量。

二、 代码性能

  1. 频繁且重复的 DBus 通信
    session/power1/power_save_plan.goisThirdPartyAppRunning 中,每次判断都要通过 getLaunchedApplications() 进行 DBus 同步调用获取当前运行的应用。而在 changeShortIdleState 中,进入和退出短 Idle 都会调用此检查。
    如果系统频繁触发 Idle 状态切换,这会导致大量的 DBus IPC 开销。
    改进意见:建议监听 org.desktopspec.ApplicationManager1 的信号,在内存中维护一个当前运行应用的缓存列表,而不是每次都主动去 Get。

  2. time.Sleep 阻塞协程
    changeShortIdleState 中使用了 time.Sleep(300 * time.Millisecond)。如果该函数是在主循环或关键异步任务中被调用,阻塞 300ms 是不可接受的。
    改进意见:如果是等待 DConfig 异步生效,应该监听 DConfig 的 ValueChanged 信号后再触发 callSetIdleState,而不是盲目 Sleep。如果必须延时,应使用 time.AfterFunc 非阻塞方式。

  3. Map 的误用与内存浪费
    session/power1/manager.go 中,systemApplicationsMapshortIdleBlackListApplicationsMap 被定义为 map[string]string,但代码中只用来判断 key 是否存在,value 根本没有使用。
    改进意见:应改为 map[string]struct{},空结构体不占用内存,且语义上更符合“集合”的概念。

  4. isStrInList 效率低下
    bin/dde-system-daemon/power.go 中的 isStrInList$O(N)$ 复杂度。虽然目前 list 可能不长,但 Go 中通常有更优雅的写法。
    改进意见:可以直接使用 sort.SearchStrings 或在初始化时转为 map。

三、 语法与逻辑

  1. 严重的 Map 初始化逻辑错误
    session/power1/power_save_plan.goinitDsgConfig 中:

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

    Bug:这段代码的意思是“如果 Map 长度不为 0,才重新分配内存”。这会导致第一次初始化时(len == 0)不会执行 make,如果 map 在 newManager 中没有正确初始化,会引发 panic;而后续更新时,旧的 map 没有被清空,只是新建了一个 map 替换了引用,可能引发内存泄漏。
    修改为

    psp.manager.systemApplicationsMap = make(map[string]struct{})
    for _, app := range dsgSystemApplications {
        psp.manager.systemApplicationsMap[strings.ToLower(psp.getDesktopName(app))] = struct{}{}
    }
  2. setState 中的逻辑缺陷
    bin/dde-system-daemon/power.gosetState 中:

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

    Bug:如果 Get(0) 发生错误,shortIdleState 会是零值,程序不会 return,会继续往下执行,导致在获取状态失败的情况下依然写入文件。
    改进意见:获取失败时应该直接返回错误:

    shortIdleState, err := d.systemPower.ShortIdleState().Get(0)
    if err != nil {
        return fmt.Errorf("failed to get current shortIdleState: %v", err)
    }
  3. 未使用的变量
    bin/dde-system-daemon/main.goDaemon 结构体新增了 idleScreenStatePath,并在 getDsgValue 中赋值,但在 setState 中只检查了 file == d.idleStatePathidleScreenStatePath 没有被用于任何特殊逻辑判断,显得冗余。

  4. DBus 方法参数类型不匹配
    keybinding1/utils.gopower_save_plan.go 调用 DBus 方法时:
    Call("org.deepin.dde.Daemon1.SetScreenState", 0, dbus.MakeVariant(state))
    但在 exported_methods_auto.go 定义中,InArgs: []string{"state"},通常底层生成的代码期望的是基本类型(如 bool),包装成 dbus.Variant 可能会导致 DBus 类型签名不匹配(v 而非 b),引发调用失败。
    改进意见:确认 dbusutil-gen 生成的代码是否接受 Variant。通常直接传 state 即可:.Call("...", 0, state)

  5. 第三方应用判断的“硬伤”

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

    通过字符串包含关系判断是否为系统应用是非常不可靠的。如果第三方应用名字叫 deepin-music-player-unofficial.desktop,就会被误判为系统应用。
    改进意见:去除这个 Hack 逻辑,完全依赖 DConfig 中的 systemApplications 白名单即可。如果白名单不全,应该更新 DConfig 配置,而不是在代码里猜测。

四、 代码质量与规范

  1. 废弃的 API 使用
    ioutil.ReadFileioutil.WriteFile 在 Go 1.16 之后已被标记为废弃,建议分别替换为 os.ReadFileos.WriteFile

  2. syscall.Sync() 的滥用
    setState 写入 sysfs 节点后调用了 syscall.Sync()Sync() 会将整个系统的脏页刷回磁盘,这是一个非常昂贵的全局阻塞操作。对于 sysfs 虚拟文件系统,写入通常直接反映到内核态,不需要也不应该触发全局 Sync。
    改进意见:删除 syscall.Sync()。如果确实需要确保数据落盘(对 sysfs 无意义),应使用 file.Sync()

  3. 重复代码
    callSetIdleStatecallSetScreenState 逻辑几乎完全一致,仅调用的 DBus Method 名字不同。建议抽象为一个通用函数:

    func callDaemonMethod(method string, state bool) {
        systemConn, err := dbus.SystemBus()
        if err != nil {
            logger.Errorf("Failed to get system bus: %v", err)
            return
        }
        err = systemConn.Object("org.deepin.dde.Daemon1", "/org/deepin/dde/Daemon1").Call("org.deepin.dde.Daemon1." + method, 0, state).Err
        if err != nil {
            logger.Warning(err)
        }
    }
  4. 注释与拼写

    • stopNetworkDisaplay 应为 stopNetworkDisplay
    • systemApplicationsMap 赋值时的日志 []string -> map 可以去掉,数据量太大容易冲刷日志。

总结建议

这是一个比较大的 Feature 合入,涉及跨进程(Session <-> System Daemon <-> Kernel)的状态同步。建议重点关注内核路径的校验Map 初始化的 Bug以及DBus 参数类型的匹配。在合入前,最好能模拟 DConfig 被恶意篡改、DBus通信断开、应用频繁启停等异常场景进行边界测试。

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.

2 participants