feat: add short idle and TLP mode support#1111
Conversation
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
There was a problem hiding this comment.
Sorry @xionglinlin, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
deepin pr auto review这份代码引入了一个“短空闲”电源管理机制,涉及跨进程(session/power1, system/power1, dde-system-daemon)的DBus通信、sysfs文件写入、dconfig配置读取以及第三方应用检测等功能。整体逻辑较为复杂,且涉及系统级权限操作。 以下是针对语法逻辑、代码质量、代码性能和代码安全四个方面的详细审查意见及改进建议: 一、 语法与逻辑
二、 代码质量
三、 代码性能
四、 代码安全
|
mhduiy
left a comment
There was a problem hiding this comment.
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 并发读写竞态
systemApplicationsMap 和 shortIdleBlackListApplicationsMap 在以下场景存在竞态:
initDsgConfig的ValueChanged回调中写 map(在 signalLoop goroutine 中)isThirdPartyAppRunning中读 map(在 metaTask 执行中)- 没有任何 mutex 保护,可能触发
concurrent map read and map writepanic。
建议:使用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.ReadFile 和 os.WriteFile(ioutil 自 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 - setState 的 shortIdleState 零值问题
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 - setTlpMode 和 setShortIdleState 中 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", |
| "dde-file-manager.desktop", | ||
| "dde-computer.desktop", | ||
| "dde-trash.desktop", | ||
| "dde-control-center.desktop", |
| "deepin-diskmanager.desktop", | ||
| "deepin-camera.desktop", | ||
| "deepin-data-transfer.desktop", | ||
| "deepin-ab-recovery.desktop", |
| "gnome-keyring-ssh.desktop", | ||
| "gnome-keyring-pkcs11.desktop", | ||
| "gnome-keyring-secrets.desktop", | ||
| "fcitx-helper.desktop", |
| "gnome-keyring-secrets.desktop", | ||
| "fcitx-helper.desktop", | ||
| "xdg-user-dirs.desktop", | ||
| "permission_manager_dbus_session_daemon.desktop", |
|
TAG Bot New tag: 6.1.90 |
Log: Added short idle state management and TLP mode configuration for power saving optimization
Influence:
feat: 添加短idle和TLP模式支持
Log: 新增短idle状态管理和TLP模式配置,优化节能策略
Influence:
PMS: TASK-389737
Change-Id: Ia3572bf438ff45f8c67e7f354f991fd6535f7775