Skip to content

[暂不合并]chore(xsettings): add ExecStartPre to disable THP before service startup#69

Open
mhduiy wants to merge 1 commit into
linuxdeepin:masterfrom
mhduiy:thp
Open

[暂不合并]chore(xsettings): add ExecStartPre to disable THP before service startup#69
mhduiy wants to merge 1 commit into
linuxdeepin:masterfrom
mhduiy:thp

Conversation

@mhduiy
Copy link
Copy Markdown
Contributor

@mhduiy mhduiy commented May 25, 2026

  1. Added ExecStartPre=-/usr/libexec/dde-thp-disable to XSettings1 D-Bus service
  2. Ensures touchpad palm rejection (THP) is disabled before the XSettings service starts

Log: Run dde-thp-disable as ExecStartPre in XSettings1 D-Bus service

chore(xsettings): 在 XSettings 服务启动前添加 ExecStartPre 禁用 THP

  1. 在 XSettings1 D-Bus service 中添加了 ExecStartPre=-/usr/libexec/dde-thp-disable
  2. 确保在 XSettings 服务启动前禁用触摸板手掌防误触(THP)

Log: 在 XSettings1 D-Bus 服务中添加 dde-thp-disable 作为 ExecStartPre
PMS: TASK-390043

Summary by Sourcery

Enhancements:

  • Add a pre-start command to the XSettings1 D-Bus service to run dde-thp-disable before service startup.

1. Added ExecStartPre=-/usr/libexec/dde-thp-disable to XSettings1 D-Bus service
2. Ensures touchpad palm rejection (THP) is disabled before the XSettings service starts

Log: Run dde-thp-disable as ExecStartPre in XSettings1 D-Bus service

chore(xsettings): 在 XSettings 服务启动前添加 ExecStartPre 禁用 THP

1. 在 XSettings1 D-Bus service 中添加了 ExecStartPre=-/usr/libexec/dde-thp-disable
2. 确保在 XSettings 服务启动前禁用触摸板手掌防误触(THP)

Log: 在 XSettings1 D-Bus 服务中添加 dde-thp-disable 作为 ExecStartPre
PMS: TASK-390043
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhduiy

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

@mhduiy
Copy link
Copy Markdown
Contributor Author

mhduiy commented May 25, 2026

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 25, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds a pre-start step to the XSettings1 D-Bus service to run a helper that disables touchpad palm rejection (THP) before the service starts.

File-Level Changes

Change Details Files
Run dde-thp-disable as an ExecStartPre command for the XSettings1 D-Bus service so THP is disabled before XSettings starts.
  • Add ExecStartPre=-/usr/libexec/dde-thp-disable to the XSettings1 D-Bus service unit configuration
  • Ensure the pre-start helper is optional (failure-tolerant) by using the '-' prefix in ExecStartPre
src/plugin-qt/xsettings/misc/org.deepin.dde.XSettings1.service

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的 Git Diff 内容。

本次修改在 systemd 服务单元文件 org.deepin.dde.XSettings1.service[Service] 段中新增了一行 ExecStartPre=-/usr/libexec/dde-thp-disable,即在主服务启动前执行一个名为 dde-thp-disable 的脚本或程序,并且使用了前缀 - 忽略其执行错误。

以下是我从语法逻辑、代码质量、代码性能和代码安全四个维度提出的审查意见和改进建议:

1. 语法逻辑

  • - 前缀的使用:在 systemd 中,ExecStartPre 前的 - 表示如果该命令执行失败(如返回非零退出码或被信号杀死),systemd 会忽略该错误并继续执行后续的 ExecStart。这对于可选的或环境依赖的预处理操作是合理的。
  • 潜在逻辑风险:忽略错误意味着即使 dde-thp-disable 未能成功禁用 THP(透明大页),XSettings1 服务仍会正常启动。你需要确认这是否符合预期。如果 THP 未禁用会导致 XSettings1 或其关联的图形服务发生严重故障(如内存碎片化导致的延迟、死锁等),那么静默忽略这个失败可能会让问题更难排查。

