HID: hid-oxp: fix and extend X2-family controller support - #13
HID: hid-oxp: fix and extend X2-family controller support#13Grippy98 wants to merge 517 commits into
Conversation
pastaq
left a comment
There was a problem hiding this comment.
Minor nit for patch 2
With that addressed for v2,
Reviewed-by: Derek J. Clark [email protected]
| { | ||
| u8 page_1[59] = { 0x02, 0x38, 0x20, 0x01, 0x01 }; | ||
| u8 page_2[59] = { 0x02, 0x38, 0x20, 0x02, 0x01 }; | ||
| u8 page_1[59] = { 0x02, 0x38, OXP_BMAP_FORMAT_DEFAULT, 0x01, 0x01 }; |
There was a problem hiding this comment.
Addressed in 0950245e1f4e. oxp_set_buttons() initializes all three pages with cfg->bmap_format.
|
|
||
| oxp_page_fill_data(page_1, p1, ARRAY_SIZE(p1)); | ||
| oxp_page_fill_data(page_2, p2, ARRAY_SIZE(p2)); | ||
| page_1[2] = drvdata.bmap_format; |
| }; | ||
|
|
||
| oxp_page_fill_data(page_1, p1, ARRAY_SIZE(p1)); | ||
| oxp_page_fill_data(page_2, p2, ARRAY_SIZE(p2)); |
There was a problem hiding this comment.
So we not need to add/fill page_3 here for some reason?
There was a problem hiding this comment.
It uses the fixed factory M3/M4 encodings because the buttons do not have mutable sysfs slots or mapping-table entries like pages 1 and 2. Added a comment to document it
There was a problem hiding this comment.
For patch 1, please add a fixes tag when up-streaming/
Reviewed-by: Derek J. Clark [email protected]
pastaq
left a comment
There was a problem hiding this comment.
There are a lot of comments for patch 3 but overall it isn't bad. The summary is
- Make each individual fix for an issue a fixes patch. use a fixes tag with the sha of the patch that added the feature.
- Instead of branching in gen 2, identify gen 3 early and make new paths, leaving as much existing code in place. This is to avoid regressions in hardware that is difficult to get tested.
| struct oxp_gen_1_rgb_report *rgb_rep; | ||
|
|
||
| if (size < sizeof(*rgb_rep) || !led_mc) | ||
| return 0; |
There was a problem hiding this comment.
Please put general fixes in thier own separate preceding patches so they can be backported to stable kernels. One patch per fix.
| led_mc->led_cdev.brightness = rgb_rep->brightness / 4 * | ||
| led_mc->led_cdev.brightness = rgb_rep->brightness * | ||
| led_mc->led_cdev.max_brightness; | ||
| /* If monocolor had less than 100% brightness on the previous boot, |
| drvdata.rgb_en = rgb_rep->enabled == 0 ? OXP_FEAT_DISABLED : | ||
| OXP_FEAT_ENABLED; | ||
| drvdata.rgb_brightness = rgb_rep->brightness; | ||
| led_mc->led_cdev.brightness = rgb_rep->brightness / 4 * |
There was a problem hiding this comment.
Moved into the standalone fix d6fe231751c2
| static int oxp_gen_2_property_out(enum oxp_function_index fid, u8 *data, u8 data_size); | ||
| static int oxp_set_buttons(void); | ||
| static int oxp_rumble_intensity_set(u8 intensity); | ||
| static int oxp_rgb_status_store(u8 enabled, u8 speed, u8 brightness); |
There was a problem hiding this comment.
Please make the restore on resume a fix patch as well
There was a problem hiding this comment.
Done in a3092d79d317 . RGB cache replay is in 1cf1e8b383b3 and X2/OXP3 only since it's not verified on other devices yet (potential for improvement once tested)
| /* Remainder only applies for xinput mode */ | ||
| if (drvdata.gamepad_mode == OXP_GP_MODE_DEBUG) | ||
| if (drvdata.gamepad_mode == OXP_GP_MODE_DEBUG) { | ||
| oxp_rgb_restore(); |
There was a problem hiding this comment.
Is this possible to do once prior to this early return block? I don't remember if the xinput mode needs to be set first.
There was a problem hiding this comment.
Addressed in 1cf1e8b383b3. oxp_mcu_init_fn() selects the final debug/XInput mode, restores RGB once and then takes the debug-mode early return or applies XInput rumble.
| guard(mutex)(&drvdata.rgb_mutex); | ||
| old_effect = drvdata.rgb_effect; | ||
|
|
||
| if (!drvdata.x2_rgb) { |
There was a problem hiding this comment.
I'd prefer to fix up oxp_rgb_status_store to branch instead of branching here. Is there a technical reason you did it here?
| { | ||
| .color_index = LED_COLOR_ID_RED, | ||
| .intensity = 0x24, | ||
| .max_intensity = 0xff, |
| drvdata.gen2_work_initialized = false; | ||
| err_cancel_rgb: | ||
| WRITE_ONCE(drvdata.removing, true); | ||
| spin_lock_irqsave(&oxp_rgb_reply_lock, flags); |
| drvdata.led_mc = &oxp_cdev_rgb; | ||
|
|
||
| INIT_DELAYED_WORK(&drvdata.oxp_rgb_queue, oxp_rgb_queue_fn); | ||
| drvdata.rgb_work_initialized = true; |
| bool removing; | ||
| } drvdata; | ||
|
|
||
| static DEFINE_SPINLOCK(oxp_rgb_reply_lock); |
There was a problem hiding this comment.
In the future I'll be making drvdata unique per init. Please make this a member of drvdata to ease that transition.
pastaq
left a comment
There was a problem hiding this comment.
For patch 4, I'd like to break it into 2 patches. The first will refactor the existing RGB to use a common LED array for all LEDS, so that they are all handled the same way. Use a void pointer for the state and a new type enum for the switch case for casting. The second patch can then expand on the new format to add the aux zones
| @@ -0,0 +1,39 @@ | |||
| What: /sys/class/leds/oxp:rgb:guide_button/effect | |||
There was a problem hiding this comment.
We can combine these I think.
/sys/class/leds/oxp:rgb:[joystick_rings|guide_button|rear_logo]/effect
Description would need to be more descriptive of existing effects.
| struct oxp_button_idx btn_m2; | ||
| } __packed; | ||
|
|
||
| enum oxp_x2_aux_effect { |
There was a problem hiding this comment.
Please move these to live after the existing rgb structs/enums
| bool valid; | ||
| }; | ||
|
|
||
| struct oxp_x2_aux_led { |
There was a problem hiding this comment.
Why are we using a different pattern for this? A separate __packed rgb report would make more sense to me to keep things consistent. If this struct is necessary we should refactor existing RGB to use it and provide state as a void pointer we can cast for member access.
|
|
||
| static const u8 oxp_x2_rgb_zones[] = { 0x01, 0x02, 0x07 }; | ||
|
|
||
| #define OXP_GEN2_RGB_DATA_SIZE 59 |
There was a problem hiding this comment.
Please keep all defines at the top of the file
| int ret; | ||
| int attempt; | ||
|
|
||
| for (attempt = 0; attempt < 2; attempt++) { |
There was a problem hiding this comment.
Why are we looping here? This isn't iterating on an index.
| if (ret || !brightness) | ||
| return ret; | ||
|
|
||
| red = ((unsigned int)state->red * brightness + max_brightness / 2) / |
| return container_of(mc_cdev, struct oxp_x2_aux_led, mc_cdev); | ||
| } | ||
|
|
||
| static const char *const oxp_x2_aux_effect_text[] = { |
There was a problem hiding this comment.
Please keep this directly under the enum
| .subled_info = oxp_rgb_subled_info, | ||
| }; | ||
|
|
||
| struct oxp_x2_aux_led_desc { |
|
|
||
| struct oxp_x2_aux_led_desc { | ||
| const char *name; | ||
| u8 zone; |
There was a problem hiding this comment.
Add a type enum (RGB_FULL/RGB_AUX) u8 here so we can match on it and reuse the same structs and functions.
There was a problem hiding this comment.
If we're adding docs (something I missed) we should include the rest of the ABI that was left out as well
Previously, the per-VMA locking could fail in the face of writers which necessitates a fallback to mmap_lock. The new vma_start_read_unlocked() will wait for writers instead of failing. Use the new helper. Wait for writers. Remove the fallback to mmap_lock. The fallback removal does not affect NOMMU case because TCP_ZEROCOPY is gated on CONFIG_MMU. This really is a nice cleanup. It removes the need to pass the lock state back and forth to find_tcp_vma(). Link: https://lore.kernel.org/[email protected] Signed-off-by: Dave Hansen <[email protected]> Signed-off-by: Suren Baghdasaryan <[email protected]> Acked-by: Lorenzo Stoakes <[email protected]> Acked-by: Vlastimil Babka (SUSE) <[email protected]> Tested-by: [email protected] Cc: Liam R. Howlett <[email protected]> Cc: Vlastimil Babka <[email protected]> Cc: Shakeel Butt <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Cc: Arve Hjønnevåg <[email protected]> Cc: Todd Kjos <[email protected]> Cc: Christian Brauner <[email protected]> Cc: Carlos Llamas <[email protected]> Cc: Alice Ryhl <[email protected]> Cc: David S. Miller <[email protected]> Cc: David Ahern <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Charges that exceed memory.max and return through the nomem label can raise no event and simply return -ENOMEM. A non-blocking charge can hit the limit, get rejected, but is not visible in memory.events. This was noticed in a production setting where bpf_mem_alloc() attempted to refill its per-cpu freelists, which triggered a non-blocking charge while at the limit. Commit d6e103a ("mm: memcontrol: do not miss MEMCG_MAX events for enforced allocations") added raised_max_event to cover charges that are force charged without ever reaching reclaim, but charges that are rejected outright were left out. Getting an allocation failure without the corresponding MEMCG_MAX event is unexpected and makes debugging and monitoring harder. Raise the event on the way out for rejected charges as well, by routing the -ENOMEM return through the same exit path that already covers forced charges. The existing behavior of raising a MEMCG_MAX event on every charge/reclaim/retry iteration is left unchanged. Tested with a module that performs accounted GFP_NOWAIT page allocations from a task in a cgroup at its memory.max, and measures the resulting memory.events:max delta. Without this patch the rejected charges raise no event at all; with it the delta matches the number of rejected charges exactly. A GFP_KERNEL|__GFP_NORETRY control, which reaches reclaim, raises the same two events per failed charge before and after, confirming the existing charge/reclaim/retry accounting is unchanged. Link: https://lore.kernel.org/[email protected] Fixes: d6e103a ("mm: memcontrol: do not miss MEMCG_MAX events for enforced allocations") Signed-off-by: Joe Damato <[email protected]> Suggested-by: Shakeel Butt <[email protected]> Acked-by: Shakeel Butt <[email protected]> Cc: Johannes Weiner <[email protected]> Cc: Michal Hocko <[email protected]> Cc: Muchun Song <[email protected]> Cc: Roman Gushchin <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Patch series "mm/damon: add kunit tests for probe_hits handling and probe params validation", v2. DAMON recently introduced probes and probe weights. Add kunit tests for the propagation of probe_hits at region split and merge, and the rejection of invalid probe parameters by damon_valid_probe_params(). This patch (of 2): damon_split_region_at() copies probe_hits[] and last_probe_hits[] to the new split region. damon_merge_two_regions() sets probe_hits[] to the size-weighted average of the merged regions. Extend damon_test_split_at() and damon_test_merge_two() tests to cover those fields. Link: https://lore.kernel.org/[email protected] Link: https://lore.kernel.org/[email protected] Signed-off-by: Jason Angelov <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: David Gow <[email protected]> Cc: Brendan Higgins <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
damon_valid_probe_params() makes damon_commit_ctx() reject probe configurations that could overflow a probe_hits counter, a single (weight * probe_hits) product, or the sum of those products. Add a kunit test covering each rejection at its boundary: - samples per aggregation interval: U8_MAX is allowed, one more could overflow a probe_hits counter - single weight: the largest whose product fits in unsigned int is allowed, one larger is rejected - multiple probes: each product fits, but their sum overflows - no weight set: the validation is skipped Link: https://lore.kernel.org/[email protected] Signed-off-by: Jason Angelov <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: Brendan Higgins <[email protected]> Cc: David Gow <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Patch series "docs/mm/damon/design: add explanation of nr_snapshots", v3. Add an explanation of nr_snapshots to avoid misunderstandings. This patch (of 3): Change "tried to be applied" -> "completely tried to be applied" to maintain consistency between the documentation and the code. Link: https://lore.kernel.org/[email protected] Link: https://lore.kernel.org/[email protected] Signed-off-by: Liew Rui Yan <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: David Hildenbrand <[email protected]> Cc: Jonathan Corbet <[email protected]> Cc: Liam R. Howlett <[email protected]> Cc: Lorenzo Stoakes <[email protected]> Cc: Michal Hocko <[email protected]> Cc: Mike Rapoport <[email protected]> Cc: Randy Dunlap <[email protected]> Cc: Suren Baghdasaryan <[email protected]> Cc: Vlastimil Babka <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Explain the difference between nr_snapshots reaches max_nr_snapshots and watermarks. Link: https://lore.kernel.org/[email protected] Signed-off-by: Liew Rui Yan <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: David Hildenbrand <[email protected]> Cc: Jonathan Corbet <[email protected]> Cc: Liam R. Howlett <[email protected]> Cc: Lorenzo Stoakes <[email protected]> Cc: Michal Hocko <[email protected]> Cc: Mike Rapoport <[email protected]> Cc: Randy Dunlap <[email protected]> Cc: Suren Baghdasaryan <[email protected]> Cc: Vlastimil Babka <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Fix a typo (nr_snapshots -> max_nr_snapshots) and corrects a grammar error. Link: https://lore.kernel.org/[email protected] Signed-off-by: Liew Rui Yan <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: David Hildenbrand <[email protected]> Cc: Jonathan Corbet <[email protected]> Cc: Liam R. Howlett <[email protected]> Cc: Lorenzo Stoakes <[email protected]> Cc: Michal Hocko <[email protected]> Cc: Mike Rapoport <[email protected]> Cc: Randy Dunlap <[email protected]> Cc: Suren Baghdasaryan <[email protected]> Cc: Vlastimil Babka <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Patch series "mm/damon/core: remove unused helper functions", v2. Both damon_targets_empty() and damon_nr_running_ctxs() have had no in-tree users since commit 5ec4333 ("mm/damon: remove DAMON debugfs interface") removed their remaining callers. Remove the unused declarations and definitions. This patch (of 2): damon_targets_empty() has had no in-tree users since commit 5ec4333 ("mm/damon: remove DAMON debugfs interface") removed its last caller. Remove the unused declaration and definition. Link: https://lore.kernel.org/[email protected] Link: https://lore.kernel.org/[email protected] Signed-off-by: Cheng-Han Wu <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
damon_nr_running_ctxs() has had no in-tree users since commit 5ec4333 ("mm/damon: remove DAMON debugfs interface") removed all of its callers. Remove the unused declaration and definition. Link: https://lore.kernel.org/[email protected] Signed-off-by: Cheng-Han Wu <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Patch series "mm/damon: Introduce a huge page collapsing mechanism using auto tuning", v4. Overview ======== This patchset introduces a new autotuning which allows to collapse hot regions into hugepages. Motivation ========== Since TLB is a bottleneck for many systems[1], a way to optimize TLB misses (or hits) is to use huge pages. Unfortunately, using "always" in THP leads to memory fragmentation and memory waste. For this reason, most application guides and system administrators suggest to disable THP. Selective huge page collapse per process is possible using prctl and a launcher. However, this does not solve the issue with hot region detection. Additionally, it the sysadmin should create a launcher that uses PRCTL to enable THP for a particular process. We can use the DAMON support for DAMOS_HUGEPAGE and DAMOS_COLLAPSE, to target a certain process. DAMOS_COLLAPSE can also target the hot regions in that process. Still, there is an issue with the amount of huge page consumption. Since huge pages can lead to memory fragmentation and waste, there should be a way to limit the amount of huge page consumption. There is hugetlbfs, but it requires changes to the application code or the use of libhugetlbfs. DAMON has now a way to autotune some of the variables and adjust quotas automatically, so that DAMON is fired only under the right circumstances. It would be nice to have something similar, but for huge pages. Solution ======== A new autotuning quota goal[2], damos_hugepage_mem_bp, is introduced, which checks the huge page consumption to total memory consumption. This new quota mechanism reuses current autotuning architecture. In order to test this new mechanism, a sample module[3] was created, but not included in this patch series. To demonstrate the tool, damo user space tool was modified[4], which sets up huge pages collapse autotuning. Benchmarks ========== Setup: physical server with arm64 processor with 4 NUMA nodes, 1 TB RAM and running mariaDB 10.5.29. Sysbench was used for the benchmark, with 20 tables and 3 million rows per table. The database was pinned to one of the nodes, and the benchmark framework to a different node. No network traffic involved in the benchmark. Damo user space tool was forked and hugepage_mem_bp support added[4]. DAMON was lauched using this command line: sudo ./damo start $(pidof mariadbd) \ --monitoring_nr_regions_range 10 1000 \ --monitoring_intervals 5000 100000 60000000 \ --damos_quota_time 0 --damos_quota_space 128000000 \ --damos_quota_interval 1000 \ --damos_quota_weights 0 1 1 \ --damos_quota_goal hugepage_mem_bp <target> \ --damos_quota_goal_tuner temporal \ --damos_apply_interval 50000 \ --damos_access_rate 0 max --damos_age 50 max \ --damos_action collapse --debug_damon <target> was 1000 to taget 10% hugepage to total memory ratio, or 2500 to target 25%. Tuner was also tested with consistent and temporal. Results ======= After the last timestamp, there was no change in huge page use, and the total huge page to memory consumption ratio barely moved. hugepage_mem_bp: 1000 goal tuner: temporal +-----------+----------------+----------------+----------------------+ | timestamp | total mem used | huge page used | percentage hugepage | +-----------+----------------+----------------+----------------------+ | 0 | 16945.04297 | 0 | 0 | | 7 | 17008.69531 | 74 | 0.435071583 | | 8 | 17036.40234 | 194 | 1.138738074 | | 9 | 17017.01563 | 314 | 1.845211916 | | 10 | 17029.67969 | 434 | 2.548491856 | | 61 | 17111.30859 | 584 | 3.412947623 | | 120 | 17071.05859 | 694 | 4.065360072 | | 180 | 17133.88281 | 804 | 4.692456513 | | 203 | 17088.16406 | 916 | 5.360435426 | | 204 | 17126.34766 | 1046 | 6.107548562 | | 205 | 17093.84375 | 1176 | 6.879669764 | | 206 | 17142.77734 | 1298 | 7.571701913 | | 209 | 17149.17969 | 1686 | 9.831374041 | | 210 | 17097.30859 | 1754 | 10.25892462 | +-----------+----------------+----------------+----------------------+ hugepage_mem_bp: 1000 goal tuner: consistent +-----------+----------------+----------------+----------------------+ | timestamp | total mem used | huge page used | percentage hugepage | +-----------+----------------+----------------+----------------------+ | 0 | 16955.24609 | 0 | 0 | | 34 | 17039.71875 | 106 | 0.622075995 | | 78 | 17009.47656 | 554 | 3.257007927 | | 90 | 17048.92188 | 596 | 3.495822225 | | 150 | 17092.90625 | 706 | 4.130368409 | | 180 | 17053.08984 | 764 | 4.480126517 | | 233 | 17100.50391 | 1496 | 8.748280216 | | 239 | 17098.89063 | 2216 | 12.95990511 | | 240 | 17135.44531 | 2334 | 13.62088908 | | 245 | 17132.55078 | 2932 | 17.11362212 | | 246 | 17117.95313 | 3052 | 17.82923448 | | 250 | 17163.12109 | 3532 | 20.57900763 | +-----------+----------------+----------------+----------------------+ hugepage_mem_bp: 2500 goal tuner: temporal +-----------+----------------+----------------+----------------------+ | timestamp | total mem used | huge page used | percentage hugepage | +-----------+----------------+----------------+----------------------+ | 0 | 17010.31641 | 0 | 0 | | 9 | 17063.6875 | 50 | 0.2930199 | | 10 | 17051.75781 | 170 | 0.996964664 | | 60 | 17133.85547 | 572 | 3.338419663 | | 90 | 17192.07813 | 626 | 3.641211932 | | 120 | 17221.44531 | 682 | 3.960178647 | | 181 | 17199.76172 | 790 | 4.593086886 | | 208 | 17222.77734 | 1206 | 7.002354939 | | 214 | 17245.17969 | 1904 | 11.04076637 | | 215 | 17240.45703 | 2024 | 11.73982799 | | 220 | 17234.79688 | 2624 | 15.22501262 | | 228 | 17222.83594 | 3584 | 20.80958103 | | 231 | 17247.55469 | 3944 | 22.86700968 | | 235 | 17229.37109 | 4424 | 25.67708349 | +-----------+----------------+----------------+----------------------+ hugepage_mem_bp: 1000 goal tuner: consist +-----------+----------------+----------------+----------------------+ | timestamp | total mem used | huge page used | percentage hugepage | +-----------+----------------+----------------+----------------------+ | 0 | 17125.85156 | 0 | 0 | | 38 | 17081.23438 | 76 | 0.444932716 | | 39 | 17133.11719 | 196 | 1.143983304 | | 40 | 17119.83984 | 316 | 1.84581166 | | 60 | 17109.72656 | 554 | 3.237924335 | | 90 | 17164.11328 | 628 | 3.65879664 | | 180 | 17177.66016 | 792 | 4.610639591 | | 220 | 17180.86719 | 1378 | 8.020549749 | | 226 | 17187.82031 | 1980 | 11.51978531 | | 233 | 17143.48438 | 2818 | 16.4377319 | | 240 | 17137.38281 | 3656 | 21.33347921 | | 250 | 17175.5 | 4856 | 28.27283049 | | 260 | 17199.66406 | 6056 | 35.20999002 | | 270 | 17203.98438 | 7254 | 42.16465118 | | 275 | 17207.21875 | 7762 | 45.10897498 | +-----------+----------------+----------------+----------------------+ More detailed tables are provided here[5] From this, we can conclude that the huge page autotuner works fine, achieving the target. When using consistent autotuner, it actually over-achieves the target, which is expected, since quota esz_bp is not set to 0 to cap the DAMOS policy. Patches Sequence ================ Patch 1 -> Introduce DAMOS_QUOTA_HUGEPAGE_MEM_BP and autotuning Patch 2 -> sysfs support for the new quota goal Patch 3 -> Document hugepage_mem_bp parameter This patch (of 3): Introduce DAMOS_QUOTA_HUGEPAGE_MEM_BP auto tuning. Add a new DAMOS quota goal metric to measure the amount of huge page consumption to total memory consumption ratio. Vmstat may lag, which in some cases may lead to NR_FREE_PAGES being greater than or equal to the amount of RAM in the system. A guard is added to avoid the extremely unlikely case [6]. In the case, return 100% (10000 bp). Link: https://lore.kernel.org/[email protected] Link: https://lore.kernel.org/[email protected] Link: https://dl.acm.org/doi/pdf/10.1145/3307650.3322227 [1] Link: https://lore.kernel.org/[email protected] [2] Link: https://lore.kernel.org/[email protected] [3] Link: asierHuawei/damo@79ae1a4 [4] Link: https://lore.kernel.org/all/[email protected]/ [5] Link: https://lore.kernel.org/all/[email protected]/ [6] Signed-off-by: Asier Gutierrez <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: David Hildenbrand <[email protected]> Cc: Jonathan Corbet <[email protected]> Cc: Liam R. Howlett <[email protected]> Cc: Lorenzo Stoakes <[email protected]> Cc: Michal Hocko <[email protected]> Cc: Mike Rapoport <[email protected]> Cc: Randy Dunlap <[email protected]> Cc: Suren Baghdasaryan <[email protected]> Cc: Vlastimil Babka <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
DAMOS has a new autotune policy metric: DAMOS_QUOTA_HUGEPAGE_MEM_BP. This patch exposes DAMOS_QUOTA_HUGEPAGE_MEM_BP through sysfs. Add the "hugepage_mem_bp" to the sysfs-schemes interface. Link: https://lore.kernel.org/[email protected] Signed-off-by: Asier Gutierrez <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: David Hildenbrand <[email protected]> Cc: Jonathan Corbet <[email protected]> Cc: Liam R. Howlett <[email protected]> Cc: Lorenzo Stoakes <[email protected]> Cc: Michal Hocko <[email protected]> Cc: Mike Rapoport <[email protected]> Cc: Randy Dunlap <[email protected]> Cc: Suren Baghdasaryan <[email protected]> Cc: Vlastimil Babka <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Document hugepage_mem_bp metric exposed by sysfs. Link: https://lore.kernel.org/[email protected] Signed-off-by: Asier Gutierrez <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: David Hildenbrand <[email protected]> Cc: Jonathan Corbet <[email protected]> Cc: Liam R. Howlett <[email protected]> Cc: Lorenzo Stoakes <[email protected]> Cc: Michal Hocko <[email protected]> Cc: Mike Rapoport <[email protected]> Cc: Randy Dunlap <[email protected]> Cc: Suren Baghdasaryan <[email protected]> Cc: Vlastimil Babka <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Patch series "mm/damon: misc cleanups". Cleanup the code, tests and samples for clarifications and readability. The patches are individually sent by the authors. I'm reposting those as one series for convenience of handling. For this reason, changelog is on each patch's commentary section. This patch (of 7): __damon_commit_ctx() was added by commit b1471af ("mm/damon/core: do parameter testing commit on damon_start()") but is actually not needed. Remove it. Link: https://lore.kernel.org/[email protected] Link: https://lore.kernel.org/[email protected] Signed-off-by: Zenghui Yu (Huawei) <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Cc: Shuah Khan <[email protected]> Cc: Zenghui Yu <[email protected]> Cc: Enze Li <[email protected]> Cc: Hari Mishal <[email protected]> Cc: Jaeyeon Lee <[email protected]> Cc: Li Youhong <[email protected]> Cc: zhaozhengzhuo <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
The logic that finds the struct pid for a given pid number and assigns it to a damon_target is duplicated in multiple places. Including damon_sysfs_add_target() of mm/damon/sysfs.c and the start functions of the two sample modules, samples/damon/wsse.c and samples/damon/prcl.c. Add a function that does the work, and replace the duplicated code in the places with calls to the function. Link: https://lore.kernel.org/[email protected] Signed-off-by: Enze Li <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Cc: Hari Mishal <[email protected]> Cc: Jaeyeon Lee <[email protected]> Cc: Li Youhong <[email protected]> Cc: Shuah Khan <[email protected]> Cc: "Zenghui Yu (Huawei)" <[email protected]> Cc: zhaozhengzhuo <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
The putback loop is duplicated in damon_migrate_folio_list() and on the invalid-nid path of damon_migrate_pages(). Factor it into a small helper for readability. No functional change. Link: https://lore.kernel.org/[email protected] Signed-off-by: Li Youhong <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: Enze Li <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Cc: Hari Mishal <[email protected]> Cc: Jaeyeon Lee <[email protected]> Cc: Shuah Khan <[email protected]> Cc: "Zenghui Yu (Huawei)" <[email protected]> Cc: zhaozhengzhuo <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
…get test The obsolete_target test spawns three sh processes and uses their pids as DAMON monitoring targets. These processes are never terminated or waited on, so they are left running (or become zombies) as orphaned children after the test program exits. Terminate each process and communicate() with it after the targets are no longer needed, so it exits and gets reaped instead of being leaked. Link: https://lore.kernel.org/[email protected] Signed-off-by: Hari Mishal <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Cc: Enze Li <[email protected]> Cc: Jaeyeon Lee <[email protected]> Cc: Li Youhong <[email protected]> Cc: Shuah Khan <[email protected]> Cc: "Zenghui Yu (Huawei)" <[email protected]> Cc: zhaozhengzhuo <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Replace manual mutex_lock() and mutex_unlock() calls with the scoped_guard() macro. This simplifies the code, improves readability, and ensures that the lock is automatically released when the scope ends, preventing potential lock leaks in the future. Link: https://lore.kernel.org/[email protected] Signed-off-by: Jaeyeon Lee <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: Enze Li <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Cc: Hari Mishal <[email protected]> Cc: Li Youhong <[email protected]> Cc: Shuah Khan <[email protected]> Cc: "Zenghui Yu (Huawei)" <[email protected]> Cc: zhaozhengzhuo <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
_damon_sysfs.py defines constructors with mutable default arguments, including DamosAccessPattern(), DamosQuota(), DamosWatermarks(), DamosDests(), IntervalsGoal(), and empty lists. Default arguments are evaluated once at function definition time. Damos() instances created without explicit arguments therefore share the same DamosQuota(), and the other default-constructed sub-objects and lists are shared in the same way. The sub-objects keep back-pointers to their owner scheme, so constructing the second Damos() rebinds the shared quota's scheme pointer to the second object. An item appended to one object's default contexts or filters list is also visible from other default-constructed objects. The shared state can corrupt test configurations. DamosQuota.sysfs_dir() derives the sysfs directory from its scheme pointer, so operating on the first scheme's default quota may write to the second scheme's directory. The wrong values often match the defaults, so tests still pass, but the behavior depends on object creation order. Commit 8319dad ("selftests/damon: prevent cross-context state pollution in DamonCtx") fixed the same pattern in DamonCtx only. Fix the remaining constructors by defaulting to None and creating fresh objects or lists inside each constructor. Explicit arguments keep their previous behavior. Link: https://lore.kernel.org/[email protected] Signed-off-by: zhaozhengzhuo <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: Enze Li <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Cc: Hari Mishal <[email protected]> Cc: Jaeyeon Lee <[email protected]> Cc: Li Youhong <[email protected]> Cc: Shuah Khan <[email protected]> Cc: "Zenghui Yu (Huawei)" <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
The mtier sample defines a local struct region_range using phys_addr_t instead of damon_addr_range which uses unsigned long. Add a comment explaining the rationale: on 32-bit systems with more than 4GiB memory, phys_addr_t will be 64-bit while unsigned long is 32-bit. Link: https://lore.kernel.org/[email protected] Signed-off-by: Enze Li <[email protected]> Signed-off-by: SJ Park <[email protected]> Reviewed-by: SJ Park <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Cc: Hari Mishal <[email protected]> Cc: Jaeyeon Lee <[email protected]> Cc: Li Youhong <[email protected]> Cc: Shuah Khan <[email protected]> Cc: "Zenghui Yu (Huawei)" <[email protected]> Cc: zhaozhengzhuo <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Patch series "mm: optimize zone-device memmap initialization", v11. memmap_init_zone_device() can take a noticeable amount of time when large pmem namespaces are bound or rebound, because it initializes nearly identical struct page descriptors one PFN at a time. This series reduces that ZONE_DEVICE memmap initialization overhead by reusing prepared struct page templates and, on x86, using memcpy_nontemporal() for the template copy path. The main target is large fsdax/devdax pmem configurations, where the cost of initializing the memmap shows up directly in nd_pmem/dax_pmem bind and rebind latency. This matters because the cost is paid in the synchronous probe/bind path for large DAX/PMEM ZONE_DEVICE mappings. Userspace workflows such as provisioning or reconfiguring nd_pmem/dax_pmem namespaces, bringing hot-added PMEM-backed capacity online, and recovering or rebinding a device after driver or device changes all wait for this initialization to finish. Reducing this cost will yield benefits as lower user-visible provisioning, hot-add, recovery, and rebind latency for large DAX/PMEM devices. Patches 1-2 are preparatory cleanups and helper extraction. Patches 3-4 add the template-copy path for head pages and compound tails. Patch 5 introduces memcpy_nontemporal(). Patch 6 switches the ZONE_DEVICE template-copy path over to memcpy_nontemporal(). Patch 7 extends the x86 fixed-size memcpy_flushcache() inline cases used by the x86 memcpy_nontemporal() backend for struct page sized copies. Architectures without a specialized memcpy_nontemporal() backend fall back to memcpy(), so the generic template-copy optimization remains available without arch-specific support. On x86, memcpy_nontemporal() maps to the existing memcpy_flushcache() backend and can use the fixed-size MOVNTI paths added by this series for struct page sized copies. memcpy_nontemporal() is only a copy primitive. It does not imply a drain or a publication barrier. Callers that use it before a producer-consumer or device-visible handoff must provide the required ordering. The ZONE_DEVICE template-copy path uses it only while initializing struct page metadata, so the copy primitive itself does not grow a separate drain contract. The numbers below measure the time spent in memmap_init_zone_device() during driver bind/rebind. They are not measurements of the full nd_pmem or dax_pmem bind/rebind operation. Tested in an x86_64 QEMU/KVM VM with a 100 GB fsdax namespace device configured with map=dev and a 100 GB devdax namespace (align=2097152) on Intel Ice Lake server. Test procedure: Rebind the nd_pmem and dax_pmem drivers 30 times and collect the memmap initialization time from the pr_debug() output of memmap_init_zone_device(). Base(v7.3-rc1): Average of nd_pmem rebinds: 221.07 ms Average of dax_pmem rebinds: 191.20 ms With this series applied: Average of nd_pmem rebinds: 71.93 ms Average of dax_pmem rebinds: 87.37 ms This reduces the average memmap initialization time measured during rebind by about 67.5% for nd_pmem and 54.3% for dax_pmem. As an additional x86_64 data point, I also ran measurements on the same physical host with a 100 GB PMEM region created via the memmap= kernel command line, configured as fsdax and devdax namespaces with map=dev and 2 MiB alignment. For brevity, the individual patches keep only the VM results rather than including a second set of physical-host measurements throughout the series. The physical-host numbers below are included only as supplemental evidence that the same optimization also provides a similar benefit on a non-virtualized system. Test procedure: Reconfigure the namespace mode, rebind the nd_pmem or dax_pmem driver 30 times, and collect the memmap initialization time from the pr_debug() output of memmap_init_zone_device(). Base (v7.3-rc1): nd_pmem / fsdax: 205.90 ms dax_pmem / devdax: 225.43 ms With this series applied: nd_pmem / fsdax: 69.13 ms dax_pmem / devdax: 90.67 ms This reduces the measured memmap initialization time during rebind by about 66.4% for nd_pmem and 59.8% for dax_pmem on that setup, which is broadly consistent with the VM results above. As another supplemental data point, I measured the test_hmm.ko module on the same physical x86_64 host, using the test_hmm.ko setup from the previous discussion that times ten 64 GB memremap_pages()/memunmap_pages() iterations during module insertion[1]. By default, module insertion initializes two DEVICE_PRIVATE dmirror devices, so two avg memremap values are reported; each value is the average for one 64 GB chunk. This is not the primary target workload of the series, but it exercises the same large ZONE_DEVICE memmap initialization path and shows the same direction of improvement. Base (v7.3-rc1): avg memremap reported during module insertion: 116500596 ns, 116438028 ns With this series applied: avg memremap reported during module insertion: 46953088 ns, 4642839 ns This corresponds to about a 59.9% reduction based on the mean of the reported values, which is again consistent with the pmem bind/rebind results above. I also include an arm64 data point for the generic template-copy part. It was measured on an arm64 QEMU virt VM with 64 KB pages and a 100 GB ACPI NVDIMM sparse backend. This setup does not use the x86 MOVNTI fast paths, so it exercises the architecture-independent part of the optimization. For devdax, 2 MiB alignment is rejected in this 64 KB page setup, so the devdax namespace was tested with the supported default 512 MiB alignment. Base (v7.3-rc1): Average of rebinds for nd_pmem driver: 27.93 ms Average of rebinds for dax_pmem driver: 27.87 ms With this series applied: Average of rebinds for nd_pmem driver: 14.53 ms Average of rebinds for dax_pmem driver: 16.27 ms This reduces the average memmap initialization time measured during rebind by about 48.0% for nd_pmem and 41.6% for dax_pmem on that arm64 VM setup. Since this arm64 setup does not use the x86 MOVNTI fast paths, the result also suggests that the generic template-copy optimization can benefit architectures without an architecture-specific memcpy_nontemporal() backend. This patch (of 7): The comment in __init_zone_device_page() still uses the old MEMORY_TYPE_* names and implies that FS_DAX pages regain a refcount of 1 in the free path. That no longer matches the code. Update the comment to describe the current policy correctly: MEMORY_DEVICE_GENERIC pages regain a refcount of 1 in the free path, while the remaining ZONE_DEVICE types start from 0 here and raise the count again when the allocator or driver hands the page out. No functional change intended. Link: https://lore.kernel.org/[email protected] Link: https://lore.kernel.org/[email protected] Link: https://lore.kernel.org/all/[email protected]/ [1] Signed-off-by: Li Zhe <[email protected]> Reviewed-by: David Hildenbrand (Arm) <[email protected]> Reviewed-by: Alistair Popple <[email protected]> Reviewed-by: Muchun Song <[email protected]> Reviewed-by: Mike Rapoport (Microsoft) <[email protected]> Cc: Arnd Bergmann <[email protected]> Cc: Balbir Singh <[email protected]> Cc: "Borislav Petkov (AMD)" <[email protected]> Cc: Dave Hansen <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Kees Cook <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Callers that want to update section bits from a PFN currently need to open-code: set_page_section(page, pfn_to_section_nr(pfn)); and guard that sequence with #ifdef SECTION_IN_PAGE_FLAGS. Add set_page_section_from_pfn() to wrap that update in one place. When section bits are stored in page flags, the helper derives the section number from the PFN and updates the page flags. Otherwise keep it as a no-op so callers can use one helper without open-coding SECTION_IN_PAGE_FLAGS. Convert set_page_links() to use the new helper so later ZONE_DEVICE fast-path patches can also update section bits without open-coding SECTION_IN_PAGE_FLAGS at each callsite. This keeps the PFN-to-section translation local to the configurations that actually store section bits in struct page flags, and avoids exposing that detail to generic callers. No functional change intended. Link: https://lore.kernel.org/[email protected] Signed-off-by: Li Zhe <[email protected]> Reviewed-by: Mike Rapoport (Microsoft) <[email protected]> Acked-by: Muchun Song <[email protected]> Reviewed-by: Balbir Singh <[email protected]> Cc: Alistair Popple <[email protected]> Cc: Arnd Bergmann <[email protected]> Cc: "Borislav Petkov (AMD)" <[email protected]> Cc: Dave Hansen <[email protected]> Cc: David Hildenbrand (Arm) <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Kees Cook <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
memmap_init_zone_device() repeats nearly identical head-page initialization for each PFN. Initialize the first real ZONE_DEVICE head page through the existing path, copy that final state into a reusable template, refresh the PFN-dependent fields in that template before each copy, and copy it into the remaining destination pages. Use the template path unconditionally. The page_ref_set tracepoint is primarily a debugging aid, while this code is still initializing struct pages before they are handed out. From the perspective of users of those pages, the initialization-time refcount transitions are not part of the observable page lifetime. This means page_ref_set will no longer observe every initialization-time refcount assignment for copied ZONE_DEVICE head pages. The impact is controlled because the final initialized struct page state is unchanged, and keeping a separate non-template path only for this local tracepoint observability would add complexity to the common path. This patch accelerates head-page initialization. The pfns_per_compound == 1 case gets the full benefit here, compound tails are handled in the next patch. Tested in a VM with a 100 GB fsdax namespace device configured with map=dev on Intel Ice Lake server. This test exercises the nd_pmem rebind path (pfns_per_compound == 1). Test procedure: Rebind the nd_pmem driver 30 times and collect the memmap initialization time from the pr_debug() output of memmap_init_zone_device(). Base(v7.3-rc1): Average of rebinds for nd_pmem driver: 221.07 ms With this patch and its prerequisites applied: Average of rebinds for nd_pmem driver: 155.00 ms This reduces the average memmap initialization time measured during rebind from 221.07 ms to 155.00 ms, or about 29.9%. Link: https://lore.kernel.org/[email protected] Signed-off-by: Li Zhe <[email protected]> Cc: Alistair Popple <[email protected]> Cc: Arnd Bergmann <[email protected]> Cc: Balbir Singh <[email protected]> Cc: "Borislav Petkov (AMD)" <[email protected]> Cc: Dave Hansen <[email protected]> Cc: David Hildenbrand (Arm) <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Kees Cook <[email protected]> Cc: Mike Rapoport (Microsoft) <[email protected]> Cc: Muchun Song <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
The template fast path from the previous patch only accelerates head pages. Compound tails in memmap_init_compound() still go through the normal initialization path one by one. Build separate head and tail templates and reuse one prepared tail template across the tail pages in a compound range. Head pages preserve the existing refcount policy, while compound tails always start with a refcount of 0 after prep_compound_tail(). This extends the template-copy fast path to pfns_per_compound > 1. Tail-page PFN-dependent fields are refreshed in the reusable tail template before each copy. Do not keep a separate non-template fallback for compound tails either. These pages are still under memmap initialization, and the initialization-time refcount updates are not part of the observable lifetime of pages handed out later. The impact is controlled for the same reason as for head pages. The first tail page still seeds the reusable tail template through the normal tail initialization sequence, and the copied tail pages have the same final initialized state except for the PFN-dependent fields refreshed before each copy. Tested in a VM with a 100 GB devdax namespace (align=2097152) on Intel Ice Lake server. This test exercises the dax_pmem rebind path and measures memmap initialization latency. Test procedure: Unbind and rebind the dax_pmem driver 30 times, collect memmap initialization time from the pr_debug() output of memmap_init_zone_device(). Base(v7.3-rc1): Average of rebinds for dax_pmem driver: 191.20 ms With this patch and its prerequisites applied: Average of rebinds for dax_pmem driver: 176.87 ms This reduces the average memmap initialization time measured during rebind from 191.20 ms to 176.87 ms, or about 7.5%. Link: https://lore.kernel.org/[email protected] Signed-off-by: Li Zhe <[email protected]> Cc: Alistair Popple <[email protected]> Cc: Arnd Bergmann <[email protected]> Cc: Balbir Singh <[email protected]> Cc: "Borislav Petkov (AMD)" <[email protected]> Cc: Dave Hansen <[email protected]> Cc: David Hildenbrand (Arm) <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Kees Cook <[email protected]> Cc: Mike Rapoport (Microsoft) <[email protected]> Cc: Muchun Song <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Introduce memcpy_nontemporal() for write-once copy sites that want a named non-temporal copy primitive. On x86_64, override the helper in arch/x86/include/asm/string_64.h using the usual self-macro pattern, next to the existing memcpy_flushcache() backend that memcpy_nontemporal() wraps. include/linux/string.h provides the generic memcpy_nontemporal() fallback as #define memcpy_nontemporal(dst, src, len) \ ((void)memcpy(dst, src, len)) instead of an inline wrapper, so architectures without a specialized backend keep the usual memcpy() FORTIFY coverage when the compiler can still see object sizes at the original call site. It also makes the memcpy_nontemporal() API uniformly void, matching memcpy_flushcache() and the x86 backend, so callers cannot accidentally depend on a return value on fallback architectures. memcpy_nontemporal() is only a copy primitive. It does not imply a drain or a publication barrier. Callers that use it before a producer-consumer or device-visible handoff must provide the required ordering at that handoff point. The immediate user is the ZONE_DEVICE template-copy path. It populates struct page descriptors in a write-once pattern, so a regular cached memcpy() can incur avoidable write-allocate traffic and cache pollution for data with little near-term reuse. Link: https://lore.kernel.org/[email protected] Signed-off-by: Li Zhe <[email protected]> Cc: Alistair Popple <[email protected]> Cc: Arnd Bergmann <[email protected]> Cc: Balbir Singh <[email protected]> Cc: "Borislav Petkov (AMD)" <[email protected]> Cc: Dave Hansen <[email protected]> Cc: David Hildenbrand (Arm) <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Kees Cook <[email protected]> Cc: Mike Rapoport (Microsoft) <[email protected]> Cc: Muchun Song <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
The template fast path currently uses memcpy() for the actual struct page copy. Switch zone_device_page_init_from_template() to memcpy_nontemporal(). ZONE_DEVICE memmap initialization is largely write-once: each struct page is populated once, and most destination cachelines are not expected to be reused immediately afterwards. On x86, a regular cached memcpy() can therefore incur write-allocate traffic by pulling destination cachelines into the cache before writeback, and can populate the cache with data that has little near-term reuse. Using memcpy_nontemporal() lets this path request nontemporal stores for that copy pattern, which can reduce cache pollution and avoid part of the associated write-allocate overhead, while architectures without a specialized backend still fall back to memcpy(). Do not add a KASAN/KMSAN-specific fallback around this call site. As Muchun pointed out, special KASAN handling for memcpy_flushcache() or memcpy_nontemporal(), if needed, belongs in the low-level helper rather than in this ZONE_DEVICE caller. No separate drain is added here. memcpy_nontemporal() is used only as the copy primitive while memmap_init_zone_device() is still initializing the struct page array. The ordinary stores that follow in this path, such as compound-page setup, are part of the same CPU's initialization sequence; they are not used as a publication store that tells another CPU or device to consume data written by the non-temporal copy. Therefore this call site does not need a helper-level drain for correctness. Callers that use memcpy_nontemporal() as part of a producer-consumer or device-visible handoff must add the required ordering themselves. Tested in a VM with a 100 GB fsdax namespace device configured with map=dev and a 100 GB devdax namespace (align=2097152) on Intel Ice Lake server. Test procedure: Rebind the nd_pmem and dax_pmem driver 30 times and collect the memmap initialization time from the pr_debug() output of memmap_init_zone_device(). Base(v7.3-rc1): Average of rebinds for nd_pmem driver: 221.07 ms Average of rebinds for dax_pmem driver: 191.20 ms With this patch and its prerequisites applied: Average of rebinds for nd_pmem driver: 150.40 ms Average of rebinds for dax_pmem driver: 161.83 ms This reduces the average memmap initialization time measured during rebind by about 32.0% for nd_pmem and 15.4% for dax_pmem. Link: https://lore.kernel.org/[email protected] Signed-off-by: Li Zhe <[email protected]> Cc: Alistair Popple <[email protected]> Cc: Arnd Bergmann <[email protected]> Cc: Balbir Singh <[email protected]> Cc: "Borislav Petkov (AMD)" <[email protected]> Cc: Dave Hansen <[email protected]> Cc: David Hildenbrand (Arm) <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Kees Cook <[email protected]> Cc: Mike Rapoport (Microsoft) <[email protected]> Cc: Muchun Song <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
The x86 memcpy_nontemporal() helper maps to memcpy_flushcache(), and the ZONE_DEVICE template-copy path uses it to copy one struct page at a time. The relevant copy size is sizeof(struct page). On x86_64, the base struct page layout is 64 bytes. Adding either the KMSAN metadata pointers or an out-of-flags last_cpupid field can make it 80 bytes after alignment, and enabling both can make it 96 bytes. memcpy_flushcache() currently only has inline fixed-size cases for 4, 8, and 16 bytes. As a result, these constant-sized struct page copies fall through to __memcpy_flushcache() even though the compiler knows the copy size at the call site. Add fixed-size MOVNTI cases up to 96 bytes so the ZONE_DEVICE template-copy path can keep these struct page copies in the inline memcpy_flushcache() path. This matters for ZONE_DEVICE memmap initialization because the copy happens once per initialized struct page. For a 100 GB fsdax namespace with map=dev, this is about 25 million struct page copies during nd_pmem binding or rebinding. Tested in a VM with a 100 GB fsdax namespace device configured with map=dev and a 100 GB devdax namespace (align=2097152) on Intel Ice Lake server. Test procedure: Rebind the nd_pmem and dax_pmem drivers 30 times and collect the memmap initialization time from the pr_debug() output of memmap_init_zone_device(). With memcpy_nontemporal() used by the ZONE_DEVICE template-copy path: Average of rebinds for nd_pmem driver: 150.40 ms Average of rebinds for dax_pmem driver: 161.83 ms With this x86 fixed-size fastpath patch applied: Average of rebinds for nd_pmem driver: 71.93 ms Average of rebinds for dax_pmem driver: 87.37 ms This further reduces the average memmap initialization time measured during rebind by about 52.2% for nd_pmem and 46.0% for dax_pmem. Link: https://lore.kernel.org/[email protected] Signed-off-by: Li Zhe <[email protected]> Suggested-by: Borislav Petkov <[email protected]> Acked-by: Borislav Petkov (AMD) <[email protected]> Acked-by: Dave Hansen <[email protected]> Cc: Alistair Popple <[email protected]> Cc: Arnd Bergmann <[email protected]> Cc: Balbir Singh <[email protected]> Cc: David Hildenbrand (Arm) <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Kees Cook <[email protected]> Cc: Mike Rapoport (Microsoft) <[email protected]> Cc: Muchun Song <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
Commit 0919db9 ("HID: asus: always fully initialize devices") added a loop during asus_probe() to send keyboard feature report initializations (asus_kbd_init) to all ASUS HID devices. On ASUS laptops with I2C/HID touchpads (such as the ASUS E200HA), sending keyboard feature reports (FEATURE_KBD_REPORT_ID) to touchpad endpoints sends invalid feature requests to touchpad hardware, corrupting probe state and causing the touchpad to become unresponsive. Wrap the asus_report_id_init loop in an `if (!drvdata->tp)` check so keyboard feature initialization only runs for actual keyboards. Tested on ASUS E200HA (where touchpad functionality is fully restored) and ASUS VivoBook Flip 14 TP401MA (confirming zero regressions). Fixes: 0919db9 ("HID: asus: always fully initialize devices") Cc: [email protected] Signed-off-by: Panz Dev <[email protected]>
The AYANEO 3 handheld has a detachable controller with swappable
modules ("Magic Modules"). The controller exposes three USB HID
interfaces behind 1c4f:0002 (a generic SigmaMicro VID/PID, hence the
DMI gate): a gamepad, a keyboard for the extra buttons, and a vendor
interface accepting 65-byte commands.
Add a driver for the vendor interface providing module identification
(module_left/module_right sysfs attributes), software eject of the
modules (eject sysfs attribute, blocking until the firmware confirms
the release handshake), and RGB control of the joystick rings as a
multicolor LED class device ("<device name>:rgb:joystick_rings";
userspace such as InputPlumber matches the function suffix). The
firmware's fixed breathing pattern is exposed through the hw_pattern
trigger ABI.
This complements the ayaneo-ec platform driver, which exposes module
attach state and controller power. A full physical eject is performed
by writing to eject and then cutting power through ayaneo-ec's
controller_power attribute; that orchestration is deliberately left
to userspace.
The protocol was reverse engineered in the Handheld Daemon project by
Antheas Kapenekakis. Tested on an AYANEO 3: module identification,
RGB solid and breathing, a full eject/reinsert/repower cycle, and
repeated driver unbinds under a concurrent brightness-write load.
Signed-off-by: Matías Martínez <[email protected]>
Reviewed-by: Denis Benato <[email protected]>
Lets the build workflow compile the new driver. The real OGC config change is OpenGamingCollective/kernel-packages#35, which lands once the driver merges. Signed-off-by: Matías Martínez <[email protected]>
The default button map intends to assign F15 and F16 to M1 and M2, but the selected mapping table indexes resolve to F16 and F17. Use indexes 47 and 48 so the programmed usages match the existing comments. Assisted-by: LLM Fixes: e4c850a ("HID: hid-oxp: Add Button Mapping Interface") Signed-off-by: Andrei Aldea <[email protected]>
Check the short Gen2 status header before reading its command, and require a complete RGB status report before reading either generation's fields. Use the supplied input length for the debug dump as well, so a short report cannot cause an out-of-bounds read while logging. Fixes: 84910c4 ("HID: hid-oxp: Add OneXPlayer configuration driver") Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
Multiply the hardware brightness level by the LED brightness range before dividing by four. Dividing first reported every intermediate hardware level as zero instead of 25, 50 or 75 percent. Fixes: 84910c4 ("HID: hid-oxp: Add OneXPlayer configuration driver") Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
Do not replace the cached RGB state with a Gen2 status report whose enable, speed or hardware brightness fields are outside the supported ranges. Fixes: 252c4bf ("HID: hid-oxp: Add Second Generation RGB Control") Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
The controller accepts eight-bit RGB components, but the sub-LED channel maximum currently inherits the 0-100 overall brightness range. This prevents userspace from selecting channel intensities above 100. Set each channel maximum to 255 and use the multicolor LED core helper to scale components with the requested brightness. This also replaces the private truncating calculation with the LED core's rounded calculation. Keep the existing Gen1 and Gen2 color packet layouts unchanged. Fixes: 84910c4 ("HID: hid-oxp: Add OneXPlayer configuration driver") Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
The transport mutex protects one report at a time, but changing an RGB effect requires a status command followed by a color or effect command. Serialize whole RGB transactions so sysfs and delayed brightness work do not interleave these command sequences. Fixes: 84910c4 ("HID: hid-oxp: Add OneXPlayer configuration driver") Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
Set the requested effect before constructing its status command, since monocolor uses maximum hardware brightness and software intensity scaling. Previously entering or leaving monocolor used the old effect's brightness policy. Restore the cached effect if either output command fails. Fixes: 84910c4 ("HID: hid-oxp: Add OneXPlayer configuration driver") Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
Configuration callbacks can queue delayed RGB, button-mapping, and MCU initialization work. Normal removal previously closed the HID transport without synchronously stopping that work, while a later configuration probe failure could leave work queued as devres released its objects. Track which work items have been initialized, reject new output once teardown begins, disable initialized work synchronously, and drain an in-flight transport write before closing the device. Use the same shutdown path for normal removal and every probe-error unwind after work can have been exposed. This also avoids operating on uninitialized work for HID interfaces that do not own configuration state. Fixes: 84910c4 ("HID: hid-oxp: Add OneXPlayer configuration driver") Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
Replace the shared driver state and mutable LED object with devm-managed state owned by each configuration HID. Resolve callbacks through their HID, LED or embedded work object instead of the last interface probed. Hybrid devices have distinct Gen1 RGB and Gen2 controller interfaces. Sharing the transport pointer and work state lets one overwrite the other. Keep the HID drvdata pointer valid until LED and sysfs objects have been released, then clear it before freeing the configuration allocation. Fixes: 252c4bf ("HID: hid-oxp: Add Second Generation RGB Control") Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
|
Thanks for the review again - to make it easier to review I rebuilt it like you suggested as per-fix commits for the most part. So 1-10 are fixes, 11 is some group declarations and protocol definitions and then 12-15 is all X2/OXP3 changes. |
| return ret; | ||
| if (ret >= 0) { | ||
| /* MCU takes 200ms to be ready for another command. */ | ||
| msleep(200); |
There was a problem hiding this comment.
I don't think we need to move the msleep as well, that should fire even if this request had an error
There was a problem hiding this comment.
yeah you're right I'll make it unconditional
| #define OXP_PACKET_SIZE 64 | ||
| #define OXP_STATUS_HEADER_SIZE 6 | ||
| #define OXP_STATUS_ACK 0x20 | ||
|
|
There was a problem hiding this comment.
It is easier to read if we keep these broken up by functionality
|
|
||
| /* Hybrid devices expose RGB and controller configuration on separate HIDs. */ | ||
| struct oxp_hid_cfg { | ||
| struct led_classdev_mc cdev; |
There was a problem hiding this comment.
If we're re-ordering these lets group by function in reverse xmas tree.
See hid-msi and my PR here for hid-lenovo-go-s
There was a problem hiding this comment.
Missed that but I think I see what you mean now
| static int mcu_property_out(struct oxp_hid_cfg *cfg, u8 *header, size_t header_size, u8 *data, | ||
| size_t data_size, u8 *footer, size_t footer_size) | ||
| static int mcu_property_out(struct oxp_hid_cfg *cfg, u8 *header, | ||
| size_t header_size, u8 *data, size_t data_size, |
There was a problem hiding this comment.
checkpatch will flag these if they aren't lined up with the opening (
| return; | ||
|
|
||
| /* Re-apply the button mapping */ | ||
| /* Re-apply the button mapping. */ |
pastaq
left a comment
There was a problem hiding this comment.
A couple nits before upstreaming, but none blocking for merging here. I still need to review the last 2 patches in detail but so far looks good.
A Gen2 monocolor reply uses the same command value as the asynchronous MCU reset notification. Treating every such report as a reset can schedule a spurious controller reinitialization. Track an outstanding monocolor write by command and zone under a per-HID spinlock, and consume its matching acknowledgment before considering the report a reset notification. Clear pending reply state during suspend and teardown. A missing reply does not change legacy transport success semantics. For system suspend, disable and drain initialized configuration work and reject new output while the device is suspended. Re-enable work on resume and queue a fallback reinitialization after the documented MCU reset interval. A qualifying reset notification can still bring that work forward. Leave runtime autosuspend unchanged. Fixes: 2f424f2 ("HID: hid-oxp: Add Second Generation Gamepad Mode Switch") Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
Collect protocol definitions and helper macros at the top of the file, place the quirk structure with the other type declarations, and tidy configuration field ordering and callback declaration formatting. Keep this cleanup separate from the preceding behavioral fixes and the new-controller feature patches. Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
The ONEXPLAYER 3 and X2 Mini Pro need mapping format 0x02 and a third page preserving the extra buttons' factory mappings. Use exact DMI matches and select configuration interface 2; the other HID interfaces remain available without duplicate configuration or LED registration. Initialize each page with the selected format. Page three is fixed because its factory encodings have no entries in the public mapping table. Legacy devices keep their two-page format 0x20 transaction. Fixes: e4c850a ("HID: hid-oxp: Add Button Mapping Interface") Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
Move the existing joystick-ring class device, color components, delayed work, and cached settings into a per-LED wrapper owned by each HID configuration. Use a tagged state pointer so later LED types can share registration and work management without duplicating the lifecycle. Track only fully initialized work items and walk that count when quiescing, suspending, or resuming the configuration. Resolve LED callbacks through their containing wrapper, preserving the LED core's drvdata. Keep a single FULL joystick-ring LED and retain the existing Gen1/Gen2 55/57-byte RGB payloads, controls, and defaults. This commit adds no new hardware protocol or lighting zones. Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
Select the X2/Gen3 RGB protocol early for the ONEXPLAYER 3 and X2 Mini Pro. These controllers share the Gen2 HID usage page but require a 59-byte color payload and writes to ring zones 1, 2 and 7. Keep the Gen1 55-byte and legacy Gen2 57-byte color builders separate and unchanged. Add dedicated Gen3 status, color, effect and brightness paths. Check its known command/zone acknowledgments, retain cached color and effect when status cannot report them, and retry one complete ring pass on failure. Legacy transport success does not depend on receiving an acknowledgment. Restore cached X2 RGB once after the final controller-mode change. Generic ACK discrimination and suspend work management are preceding fixes; this patch does not add RGB replay to older controllers. Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
Add separate multicolor LED devices for the X2 guide button and rear logo using Gen3 zones 5 and 6. Each LED retains independent color, brightness, and monocolor or breathing effect state. Use the common LED array and type-tagged state introduced earlier. Protect auxiliary state snapshots with scoped spinlocks, serialize output with other RGB operations, and retry a failed command once. Do not alter an auxiliary zone until userspace has supplied valid state, and restore valid cached settings after resume. Expose these zones only on the matched ONEXPLAYER 3 and X2 Mini Pro. Physical validation of the latter remains outstanding. Document the auxiliary effects together with the existing joystick-ring effects and add the previously omitted controller, mapping, rumble, and ring-lighting ABI. Add the ABI file to the ONEXPLAYER HID maintainer pattern. Assisted-by: LLM Signed-off-by: Andrei Aldea <[email protected]>
af0e02d to
743e77c
Compare
This four-patch series fixes button mapping and extends RGB support for
ONEXPLAYER X2-family controllers, including the ONEXPLAYER 3 and X2 Mini Pro.
zones 0x05 and 0x06.
interfaces cannot overwrite the active driver state.
handling, and teardown safety.
Testing
Tested on an ONEXPLAYER 3 running Bazzite 44 with kernel
7.2.0-ogc6.1.fc44.x86_64.
Verified:
button and rear logo.
W=1.git diff --checkandcheckpatch.pl.The X2 Mini Pro uses the same controller protocol and interface layout, but
this series has not yet been tested on physical X2 Mini Pro hardware.
AI assistance disclosure
An LLM assisted with protocol analysis, implementation, documentation, code
review, and test orchestration. I reviewed and signed off on the resulting
changes and performed the physical ONEXPLAYER 3 testing described above.