2. 代码质量

  • 可维护性与注释:systemd 配置文件通常缺乏上下文,对于后续维护者来说,可能不清楚为什么要禁用 THP。
    • 建议:在提交信息中详细说明为什么需要禁用 THP,如果可能,在服务文件上方或内部增加简短注释(systemd 支持以 # 开头的注释)。
    • 示例
      # Disable Transparent Hugepage (THP) to prevent UI latency issues in Qt environment
      ExecStartPre=-/usr/libexec/dde-thp-disable
  • 硬编码路径/usr/libexec/ 是一个常见的 FHS (文件系统层次结构标准) 路径,但在某些发行版中,可能会使用 /usr/lib//usr/lib64/。确保该路径在所有目标发行版中都是一致的。

3. 代码性能

  • 启动延迟ExecStartPre 是同步执行的,systemd 会等待该进程结束后才启动主服务。如果 dde-thp-disable 脚本执行缓慢(例如,它不仅修改内核参数,还做了耗时的检查或重载操作),将会直接拖慢 XSettings1 服务的启动时间,进而影响用户桌面的加载速度。
    • 建议:确保 dde-thp-disable 的逻辑极其轻量,例如仅仅是向 /sys/kernel/mm/transparent_hugepage/enabled/proc/sys/vm/... 写入一个字符串,避免在其中执行复杂的网络请求或磁盘 IO。

4. 代码安全

这是本次变更中最需要关注的维度:

  • 权限提升与文件权限:systemd 的 [Service] 段默认以 root 用户运行(除非配置了 User=)。修改内核参数(如 THP 状态)需要 root 权限,因此以 root 运行没有问题。但是,必须确保 /usr/libexec/dde-thp-disable 这个文件的权限被严格设置为仅 root 可写(如 07550744),否则恶意用户可以替换该脚本,从而在系统服务启动时获得 root 权限的代码执行(提权漏洞)。
  • 脚本内容的安全性:如果 dde-thp-disable 是一个 shell 脚本,必须确保它不包含任何相对路径的命令调用,防止 PATH 劫持;且不应处理不可信的输入。
  • 安全替代方案(强烈建议评估)
    通过 systemd 服务在运行时修改内核运行时参数(如 THP)是一种侵入性较强的做法,且属于全局内核状态修改,会影响整个系统而不仅仅是 XSettings1。
    更安全、更符合 Linux 标准做法的方式是:
    1. 使用 systemd 临时配置:通过 systemd-sysctl/etc/sysctl.d/ 来全局配置。
    2. 使用 systemd-tmpfiles:通过 /etc/tmpfiles.d/ 在系统启动时设置。
    3. 内核启动参数:在 GRUB 中添加 transparent_hugepage=never
      如果确实只需要在桌面环境启动时禁用,当前方案可以接受,但请务必确保该可执行文件的安全边界不被打破。

总结与改进建议

改进后的代码示例:

[Service]
Type=dbus
BusName=org.deepin.dde.XSettings1
# Workaround: Disable THP before starting to avoid Qt rendering latency/stalls
ExecStartPre=-/usr/libexec/dde-thp-disable
ExecStart=/usr/bin/deepin-service-manager -n org.deepin.dde.XSettings1
Restart=on-failure
RestartSec=3

Checklist(请确认):

  1. dde-thp-disable 执行是否足够快?(防止影响启动性能)
  2. dde-thp-disable 失败被忽略是否绝对安全?(逻辑一致性)
  3. /usr/libexec/dde-thp-disable 的文件权限是否为 root:root 755?(安全性)
  4. 是否有更好的系统级配置替代在特定服务中执行此操作?(架构合理性)

@mhduiy mhduiy changed the title chore(xsettings): add ExecStartPre to disable THP before service startup [暂不合并]chore(xsettings): add ExecStartPre to disable THP before service startup May 25, 2026
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