Gerrit Promises / 62757
1327 threads read, 1023 from checkers only skipped, 16 duplicates folded in
Open 8
Open Ensure EC read reconstruction falls back to waiting for OST recovery instead of returning -EIO; tracked/merged via another patch Zhenyu Xu COMMIT_MSG:38 PS95 high resolved thread
The reply points to 63956, but that change is ABANDONED. It never asked for a fallback anyway: candidates/63956.diff only fixes an infinite loop in ll_readpage by returning -EIO for CIT_EC_RD pages past lcc_end_index, which is the -EIO Andreas asked about. On master (550451c0e4b9, which contains 783c13bb0b8f) a failed reconstruction still ends in an error. lov_io.c:3005-3011 sets ec_recovery_failed and returns -EAGAIN when err_nr > pcount. lov_io.c:3052-3059 returns -EIO when there are not enough parity stripes. vvp_io.c:1520-1525 then skips the read and lets the -EIO out. cl_io.c:875-881 only restarts on -EAGAIN while ci_ndelay is set and ci_tried_all_mirrors is false, and after that it returns -EIO. file.c:2293-2306 restarts at most RETRY_ATTEMPTS times. lov_io_mirror_init (lov_io.c:561-571) returns -EIO straight away for CIT_EC_RD when no parity is in sync. Nothing switches a failed EC_RD back to a blocking read (ci_ndelay=0) that waits for the OST to recover and retries the rebuild from time to time. The history and ticket files have no such commit. The closest candidate is 64919 (NEW). It adds LOV_EC_WAIT and ci_ndelay=0, but only for slow-marked stripes and for files with no usable parity. When hard-dead stripes exceed the parity count it still picks LOV_EC_RECONSTRUCT (candidates/64919.diff:633-643), so the whole-cluster-restart case with one OSS still down still fails. 68339 only adds a retry budget for concurrent EC readers (EC_RECOVERY_RETRY_MAX).
Suggested action: When CIT_EC_RD fails with ec_recovery_failed, or finds no in-sync parity, don't return -EIO. Restart the read as a blocking CIT_READ (ci_ndelay=0, switch_ec cleared) that waits for the failed OST's import to recover, and retry the reconstruction periodically. Add a sanity-ec test that takes down more OSTs than the parity count, brings all but one back, and checks the read succeeds. If this can't be done now, file an LU ticket marked Critical for 2.18.0 as the reviewer asked.
“This part is merged from patch https://review.whamcloud.com/c/fs/lustre-release/+/63956”Zhenyu Xu, PS95
The check looked at 39 changes — none keeps the promise
- 63956 abandoned LU-12669 llite: fix infinite loop in ll_readpage for EC reads linked in the thread
- 68453 new LU-12669 ec: create lock placeholder for reconstructed pages stacked on this change; touches lustre/tests/sanity-ec.sh (OST); same ticket LU-12669
- 68339 abandoned LU-12669 ec: serialize concurrent EC recovery readers stacked on this change; touches lustre/lov/lov_io.c (EIO, OST); same ticket LU-12669
- 66481 new LU-12669 ec: fix EC recovery read for encrypted files stacked on this change; same ticket LU-12669
- 64921 new LU-12668 tests: add EC failure orchestrator stacked on this change; LU-12668, left in lustre/lov/lov_io.c: * again -- that's a refcount underflow + UAF (LU-12668).
- 64919 new LU-12669 lov: slow and dead OST detection for degraded reads stacked on this change; cites this commit; LU-12668, left in lustre/lov/lov_io.c: * again -- that's a refcount underflow + UAF (LU-12668).
- 64086 new LU-12668 lov: handle ESHUTDOWN for LSEEK on EC files stacked on this change; LU-12668, left in lustre/lov/lov_io.c: * again -- that's a refcount underflow + UAF (LU-12668).
- 65219 new LU-12669 llite: make AIO reads on EC file synchronous stacked on this change; touches lustre/llite/file.c (EIO); same ticket LU-12669
- 67998 new LU-20566 ec: reuse surviving DIO bounce pages on EC recovery stacked on this change; promised in LU-20566; LU-20566, left in lustre/tests/sanity-ec.sh: always_except LU-20566 41j 41k 41l 41m 41n
- 67878 new LU-20566 ec: recover O_DIRECT reads with transient pages stacked on this change; promised in LU-20566; LU-20566, left in lustre/tests/sanity-ec.sh: always_except LU-20566 41j 41k 41l 41m 41n
- 66567 new LU-20212 lov: skip invalid OST mirrors for aggregate I/O stacked on this change; touches lustre/lov/lov_io.c (EIO, OST)
- 66112 new DRAFT LU-20244 utils: resync/verify stale mirrors in 1GB chunks. stacked on this change
- 66111 new DRAFT LU-20244 utils: add support to resync a region stacked on this change
- 66110 new DRAFT LU-20244 tests: add tests for resync of FLR + EC stacked on this change
- 64616 merged LU-19823 lod: Make the stripe allocators failure domain aware. stacked on this change; same topic or hashtag (OSS, OST)
- 63809 merged LU-19852 lod: raidset aware stripe allocator stacked on this change
- 63678 new LU-19839 llite: add persistent nohybrid I/O flag stacked on this change
- 66672 merged LU-12668 ec: allow SEEK_DATA/HOLE on parity mirror lseek stacked on this change; LU-12668, left in lustre/lov/lov_io.c: * again -- that's a refcount underflow + UAF (LU-12668).
- 66113 merged LU-20036 lfs: add --ec-map getstripe output for EC raidsets stacked on this change; touches lustre/tests/sanity-ec.sh (OST)
- 64620 merged LU-19548 lfs: update mirror split for EC support stacked on this change; touches lustre/tests/sanity-ec.sh (OST)
- 68278 merged LU-20565 tests: lower required OSTCOUNT for sanity-ec/7d promised in LU-20565
- 66436 merged LU-12668 ec: ensure hole ranges for raidset parity LU-12668, left in lustre/lov/lov_io.c: * again -- that's a refcount underflow + UAF (LU-12668).
- 65921 new LU-19548 lfs: mirror extend support for EC LU-12668, left in lustre/lov/lov_io.c: * again -- that's a refcount underflow + UAF (LU-12668).
- 62859 new LU-12668 ec: Add tests for computing the parity coverage LU-12668, left in lustre/lov/lov_io.c: * again -- that's a refcount underflow + UAF (LU-12668).
- 68035 merged LU-12668 ec: add --stats and -W into EC resync LU-12668, left in lustre/lov/lov_io.c: * again -- that's a refcount underflow + UAF (LU-12668).
- 68204 new LU-19631 tests: enable sanity-ec 12a LU-19631, left in lustre/tests/sanity-ec.sh: always_except LU-19631 12a
- 66691 merged LU-12668 ec: Allow resyncing parities for nosync data mirrors LU-12668, left in lustre/lov/lov_io.c: * again -- that's a refcount underflow + UAF (LU-12668).
- 67602 new LU-20534 mdd: add single-stripe layout swap same topic or hashtag (EIO, OST)
- 57454 new LU-18461 layout: allow to be started from zero offset. touches lustre/lov/lov_io.c (EIO, OST)
- 68361 new LU-20690 ec: like-RAID10 parity layout so p may exceed k same topic or hashtag (EIO, OST)
- 64441 new LU-12187 lov: MDS layout pattern negotiation same topic or hashtag (EIO, OST)
- 67651 new LU-20541 lfs: Implement migrate_stripe command and add test same topic or hashtag (EIO, OST)
- 67973 new LU-20589 llite: fail IO when the restart budget runs out touches lustre/llite/vvp_io.c (OST)
- 69187 new LU-19900 lov: split DIO appends at stripe boundaries touches lustre/lov/lov_io.c (OST)
- 66882 new LU-20211 mdt: trunc(0) layout replace on inactive stripes same topic or hashtag (OST)
- 58097 merged LU-17916 clio: switch to struct folio touches lustre/llite/rw.c (EIO)
- 35302 new LU-13428 llite: correct cl_sync_file_range() arguments touches lustre/llite/file.c (OST)
- 65798 new LU-20238 ec: GFNI-accelerated EC primitives same topic or hashtag (OSS)
- 55419 new LU-10499 pcc: debug for the mmap panic touches lustre/llite/rw.c (EIO)
Thread · 3 messages
Open Fix CWARN/return value mismatch (rc=-EIO vs -EAGAIN) when patch is refreshed Marc Vef lustre/lov/lov_io.c:3011 PS112 med
The mismatch is still there. Both change/lustre/lov/lov_io.c:3006-3011 (PS113) and master/lustre/lov/lov_io.c:3006-3011 (550451c0e4b9) still call CWARN(... rc = %d ..., -EIO) and then GOTO(rg_cleanup, rc = -EAGAIN). No commit in history/809d5884_40b8de78.txt changes it. In the candidates, 58097.diff only shows the unchanged GOTO line as context, and 67878.diff only touches the separate i_mapping -EAGAIN exit. No follow-up changes the CWARN argument. Marc Vef called this minor and deferred it to a refresh that never happened. Andreas Dilger asked for rc to be set once and used in both places.
Suggested action: In lov_io_ec_rd_start(), set rc = -EAGAIN before the 'too many unrecoverable stripes' CWARN, pass rc to the CWARN instead of the hard-coded -EIO, then GOTO(rg_cleanup, rc). This makes the log and the return value agree.
“`CWARN` can be fixed if this patch is refreshed.”Marc Vef, PS112
The check looked at 19 changes — none keeps the promise
- 68453 new LU-12669 ec: create lock placeholder for reconstructed pages stacked on this change; touches lustre/llite/vvp_io.c (ci_ec, ec_recovery_failed); same ticket LU-12669
- 68339 abandoned LU-12669 ec: serialize concurrent EC recovery readers stacked on this change; touches lustre/lov/lov_io.c (ci_ec, ci_need_restart, ci_tried_all_mirrors); same ticket LU-12669
- 66481 new LU-12669 ec: fix EC recovery read for encrypted files stacked on this change; touches lustre/lov/lov_io.c (ec_recovery_failed, err_nr, ld_lov); same ticket LU-12669
- 64921 new LU-12668 tests: add EC failure orchestrator stacked on this change
- 64919 new LU-12669 lov: slow and dead OST detection for degraded reads stacked on this change; cites this commit; touches lustre/lov/lov_io.c (ci_ndelay, ci_need_restart, ci_tried_all_mirrors)
- 64086 new LU-12668 lov: handle ESHUTDOWN for LSEEK on EC files stacked on this change
- 65219 new LU-12669 llite: make AIO reads on EC file synchronous stacked on this change; same ticket LU-12669
- 67998 new LU-20566 ec: reuse surviving DIO bounce pages on EC recovery stacked on this change; promised in LU-20566
- 67878 new LU-20566 ec: recover O_DIRECT reads with transient pages stacked on this change; promised in LU-20566
- 66567 new LU-20212 lov: skip invalid OST mirrors for aggregate I/O stacked on this change
- 58097 merged LU-17916 clio: switch to struct folio touches lustre/lov/lov_io.c (ci_ec, err_nr, i_mapping)
- 57454 new LU-18461 layout: allow to be started from zero offset. touches lustre/lov/lov_io.c (lu_object_fid, DFID, EIO)
- 68361 new LU-20690 ec: like-RAID10 parity layout so p may exceed k touches lustre/lov/lov_io.c (parity_stripe, RETURN)
- 66131 new LU-19900 lov: DIO+APPEND corruption on PFL extent boundaries touches lustre/llite/vvp_io.c (ci_need_restart)
- 66672 merged LU-12668 ec: allow SEEK_DATA/HOLE on parity mirror lseek touches lustre/llite/vvp_io.c (lov_io_mirror_init)
- 66113 merged LU-20036 lfs: add --ec-map getstripe output for EC raidsets touches lustre/tests/sanity-ec.sh (parity_stripe)
- 67996 new LU-20590 llite: pin the layout lock for a starving IO touches lustre/llite/vvp_io.c (CIT_WRITE, DFID, PFID)
- 65921 new LU-19548 lfs: mirror extend support for EC touches lustre/tests/sanity-ec.sh (FLR)
- 64616 merged LU-19823 lod: Make the stripe allocators failure domain aware. touches lustre/tests/sanity-ec.sh (FLR)
Thread · 3 messages
Code at the comment
3006: CWARN("%s: "DFID": too many unrecoverable stripes in raid set %d: err_nr %d > pcount %d: rc = %d\n",
3007: lov2obd(dev->ld_lov)->obd_name,
3008: PFID(lu_object_fid(lov2lu(lov))),
3009: raid_set, err_nr, pcount, -EIO);
3010: io->u.ci_ec.ec_recovery_failed = true;
>>> 3011: GOTO(rg_cleanup, rc = -EAGAIN);
3012: }
3013:
3014: /* Batch parity reads if needed. par_vmpages/par_clpages
3015: * indexed as [parity_stripe * npages + page_pos].
3016: */
Open fix kmap_local unmap ordering/LIFO violation and KM_MAX_IDX overrun risk Zhenyu Xu lustre/lov/lov_io.c:2759 PS89 med resolved thread
Only half of the item is fixed. The LIFO ordering was fixed at PS113. In change/lustre/lov/lov_io.c:2673-2681 the parity mappings are now released in reverse (k = r_j-1..0). At :2684-2720 the forward data/error loop is walked again in reverse to unmap them, and :3179-3189 has the same fix. master/ is identical. The KM_MAX_IDX overrun is not fixed. lov_ec_recover_page_pos() (:2608-2650) still keeps a kmap_local_page() mapping open for every available data page and every error page. That can be up to dcount mappings, and then r_j more for parity, all held at once. Nothing limits that to KM_MAX_IDX (16), while LOV_EC_MAX_DATA_STRIPES is 255. The reviewer's suggestion to use page_address() was not taken. No master commit fixes this. Of the candidates, 58097 only swaps in cl_kmap_local/kmap_local_folio, and 66481/67878 keep the same pattern of holding every mapping at once.
Suggested action: In lov_ec_recover_page_pos(), stop holding up to dcount+pcount kmap_local mappings at the same time. Either use page_address() (these are lowmem page-cache/kernel pages) or kmap() for the data, error and parity pages. Otherwise, reject or cap dcount+pcount at KM_MAX_IDX when kmap_local_page() is not a no-op.
“Acknowledged”Zhenyu Xu, PS89
The check looked at 14 changes — none keeps the promise
- 68453 new LU-12669 ec: create lock placeholder for reconstructed pages stacked on this change; same ticket LU-12669
- 68339 abandoned LU-12669 ec: serialize concurrent EC recovery readers stacked on this change; touches lustre/lov/lov_io.c (EC_DPG_READ, EC_DPG_UPTODATE, WARN); same ticket LU-12669
- 66481 new LU-12669 ec: fix EC recovery read for encrypted files stacked on this change; touches lustre/lov/lov_io.c (err_array_j, kmap_local, kmap_local_page); same ticket LU-12669
- 64921 new LU-12668 tests: add EC failure orchestrator stacked on this change
- 64919 new LU-12669 lov: slow and dead OST detection for degraded reads stacked on this change; cites this commit; same ticket LU-12669
- 64086 new LU-12668 lov: handle ESHUTDOWN for LSEEK on EC files stacked on this change
- 65219 new LU-12669 llite: make AIO reads on EC file synchronous stacked on this change; same ticket LU-12669
- 67998 new LU-20566 ec: reuse surviving DIO bounce pages on EC recovery stacked on this change; promised in LU-20566
- 67878 new LU-20566 ec: recover O_DIRECT reads with transient pages stacked on this change; promised in LU-20566; touches lustre/lov/lov_io.c (d_j, err_array_j, kunmap_local)
- 66567 new LU-20212 lov: skip invalid OST mirrors for aggregate I/O stacked on this change
- 68278 merged LU-20565 tests: lower required OSTCOUNT for sanity-ec/7d promised in LU-20565
- 58097 merged LU-17916 clio: switch to struct folio touches lustre/lov/lov_io.c (kmap_local, kmap_local_page)
- 68409 new LU-10026 csdc: tunable to disable compression on fallocate touches lustre/lov/lov_io.c (WARN)
- 66276 new LU-10026 lov: fallocate is not allowed for compressed files touches lustre/lov/lov_io.c (WARN)
Thread · 2 messages
Code at the comment
2754: case EC_DPG_ZERO:
2755: ptrs[p_j++] = zerobuf;
2756: break;
2757: case EC_DPG_UPTODATE:
2758: case EC_DPG_READ:
>>> 2759: ptrs[p_j++] = kmap_local_page(
2760: all_vmpages[idx]);
2761: break;
2762: case EC_DPG_ERROR:
2763: err_array_j[d_j] = k;
2764: /* vmpage is still alive (the submit
Open Add --printf option support for lfs getstripe to print single-component OST indexes Andreas Dilger lustre/tests/sanity-ec.sh:4021 PS92 low resolved thread
Master has no getstripe --printf: in master/lustre/utils/lfs.c, fp_format_printf_str is only set by the lfs find option parser (lines 7446 and 8161). Change 68989 (NEW) adds 'lfs getstripe --printf', so '%Lo' would print the OST indexes of the whole file. But its lfs_getstripe_internal hunk (candidates/68989.diff:485-516) rejects -I/--component-id, --comp-flags, -E, --mirror-id and similar with 'can't be used with --printf. prefer lfs find'. So it still can't print one component's OST indexes, which was both the limitation Andreas named and the thing sanity-ec.sh:4021 needs ('getstripe -I${ids[0]}' piped to awk on l_ost_idx). Andreas's reply saying this 'is implemented' in 68989 is only partly right.
- 68989 new LU-20645 utils: add --printf and --ls opts to lfs-getstripe
Suggested action: Extend 68989, or add a follow-up, so 'lfs getstripe --printf' honours -I/--component-id (and the other component and mirror selectors) and prints %Lo for just the chosen component, instead of rejecting them. Then switch sanity-ec.sh to use 'lfs getstripe -I<id> --printf "%Lo"' in place of the awk parsing of l_ost_idx.
“This is implemented in patch https://review.whamcloud.com/68989 ("LU-20645 utils: add --printf and --ls opts to lfs-getstripe")”Andreas Dilger, PS92
The check looked at 20 changes — none keeps the promise
- 68989 new LU-20645 utils: add --printf and --ls opts to lfs-getstripe linked in the thread
- 68453 new LU-12669 ec: create lock placeholder for reconstructed pages stacked on this change; touches lustre/tests/sanity-ec.sh (LFS, OST); same ticket LU-12669
- 68339 abandoned LU-12669 ec: serialize concurrent EC recovery readers stacked on this change; same ticket LU-12669
- 66481 new LU-12669 ec: fix EC recovery read for encrypted files stacked on this change; same ticket LU-12669
- 64921 new LU-12668 tests: add EC failure orchestrator stacked on this change
- 64919 new LU-12669 lov: slow and dead OST detection for degraded reads stacked on this change; cites this commit; touches lustre/tests/sanity-ec.sh (LFS, OST)
- 64086 new LU-12668 lov: handle ESHUTDOWN for LSEEK on EC files stacked on this change
- 65219 new LU-12669 llite: make AIO reads on EC file synchronous stacked on this change; same ticket LU-12669
- 67998 new LU-20566 ec: reuse surviving DIO bounce pages on EC recovery stacked on this change; promised in LU-20566; touches lustre/tests/sanity-ec.sh (l_ost_idx, lcme_id, LFS)
- 67878 new LU-20566 ec: recover O_DIRECT reads with transient pages stacked on this change; promised in LU-20566; touches lustre/tests/sanity-ec.sh (l_ost_idx, lcme_id, LFS)
- 66567 new LU-20212 lov: skip invalid OST mirrors for aggregate I/O stacked on this change
- 68278 merged LU-20565 tests: lower required OSTCOUNT for sanity-ec/7d promised in LU-20565
- 65921 new LU-19548 lfs: mirror extend support for EC touches lustre/tests/sanity-ec.sh (l_ost_idx, lcme_id, LFS)
- 64616 merged LU-19823 lod: Make the stripe allocators failure domain aware. touches lustre/tests/sanity-ec.sh (l_ost_idx, LFS, OST)
- 68361 new LU-20690 ec: like-RAID10 parity layout so p may exceed k touches lustre/tests/sanity-ec.sh (lcme_id, LFS, OST)
- 66113 merged LU-20036 lfs: add --ec-map getstripe output for EC raidsets touches lustre/tests/sanity-ec.sh (lcme_id, LFS, OST)
- 66436 merged LU-12668 ec: ensure hole ranges for raidset parity touches lustre/tests/sanity-ec.sh (lcme_id, LFS, OST)
- 63809 merged LU-19852 lod: raidset aware stripe allocator touches lustre/tests/sanity-ec.sh (l_ost_idx, LFS, OST)
- 67613 new LU-19994 ec: block LCME_FL_PARITY flag changes touches lustre/tests/sanity-ec.sh (lcme_id, LFS)
- 64620 merged LU-19548 lfs: update mirror split for EC support touches lustre/tests/sanity-ec.sh (LFS, OST)
Thread · 3 messages
Code at the comment
4016: local sum1=$(md5sum $tf | awk '{print $1}')
4017:
4018: # collect objects
4019: local ids=($($LFS getstripe $tf | awk '/lcme_id/{print $2}' |
4020: tr '\n' ' '))
>>> 4021: local osts=($($LFS getstripe -I${ids[0]} $tf |
4022: awk '/l_ost_idx:/ { print $5 }' | tr -d "," ))
4023:
4024: # randomly disable 2 OSTs
4025: local idx1=$((RANDOM % ${#osts[@]}))
4026: local idx2=$((RANDOM % ${#osts[@]}))
Open initialize lsme_p/pindex defensively when patch is refreshed Marc Vef lustre/lov/lov_io.c:2743 PS112 low
In change/lustre/lov/lov_io.c:2743 and :2747 (PS113, 783c13bb0b8f), `lsme_p` and `pindex` are still declared without initialisers. The same lines are unchanged on master/lustre/lov/lov_io.c:2743/2747 (550451c0e4b9), and `lle_p` just above is still the only one set to NULL. None of the candidate diffs touch these declarations. 67878 only adds an argument to a call that uses lsme_p, and the other matches are unrelated `lsme->lsme_pattern` hits. The change merged at PS113 without the refresh the promise depended on. The reviewer's point holds: this is not a bug, since the reads are safe only because dstripe_count == 0 leads to pcount == 0 and so err_nr == 0. It is a readability and -Wmaybe-uninitialized clean-up that nobody has done yet.
Suggested action: In the EC recovery function in lustre/lov/lov_io.c, initialise `lsme_p` to NULL and `pindex` to -1 where they are declared, as `lle_p` already is. That makes the safety of the reads obvious locally and keeps -Wmaybe-uninitialized quiet.
“nevertheless, this can be addressed if the patch is refreshed”Marc Vef, PS112
The check looked at 20 changes — none keeps the promise
- 68453 new LU-12669 ec: create lock placeholder for reconstructed pages stacked on this change; same ticket LU-12669
- 68339 abandoned LU-12669 ec: serialize concurrent EC recovery readers stacked on this change; same ticket LU-12669
- 66481 new LU-12669 ec: fix EC recovery read for encrypted files stacked on this change; touches lustre/lov/lov_io.c (err_nr, lov_object_dev, NULL); same ticket LU-12669
- 64921 new LU-12668 tests: add EC failure orchestrator stacked on this change
- 64919 new LU-12669 lov: slow and dead OST detection for degraded reads stacked on this change; cites this commit; touches lustre/lov/lov_io.c (dstripe_count, err_nr, lov_layout_entry)
- 64086 new LU-12668 lov: handle ESHUTDOWN for LSEEK on EC files stacked on this change
- 65219 new LU-12669 llite: make AIO reads on EC file synchronous stacked on this change; same ticket LU-12669
- 67998 new LU-20566 ec: reuse surviving DIO bounce pages on EC recovery stacked on this change; promised in LU-20566
- 67878 new LU-20566 ec: recover O_DIRECT reads with transient pages stacked on this change; promised in LU-20566
- 66567 new LU-20212 lov: skip invalid OST mirrors for aggregate I/O stacked on this change
- 68834 new LU-20438 clio: eagerly evaluate P2P DMA capability via layout touches lustre/lov/lov_io.c (lle_d, lov_layout_entry, lov_stripe_md_entry)
- 64441 new LU-12187 lov: MDS layout pattern negotiation touches lustre/lov/lov_io.c (lov_layout_entry, lov_stripe_md_entry, lsme_p)
- 68361 new LU-20690 ec: like-RAID10 parity layout so p may exceed k touches lustre/lov/lov_io.c (lov_stripe_md_entry, lsme_p, NULL)
- 58097 merged LU-17916 clio: switch to struct folio touches lustre/lov/lov_io.c (err_nr, NULL)
- 67282 new LU-20483 lov: fix lo_loi NULL deref in DOM layout init touches lustre/lov/lov_object.c (lle_d, NULL)
- 69187 new LU-19900 lov: split DIO appends at stripe boundaries touches lustre/lov/lov_io.c (lov_stripe_md_entry)
- 57454 new LU-18461 layout: allow to be started from zero offset. touches lustre/lov/lov_io.c (lov_stripe_md_entry)
- 40058 new LU-13980 osd: remove osd_object_release LASSERT touches lustre/lov/lov_object.c (NULL)
- 69183 new LU-17000 lov: Remove dead code from lov_io_layout_mirror touches lustre/lov/lov_io.c
- 69205 new LU-20783 llite: ignore nolock, remove LL_IOC_SETFLAGS touches lustre/lov/lov_io.c
Thread · 2 messages
Code at the comment
2738: struct lov_device *dev = lov_object_dev(lov);
2739: struct lov_layout_composite *comp = &lov->u.composite;
2740: struct lov_layout_entry *lle_d = NULL; /* data component entry */
2741: struct lov_layout_entry *lle_p = NULL; /* parity component entry */
2742: struct lov_stripe_md_entry *lsme_d;
>>> 2743: struct lov_stripe_md_entry *lsme_p;
2744: int data_mir_idx; /* data mirror index */
2745: int parity_mir_idx; /* parity mirror index */
2746: int index; /* data component index */
2747: int pindex; /* parity component index */
2748:
Open Fix inverted top-to-bottom/bottom-to-top doc wording via follow-up change 68339 Marc Vef lustre/include/cl_object.h:442 PS112 low resolved thread 1 possible follow-up
Kept in change 68339 — NEW follow-up changes the wording to top-to-bottom in cl_object.h and cl_io.c and makes cl_io_set_range static
The review comment was correct. On master, cl_io_set_range() at lustre/obdclass/cl_io.c:1202 walks the layers with cl_object_for_each(), which cl_object.h:484 describes as top-to-bottom. The docs still say the opposite: cl_object.h:442 has 'bottom to top' and cl_io.c:1191 has 'bottom-to-top'. change/ (PS113) has the same wording. candidates/68339.diff:115-116 changes cl_object.h to 'top to bottom', lines 506/533 change the cl_io.c kerneldoc to 'top-to-bottom', and line 511 makes cl_io_set_range static. 68339 is still NEW, so this fix has not landed.
Suggested action: Land 68339. It changes coo_io_set_range and cl_io_set_range() docs to say top-to-bottom and makes cl_io_set_range static.
“Already addressed in follow-up: 68339 (direct child) flips the wording in cl_object.h and cl_io.c and makes cl_io_set_range static.”Marc Vef, PS112
Possible follow-ups — found in Gerrit, not confirmed
- 68339 abandoned LU-12669 ec: serialize concurrent EC recovery readers linked in the thread
Thread · 2 messages
Code at the comment
437: */
438: void (*coo_req_projid_set)(const struct lu_env *env,
439: struct cl_object *obj, __u32 *projid);
440: /**
441: * Set the coverage of the \a io, used by CIT_EC_RD, called from
>>> 442: * bottom to top.
443: */
444: void (*coo_io_set_range)(const struct lu_env *env,
445: struct cl_object *obj, struct cl_io *io);
446: };
447:
Open Clean up duplicate/unnecessary local EC stripe-count variables in favor of EC_DSTRIPE if patch is refreshed Marc Vef lustre/tests/sanity-ec.sh:4242 PS112 low
The redundant locals are still there at patchset 113: change/lustre/tests/sanity-ec.sh:4241-4242 has `local dstripe=$EC_DSTRIPE` and `local cstripe=$EC_CSTRIPE`. They also landed unchanged on master in test_40a (master/lustre/tests/sanity-ec.sh:4745-4746), where they are used in the setstripe call at line 4748. No commit in the history removes them, and no diff in candidates/ deletes either line. The reviewer's point is valid, because enable_ec() already sets EC_DSTRIPE and EC_CSTRIPE. This is a minor cleanup.
Suggested action: In test_40a of lustre/tests/sanity-ec.sh, drop the `local dstripe=$EC_DSTRIPE` and `local cstripe=$EC_CSTRIPE` lines and use $EC_DSTRIPE and $EC_CSTRIPE directly in the setstripe command and its error message.
“Can be updated if refreshed”Marc Vef, PS112
The check looked at 20 changes — none keeps the promise
- 68453 new LU-12669 ec: create lock placeholder for reconstructed pages stacked on this change; touches lustre/tests/sanity-ec.sh (stack_trap, DIR, LFS); same ticket LU-12669
- 68339 abandoned LU-12669 ec: serialize concurrent EC recovery readers stacked on this change; touches lustre/tests/sanity-ec.sh; same ticket LU-12669
- 66481 new LU-12669 ec: fix EC recovery read for encrypted files stacked on this change; same ticket LU-12669
- 64921 new LU-12668 tests: add EC failure orchestrator stacked on this change
- 64919 new LU-12669 lov: slow and dead OST detection for degraded reads stacked on this change; cites this commit; touches lustre/tests/sanity-ec.sh (enable_ec, stack_trap, DIR)
- 64086 new LU-12668 lov: handle ESHUTDOWN for LSEEK on EC files stacked on this change
- 65219 new LU-12669 llite: make AIO reads on EC file synchronous stacked on this change; same ticket LU-12669
- 67998 new LU-20566 ec: reuse surviving DIO bounce pages on EC recovery stacked on this change; promised in LU-20566
- 67878 new LU-20566 ec: recover O_DIRECT reads with transient pages stacked on this change; promised in LU-20566
- 66567 new LU-20212 lov: skip invalid OST mirrors for aggregate I/O stacked on this change
- 65921 new LU-19548 lfs: mirror extend support for EC touches lustre/tests/sanity-ec.sh (enable_ec, stack_trap, DIR)
- 67613 new LU-19994 ec: block LCME_FL_PARITY flag changes touches lustre/tests/sanity-ec.sh (enable_ec, stack_trap, DIR)
- 64616 merged LU-19823 lod: Make the stripe allocators failure domain aware. touches lustre/tests/sanity-ec.sh (enable_ec, stack_trap, DIR)
- 68361 new LU-20690 ec: like-RAID10 parity layout so p may exceed k touches lustre/tests/sanity-ec.sh (enable_ec, stack_trap, DIR)
- 64620 merged LU-19548 lfs: update mirror split for EC support touches lustre/tests/sanity-ec.sh (enable_ec, stack_trap, DIR)
- 62859 new LU-12668 ec: Add tests for computing the parity coverage touches lustre/tests/sanity-ec.sh (enable_ec, stack_trap, DIR)
- 66113 merged LU-20036 lfs: add --ec-map getstripe output for EC raidsets touches lustre/tests/sanity-ec.sh (enable_ec, DIR, LFS)
- 62868 new LU-12187 llite: enable FLR EC support unconditionally touches lustre/tests/sanity-ec.sh (stack_trap, LFS)
- 66672 merged LU-12668 ec: allow SEEK_DATA/HOLE on parity mirror lseek touches lustre/tests/sanity-ec.sh
- 65798 new LU-20238 ec: GFNI-accelerated EC primitives touches lustre/tests/sanity-ec.sh
Thread · 2 messages
Code at the comment
4237: local tf=$DIR/$tfile
4238:
4239: stack_trap "rm -f $tf"
4240: enable_ec
4241: local dstripe=$EC_DSTRIPE
>>> 4242: local cstripe=$EC_CSTRIPE
4243: # Create a simple EC file with data mirror + parity mirror
4244: $LFS setstripe -E eof -S 1M -c $dstripe --ec $dstripe+$cstripe $tf ||
4245: error "setstripe --ec $dstripe+$cstripe failed"
4246:
4247: # Write some data to the file
Open Make EC stripe counts global variables across all sanity-ec.sh subtests Marc Vef lustre/tests/sanity-ec.sh:4030 PS110 low resolved thread
The EC stripe-count globals exist: master/lustre/tests/sanity-ec.sh:130-133 sets EC_CSTRIPE, EC_DSTRIPE, EC_MINSTRIPE and EC_ARG. But only a few subtests use them (lines 337-1648 and 4745-4746, where test_40a now reads EC_DSTRIPE/EC_CSTRIPE). master/lustre/tests/sanity-ec.sh still has 221 hard-coded `--ec 4+2` and 335 hard-coded `--ec N+M` in total. The deferred work, converting the other subtests, has not been done. The only commit on master under LU-20565 is 9e6ca9155dd3 (change 68278). It changes one line to lower the OSTCOUNT requirement for sanity-ec/7d and does not switch any tests to the globals. None of the listed candidate diffs adds EC_ARG or EC_DSTRIPE use apart from 63809, which is not in the follow-up list. Its content (introducing the globals) already matches master.
- 68278 merged LU-20565 tests: lower required OSTCOUNT for sanity-ec/7d
Suggested action: Under LU-20565, replace the remaining hard-coded `--ec 4+2` and `-c 4`/`-c 6` values in lustre/tests/sanity-ec.sh with $EC_ARG, $EC_DSTRIPE, $EC_CSTRIPE and $EC_MINSTRIPE. Change each subtest's OSTCOUNT skip check to use EC_MINSTRIPE, so the tests can run with 3 or more OSTs.
“This needs to be done in other places too, but this is deferred to LU-20565”Marc Vef, PS110
The check looked at 24 changes — none keeps the promise
- 68453 new LU-12669 ec: create lock placeholder for reconstructed pages stacked on this change; touches lustre/tests/sanity-ec.sh (stack_trap, DIR, LFS); same ticket LU-12669
- 68339 abandoned LU-12669 ec: serialize concurrent EC recovery readers stacked on this change; touches lustre/tests/sanity-ec.sh; same ticket LU-12669
- 66481 new LU-12669 ec: fix EC recovery read for encrypted files stacked on this change; same ticket LU-12669
- 64921 new LU-12668 tests: add EC failure orchestrator stacked on this change
- 64919 new LU-12669 lov: slow and dead OST detection for degraded reads stacked on this change; cites this commit; touches lustre/tests/sanity-ec.sh (enable_ec, stack_trap, DIR)
- 64086 new LU-12668 lov: handle ESHUTDOWN for LSEEK on EC files stacked on this change
- 65219 new LU-12669 llite: make AIO reads on EC file synchronous stacked on this change; same ticket LU-12669
- 67998 new LU-20566 ec: reuse surviving DIO bounce pages on EC recovery stacked on this change; promised in LU-20566
- 67878 new LU-20566 ec: recover O_DIRECT reads with transient pages stacked on this change; promised in LU-20566
- 66567 new LU-20212 lov: skip invalid OST mirrors for aggregate I/O stacked on this change
- 68278 merged LU-20565 tests: lower required OSTCOUNT for sanity-ec/7d promised in LU-20565
- 68361 new LU-20690 ec: like-RAID10 parity layout so p may exceed k touches lustre/tests/sanity-ec.sh (enable_ec, skip_env, stack_trap)
- 64620 merged LU-19548 lfs: update mirror split for EC support touches lustre/tests/sanity-ec.sh (enable_ec, skip_env, stack_trap)
- 62859 new LU-12668 ec: Add tests for computing the parity coverage touches lustre/tests/sanity-ec.sh (enable_ec, skip_env, stack_trap)
- 68035 merged LU-12668 ec: add --stats and -W into EC resync touches lustre/tests/sanity-ec.sh (enable_ec, skip_env, stack_trap)
- 66436 merged LU-12668 ec: ensure hole ranges for raidset parity touches lustre/tests/sanity-ec.sh (enable_ec, skip_env, stack_trap)
- 65921 new LU-19548 lfs: mirror extend support for EC touches lustre/tests/sanity-ec.sh (enable_ec, stack_trap, DIR)
- 64616 merged LU-19823 lod: Make the stripe allocators failure domain aware. touches lustre/tests/sanity-ec.sh (enable_ec, stack_trap, DIR)
- 66113 merged LU-20036 lfs: add --ec-map getstripe output for EC raidsets touches lustre/tests/sanity-ec.sh (enable_ec, skip_env, DIR)
- 67613 new LU-19994 ec: block LCME_FL_PARITY flag changes touches lustre/tests/sanity-ec.sh (enable_ec, stack_trap, DIR)
- 62868 new LU-12187 llite: enable FLR EC support unconditionally touches lustre/tests/sanity-ec.sh (stack_trap, LFS)
- 65798 new LU-20238 ec: GFNI-accelerated EC primitives touches lustre/tests/sanity-ec.sh (skip_env)
- 66672 merged LU-12668 ec: allow SEEK_DATA/HOLE on parity mirror lseek touches lustre/tests/sanity-ec.sh
- 64441 new LU-12187 lov: MDS layout pattern negotiation touches lustre/tests/sanity-ec.sh
Thread · 2 messages
Code at the comment
4025: test_40a() {
4026: (( $OSTCOUNT >= 3 )) || skip_env "needs >= 3 OSTs for EC rebuild"
4027:
4028: local tf=$DIR/$tfile
4029: local dstripe=$((OSTCOUNT > 4 ? OSTCOUNT - 2 : 2))
>>> 4030: local cstripe=$((OSTCOUNT - dstripe >= 2 ? 2 : 1))
4031:
4032: stack_trap "rm -f $tf"
4033: enable_ec
4034: # Create a simple EC file with data mirror + parity mirror
4035: $LFS setstripe -E -1 -S 1M -c $dstripe --ec $dstripe+$cstripe $tf ||
In flight 3
In flight Track FLR-EC DIO+AIO reconstruction work via filed critical ticket LU-20566 for 2.18.0 Andreas Dilger lustre/llite/file.c:2213 PS92 med +1 repeat in flight: 67878
Kept in change 67878 — LU-20566 ec: recover O_DIRECT reads with transient pages (NEW), with 67998 and AIO handled in 65219 (NEW)
The ticket was filed, but no commit on master names LU-20566 (tickets/LU-20566.txt). master/lustre/llite/file.c:2222-2225 still says 'AIO EC recovery is not yet implemented.' Candidate 67878 (NEW, LU-20566) rewrites that comment and adds DIO recovery using transient pages, but its commit message says 'AIO EC recovery remains unsupported.' The AIO part is only in 65219 (NEW, LU-12669), which makes AIO reads on EC files run synchronously so they can be recovered (65219.diff:68-92). 67998 (NEW, LU-20566) reuses surviving DIO bounce pages. None of these has merged.
Suggested action: Land 67878/67998 under LU-20566 for DIO EC recovery. Also land 65219, or equivalent AIO recovery, and link it to LU-20566 so the ticket covers both DIO and AIO before 2.18.0.
“I filed LU-20566 "FLR-EC: recover data from parity code for DIO+AIO" for this, and Keguang has already pushed a patch for it.”Andreas Dilger, PS92
Possible follow-ups — found in Gerrit, not confirmed
Also raised in 1 other thread — Both note the same LU-20566 ticket filed to track FLR-EC DIO+AIO recovery, from the same PS92 discussion
(change-level) PS92 Andreas Dilger: This looks like it is doing **much** better than the previous versions. It passed **all** of the normal review test sessions on the first t
Thread · 2 messages
Code at the comment
2208: * from parity (buffered IO).
2209: *
2210: * This only handles sync DIO (!is_aio). For true
2211: * AIO, the completion fires externally via cl_dio_aio_end
2212: * after the syscall returns, so we cannot restart the
>>> 2213: * IO from that context. AIO EC recovery is not yet
2214: * implemented.
2215: */
2216: if (rc && iot == CIT_READ && io->ci_cross_ec &&
2217: !io->ci_switch_ec_io) {
2218: CDEBUG(D_VFSTRACE,
In flight Clear cp_sync_io on all exit paths (addressed in follow-up change) Marc Vef lustre/lov/lov_io.c:2168 PS112 low in flight: 67878
Kept in change 67878 — lov_ec_submit_transient() clears cp_sync_io on the qin, qout and submit-failure paths and replaces the inline block in lov_ec_read_parity_stripe()
Not fixed in the current patchset or on master. change/lustre/lov/lov_io.c:2167 and master/lustre/lov/lov_io.c:2167 still have `out_clpages[j]->cp_sync_io = &anchor;`, and nothing in that function clears it afterwards. The fix is only in candidates/67878.diff:462-493, which is still NEW (unmerged). It adds lov_ec_submit_transient(), which sets cp_sync_io to NULL for pages left in qin before cl_sync_io_note (line 480), for qout pages after cl_sync_io_wait (line 485), and for qin pages when submit fails (line 489). Hunk 931-960 in the same diff removes the old inline block and calls the new helper instead. The review comment was correct: the old code left a stale pointer to the on-stack anchor on the pages. It is minor because nothing reads that pointer today.
Suggested action: Land 67878 (lov_ec_submit_transient). If it stalls, clear cp_sync_io on every qin, qout and error path in lov_ec_read_parity_stripe(), the way cl_io_submit_sync() does.
“Addressed in followup: 67878 (LU-20566, higher in the stack) replaces the block with lov_ec_submit_transient, which clears it on every path.”Marc Vef, PS112
Possible follow-ups — found in Gerrit, not confirmed
- 67878 new LU-20566 ec: recover O_DIRECT reads with transient pages linked in the thread
Thread · 2 messages
Code at the comment
2163: */
2164: queue = &lov_env_info(sub->sub_env)->lti_cl2q;
2165: cl_2queue_init(queue);
2166: for (j = 0; j < npages; j++) {
2167: out_clpages[j]->cp_sync_io = &anchor;
>>> 2168: cl_page_list_add(&queue->c2_qin, out_clpages[j], false);
2169: }
2170:
2171: cl_sync_io_init(&anchor, npages);
2172: rc = cl_io_submit_rw(sub->sub_env, &sub->sub_io, CRT_READ, queue);
2173: if (rc == 0) {
In flight Address remaining EC/test issues in later follow-up patches Marc Vef (change-level) PS110 low +1 repeat in flight: 64921
Kept in change 64921 — Several open changes stacked on this one fix EC recovery and tests: 64921 (EC failure orchestrator tests), 68339, 66481, 68453, 65219, 67878/67998. None is merged.
The promise doesn't name the issues. It only says they are 'in the EC part or test related', and the comments it refers to aren't in this thread, so I can't check each one. What I can see: all the follow-ups that fit are still open (status NEW) and stacked on 62757. 68339 serializes concurrent EC recovery readers, 66481 fixes EC recovery reads for encrypted files, 68453 adds lock placeholders for rebuilt pages, 65219 makes AIO reads on EC files synchronous, 67878 and 67998 fix O_DIRECT recovery, and 64921 adds the EC failure test orchestrator. The stacked changes that did merge (63809, 66113, 64620) are about the stripe allocator and lfs, not these EC/test fixes. The author says he is tracking the comments himself, so I can't confirm the open patches cover every item.
Suggested action: Write down the unresolved PS110 review comments and link each one to its follow-up (64921, 68339, 66481, 68453, 65219, 67878). Then land those changes, or file an LU ticket for any comment that no follow-up covers.
“There are some issues that are addressed in later patches (some are already inflight).”Marc Vef, PS110
Also raised in 1 other thread — Same PS110 blanket promise to fix remaining test/EC issues in later follow-up patches
(change-level) PS110 Andreas Dilger: A *bunch* of cleanups for the test scripts, but if these are all passing now then I'm content to fix them up in a separate patch.
Thread · 1 message
To check 0
None.
Kept 21
Kept Fix LU-19536 (change 64359) which removed code needed for EC transient-page IO, causing crashes, instead of this patch Marc Vef (change-level) PS110 high kept in 64359
Kept in change 64359 — 64359 was reworked (Marc Vef co-signed) to keep the CPT_TRANSIENT branch in osc_io_submit() that skips cl_page_prep(), and merged in that form
The merged 64359 (commit 7d10878c40f4) no longer removes the branch in osc_io_submit(). It renames the local 'dio' to 'transient' and keeps skipping cl_page_prep() for transient pages. Its commit message says it keeps this handling because transient pages must not be passed to cl_page_prep(). On master, master/lustre/osc/osc_io.c:143-151 sets transient=true for CPT_TRANSIENT pages, and lines 175-193 skip cl_page_prep() and the async-flag setup for them. That was the code whose removal caused the ASSERTION(cp->cp_type != CPT_TRANSIENT) LBUG in sanity-ec 40a (cl_page_prep via lov_ec_read_parity_stripe). The merged 64359 still drops the transient special case in lov_io_submit() and the oe_csd anchor setup in osc_queue_sync_pages(). In the crash trace, parity reads go through cl_io_submit_rw straight into osc_io_submit and do not pass through lov_io_submit. I did not check the latest sanity-ec test results.
“Therefore, LU-19536 osc: remove dead DIO handling from sync path | https://review.whamcloud.com/c/fs/lustre-release/+/64359 breaks EC and should not land and should be updated to not remove code that is needed by another patch. I will update 64359 accordingly.”Marc Vef, PS110
Possible follow-ups — found in Gerrit, not confirmed
Thread · 4 messages
Kept Fix or file tickets for the mmap-read hang and the intermittent wrong-data reconstruction bugs in the disabled EC subtests Gerrit AI review for Lustre lustre/tests/sanity-ec.sh:29 PS108 high
Kept in patchset 113 — The shared LU-12668 exclusion was split into two new tickets: 41d is now under LU-20708 and 53a under LU-20709
At PS108, sanity-ec.sh:29 had `always_except LU-12668 41d 53a`, and LU-12668 covers neither bug. In change/lustre/tests/sanity-ec.sh:30-31 (PS113), and the same lines in master/lustre/tests/sanity-ec.sh, this became `always_except LU-20708 41d` and `always_except LU-20709 53a`, so each bug now has its own ticket. The item asked to 'fix or file tickets', and filing tickets meets that. The stronger request, fixing 53a's wrong-data reconstruction before landing, was not met. The change merged with 53a still excluded, and the comment at master sanity-ec.sh:25-28 still describes the intermittent wrong-data bug. No candidate follow-up re-enables 41d or 53a: only 68543 and 68554 mention them, and both still carry the old LU-12668 exclusion.
Suggested action: Fix the non-zero-offset reconstruction bug (LU-20709) and the OSC resend loop that hangs the mmap read (LU-20708), then remove the always_except lines for 53a and 41d from sanity-ec.sh.
“Could each get its own open LU ticket, and could the 53a wrong-data case be resolved rather than excluded before this lands?”Gerrit AI review for Lustre, PS108
Thread · 2 messages
Code at the comment
24: # than failing, so it stays off until the retry loop is fixed.
25: # tests 53a: a recovery read at a non-zero offset intermittently
26: # reconstructs wrong data. Measured at 2 failures in 10 runs, and it
27: # still fails 1 in 10 with the proactive dead-OST detection applied, so
28: # it is not the reconstruction bug the other offset tests hit (LU-12668)
>>> 29: always_except LU-12668 41d 53a
30:
31: # aio EC recovery is not working yet
32: always_except LU-12669 41j 41k 41l 41m 41n
33:
34: if [[ "$ost1_FSTYPE" == "zfs" ]]; then
Kept Add bounds check/LASSERT on k against MAX_OST_UNAVAIL to prevent stack buffer overflow Zhenyu Xu lustre/lov/lov_io.c:1867 PS40 high
Kept in patchset 113 — The stack array indexed by k is gone. Recovery arrays are now heap-allocated to LOV_EC_MAX_CODING_STRIPES, with an explicit bounds check and a LASSERT.
In PS113 the recovery path from PS40 (err_array[k] = i on a stack buffer sized MAX_OST_UNAVAIL) was rewritten, and MAX_OST_UNAVAIL no longer exists in change/. change/lustre/lov/lov_io.c:2518-2520 allocates ecr_err_array with OBD_ALLOC at LOV_EC_MAX_CODING_STRIPES entries. The comment at 2491 says parity_used gets the same size. When err_array is filled (lov_io.c:2990-2995), the loop stops once err_nr >= LOV_EC_MAX_CODING_STRIPES or err_nr > pcount, and lov_io.c:3005 then fails the read with -EIO. When the same array is reused for each page, lov_io.c:2626-2628 asserts LASSERTF(r_j < pcount) before writing ecr_err_array[r_j] and ecr_recov_ptrs[r_j]. This covers what Andreas asked for: a check, plus one constant used for both the array size and the limit. One small point remains: the array size of 2 * LOV_EC_MAX_CODING_STRIPES + 1 entries, at the fill loop's worst case of err_nr == pcount == LOV_EC_MAX_CODING_STRIPES, only fits if pcount is at most LOV_EC_MAX_CODING_STRIPES. I did not find where that is enforced.
“Acknowledged”Zhenyu Xu, PS40
Thread · 3 messages
Code at the comment
1862: /* error reading the page */
1863: lov_io_sub_rid(env, lio, sub);
1864: r0->lo_sub[stripe]->lso_status = LSS_READ_ERR;
1865:
1866: page->cp_sync_io = NULL;
>>> 1867: err_array[k] = i;
1868: recov_ptrs[k] =
1869: kmap_local_page(page->cp_vmpage);
1870: k++;
1871: /* transfer page to io->u.ci_ec.ec_page_list
1872: * so that vvp_io_ec_rd_end() can get the
Kept Fix reference leak during read reconstruction causing client OOM Marc Vef (change-level) PS69 high
Kept in patchset 113 — The merged read-reconstruction path now drops the extra page-cache (vmpage) reference and releases cl_page refs on every error path.
The thread does not include the leak description or the proposed fix, so I can't match this against one specific diff. The merged patchset does fix the obvious leak candidates. change/lustre/lov/lov_io.c:1990-1991 has the comment "drop vmpage refcount added by get_folio_grab()" followed by folio_put(_folio) after cl_page_find/cl_page_assume. Without that, every page grabbed for EC reconstruction would stay pinned and could exhaust client memory. The out_err path around line 2065 disowns the page and calls cl_page_put for UPTODATE and READ pages. The parity-stripe error path around line 2188 clears out_clpages after cl_2queue_fini, which avoids both a double put and a leak. The same code is on master in commit 783c13bb0b8f (history lines 2564-2565 and 2636-2656). Marc Vef, who raised the problem, is listed as Reviewed-by on that merged commit, and the thread is marked resolved.
Thread · 1 message
Kept Verify that NULL cp_vmpage on parity pages is safe through all downstream cl_page code paths Patrick Farrell lustre/obdclass/cl_page.c:375 PS40 high
Kept in patchset 113 — Parity pages now get a real vmpage from alloc_page(GFP_NOFS) instead of NULL, so cp_vmpage is never NULL
In change/lustre/lov/lov_io.c:2140-2148, the only EC caller allocates and locks a real page (vmpage = alloc_page(GFP_NOFS); lock_page) and passes it to cl_page_alloc_sub(..., vmpage, CPT_TRANSIENT). The only other caller, cl_page_alloc at obdclass/cl_page.c:423, passes through its own vmpage. No call site passes NULL any more, so the audit the comment asked for (checking cp_vmpage users such as osc_cache.c:2486 page_folio(), osc_io.c:466 and lov_page.c:200 kmap) is no longer needed. master/ matches: same code at lov_io.c:2146.
“Is this safe through all the downstream cl_page paths, or are there places that assume cp_vmpage is non-NULL?”Patrick Farrell, PS40
Thread · 1 message
Code at the comment
370: }
371:
372: return cl_page;
373: }
374:
>>> 375: struct cl_page *cl_page_alloc_sub(const struct lu_env *env,
376: const struct lu_env *subenv,
377: struct cl_object *o,
378: struct cl_object *subobj,
379: pgoff_t ind, struct page *vmpage,
380: enum cl_page_type type)
Kept EAGAIN path can silently return EOF instead of error on unreadable stripe Zhenyu Xu lustre/lov/lov_io.c:2858 PS99 med
Kept in patchset 113 — CIT_EC_RD in lov_io_mirror_init() jumps to ndelay_retry, so ci_ndelay_tried goes up and ci_tried_all_mirrors is eventually set; cl_io_loop() then turns -EAGAIN into -EIO
At PS113, change/lustre/lov/lov_io.c:3010-3011 still returns rc = -EAGAIN when err_nr > pcount. The reviewer's premise was that CIT_EC_RD returns early from lov_io_mirror_init() and never counts retries. That premise does not hold now. At lov_io.c:561-564, CIT_EC_RD does `GOTO(ndelay_retry, result)`, which reaches lov_io.c:792-818. On each attempt that code increments ci_ndelay_tried. Once ci_ndelay_tried % (lo_mirror_count*4) == 0, it sets ci_tried_all_mirrors (lov_io.c:803-804). The count survives restarts: llite/file.c:2303 saves it with `retried = io->ci_ndelay_tried` and file.c:2152 puts it back. After a bounded number of restarts, far fewer than RETRY_ATTEMPTS, cl_io_loop() at obdclass/cl_io.c:875-881 takes the else branch and sets result = -EIO. That value passes through the rc check at file.c:2293 without triggering a restart and reaches userspace. So read(2) now gets -EIO, not EOF, which matches Zhenyu Xu's reply. The mapping-changed -EAGAIN goes through the same cl_io_loop path. The change is merged, so master has this code too. I could not tell from here whether the early-return concern was valid at PS99.
“I disagree, return -EAGAIN and cl_io_loop would set ci_need_restart and set result to 0...”Zhenyu Xu, PS99
Thread · 2 messages
Code at the comment
2853: CWARN("%s: "DFID": too many unrecoverable stripes in raid set %d: err_nr %d > pcount %d: rc = %d\n",
2854: lov2obd(dev->ld_lov)->obd_name,
2855: PFID(lu_object_fid(lov2lu(lov))),
2856: raid_set, err_nr, pcount, -EIO);
2857: io->u.ci_ec.ec_recovery_failed = true;
>>> 2858: GOTO(rg_cleanup, rc = -EAGAIN);
2859: }
2860:
2861: /* Batch parity reads if needed. par_vmpages/par_clpages
2862: * indexed as [parity_stripe * npages + page_pos].
2863: */
Kept Fix lov_io_rw_iter_init() LASSERTF that never actually validates CIT_EC_RD iter_init path Zhenyu Xu COMMIT_MSG:48 PS97 med
Kept in patchset 113 — The widened LASSERTF was dropped. lov_io_rw_iter_init() now asserts only CIT_READ or CIT_WRITE.
In change/lustre/lov/lov_io.c:1328, lov_io_rw_iter_init() now has LASSERT(io->ci_type == CIT_READ || io->ci_type == CIT_WRITE), so CIT_EC_RD is no longer in the assertion. lov_io_ops[CIT_EC_RD].cio_iter_init is still lov_io_ec_rd_iter_init (change/lustre/lov/lov_io.c:3838-3840), which means the tightened assert is correct. master/lustre/lov/lov_io.c:1328 has the same line.
“Acknowledged”Zhenyu Xu, PS97
Thread · 2 messages
Kept Fix or document that true AIO O_DIRECT reads on EC files get no reconstruction at all Zhenyu Xu lustre/llite/file.c:2182 PS78 med
Kept in patchset 113 — The misleading comment was rewritten to say that true AIO EC recovery is not implemented and only sync DIO is handled.
At PS78 the comment claimed that EC information was passed to the AIO structure and that recovery was queued from cl_dio_aio_end. In change/lustre/llite/file.c:2222-2225 (PS113) it now reads: 'This only handles sync DIO (!is_aio). For true AIO, the completion fires externally via cl_dio_aio_end after the syscall returns, so we cannot restart the IO from that context. AIO EC recovery is not yet implemented.' The block is still under `if (io->ci_dio_aio && !is_aio)` (file.c:2199). The reviewer asked to fix or document the gap, and the documentation option was taken. The gap itself is still there in the merged code. Follow-up 65219 (NEW, 'make AIO reads on EC file synchronous') closes it by turning EC AIO reads into sync DIO (aio_ec_sync), so they can be reconstructed.
Suggested action: Land 65219 (or an equivalent) so that true AIO O_DIRECT reads on EC files are actually rebuilt from parity.
“Acknowledged”Zhenyu Xu, PS78
Thread · 2 messages
Code at the comment
2177: *
2178: * For sync DIO (!is_aio), we handle recovery inline
2179: * by restarting the I/O. For true AIO, the completion
2180: * fires externally via cl_dio_aio_end after the syscall
2181: * returns. EC information is propagated to the AIO
>>> 2182: * structure, and recovery is handled asynchronously
2183: * in the completion callback by queuing work to perform
2184: * CIT_EC_RD and populate the page cache.
2185: */
2186: if (rc && iot == CIT_READ && io->ci_cross_ec &&
2187: !io->ci_switch_ec_io) {
Kept Add cio_fini for CIT_EC_RD so ci_restore_needed / layout restore actually run Zhenyu Xu lustre/llite/vvp_io.c:1877 PS78 med
Kept in patchset 113 — CIT_EC_RD io ops now set .cio_fini = vvp_io_fini
At PS78 the CIT_EC_RD entry had no .cio_fini. In change/lustre/llite/vvp_io.c:1980-1987 at PS113 it now has `.cio_fini = vvp_io_fini`, along with new cio_iter_init and cio_lock hooks. master/lustre/llite/vvp_io.c:1980-1981 has the same code. vvp_io_fini (vvp_io.c:315-345) reads ci_restore_needed, calls ll_layout_restore() and re-checks the layout version. So the `case CIT_EC_RD: io->ci_restore_needed = 1;` in lov_io.c:4125-4126 is no longer dead code.
“Acknowledged”Zhenyu Xu, PS78
Thread · 2 messages
Code at the comment
1872: .cio_lock = vvp_io_write_lock,
1873: .cio_start = vvp_io_write_start,
1874: .cio_end = vvp_io_write_end,
1875: .cio_advance = vvp_io_advance,
1876: },
>>> 1877: [CIT_EC_RD] = {
1878: .cio_start = vvp_io_ec_rd_start,
1879: .cio_end = vvp_io_ec_rd_end,
1880: .cio_advance = vvp_io_advance,
1881: },
1882: [CIT_SETATTR] = {
Kept Fix wrong stripe index computation for multi-raid-set EC layouts (parity_foff % pcount) Zhenyu Xu lustre/lov/lov_io.c:2116 PS78 med
Kept in patchset 113 — The stripe is now computed modulo r0->lo_nr, which is the parity component's lsme_stripe_count, instead of pcount.
At change/lustre/lov/lov_io.c:2107, r0 is set to lov_r0(lov, pcomp_i). Line 2123 now reads `stripe = (start_stripe + parity_idx) % r0->lo_nr;`. change/lustre/lov/lov_object.c:187 sets r0->lo_nr = lse->lsme_stripe_count, so this matches the reviewer's first suggested fix. The reviewer's second suggestion is also partly there: line 2121 computes p_foff = parity_foff + parity_idx * lsme_stripe_size, and line 2124 passes it to lov_stripe_offset. The old `% pcount` from PS78 line 2116 is gone.
“good catch, thank you.”Zhenyu Xu, PS78
Thread · 2 messages
Code at the comment
2111: int j;
2112: int rc = 0;
2113:
2114: ENTRY;
2115: start_stripe = lov_stripe_number(lov->lo_lsm, pcomp_i, parity_foff);
>>> 2116: stripe = (start_stripe + parity_idx) % pcount;
2117:
2118: if (unlikely(!lov_r0(lov, pcomp_i)->lo_sub[stripe]))
2119: RETURN(-EIO);
2120:
2121: sub = lov_sub_get(env, lio, lov_comp_index(pcomp_i, stripe));
Kept Fix wrong error code -EINVAL to -EIO (or fallback) for unavailable EC data OST Zhenyu Xu lustre/lov/lov_io.c:379 PS15 med
Kept in patchset 113 — The CIT_EC_RD branch that finds no updated parity component now returns -EIO instead of -EINVAL, and the warning includes the rc.
In change/lustre/lov/lov_io.c:565-570, when lov_find_comp_with_ec() fails, the code sets result = -EIO, logs "no updated parity component to restore data: rc = %d" and returns result. At PS15 this was RETURN(-EINVAL) (line 379). This is the -EIO fix the reviewer asked for. The reviewer's other option, falling back to waiting for the data OST, was not done. The comment offered either option, so -EIO is enough.
“Acknowledged”Zhenyu Xu, PS15
Thread · 2 messages
Code at the comment
374: RETURN(0);
375: } else {
376: CWARN("%s: "DFID": no updated parity component to restore data\n",
377: lov2obd(lov_object_dev(obj)->ld_lov)->obd_name,
378: PFID(lu_object_fid(lov2lu(obj))));
>>> 379: RETURN(-EINVAL);
380: }
381: }
382:
383: if (!lov_is_flr(obj)) {
384: /* only locks/pages are manipulated for CIT_MISC op, no
Kept Fix uninitialized 'raid_set' symbol in lov_io_ec_rd_start Misc Code Checks Robot (Gatekeeper helper) lustre/lov/lov_io.c:2138 PS27 med +8 repeats
Kept in patchset 113 — In lov_io_ec_rd_start, raid_set is now declared as 'unsigned int raid_set = 0;', so it is always initialized.
In change/lustre/lov/lov_io.c:2754 (inside lov_io_ec_rd_start, which begins at master/lustre/lov/lov_io.c:2732), raid_set is declared with an initial value of 0. It is then given a value at lines 2837/2841 before the recovery-group loop at 2892 increments it. master/lustre/lov/lov_io.c:2754 has the same code. The robot warning from PS19–PS27 no longer applies.
“error: lov_io_ec_rd_start():uninitialized symbol 'raid_set'.”Misc Code Checks Robot (Gatekeeper helper), PS27
Also raised in 8 other threads — Same recurring static-analysis error: uninitialized symbol 'raid_set' in lov_io_ec_rd_start, reappearing every patchset until fixed
lustre/lov/lov_io.c:2120 PS19 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():uninitialized symbol 'raid_set'.
lustre/lov/lov_io.c:2117 PS20 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():uninitialized symbol 'raid_set'.
lustre/lov/lov_io.c:2117 PS21 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():uninitialized symbol 'raid_set'.
lustre/lov/lov_io.c:2183 PS22 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():uninitialized symbol 'raid_set'.
lustre/lov/lov_io.c:2120 PS23 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():uninitialized symbol 'raid_set'.
lustre/lov/lov_io.c:2127 PS24 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():uninitialized symbol 'raid_set'.
lustre/lov/lov_io.c:2120 PS25 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():uninitialized symbol 'raid_set'.
lustre/lov/lov_io.c:2120 PS26 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():uninitialized symbol 'raid_set'.
Thread · 1 message
Code at the comment
2133: CDEBUG(D_INFO, DFID": ec read %d unavailable stripes (of dcount %d), has %d parity stripes, from %llu to %llu, recovery group size %lu, pgs_per_stripe %d\n",
2134: PFID(lu_object_fid(lov2lu(lov))), err_nr, dcount, pcount,
2135: lio->lis_pos, lio->lis_pos + size, RGs, pgs_per_stripe);
2136:
2137: /* read size is within the raid sets of the same EC parameters */
>>> 2138: for (i = 0; i < (size + RGs - 1) / RGs; i++, raid_set++) {
2139: unsigned long long soff; /* start offset of a recovery group */
2140: unsigned long long eoff; /* end offset of a recovery group */
2141: int pg_seq_start;
2142: int pg_seq_end;
2143:
Kept Verify the assumption that the data component is always one before the EC component is safe Andreas Dilger (change-level) PS16 med
Kept in patchset 113 — Parity is now matched to its data component by mirror_link_id and an exact extent match, not by the component that sits just before it.
In change/lustre/lov/lov_io.c at PS113, lov_find_comp_with_ec() (lines 333-396) finds the data mirror from parity_lle->lle_lsme->lsme_mirror_link_id through lov_mirror_index_by_id(). If that lookup fails it logs a CERROR and returns false. lov_io_parity_eof() (lines 474-533) looks up the data mirror the same way (lov_mirror_by_id). It then calls lov_io_find_data_comp() (lines 448-461), which walks the data mirror for the component whose extent exactly equals the parity extent (lu_extent_is_equal). If none matches it logs a CERROR and skips that parity component. I found no 'index - 1' or 'lle - 1' access that pairs data with parity in lov/ (a grep for these patterns found none). On the MDS side, lod_object.c:6445-6455 sets llc_mirror_link_id on both the data and the parity components, so the pairing is written into the layout. The only 'index - 1' uses left in lod_object.c (lines 7226-7524) are ordinary PFL/SEL previous-component handling, not EC pairing. The adjacency assumption Andreas worried about at PS16 is gone. This rests on the current PS113 code; I did not trace the lookups in the recovery read path beyond this.
Thread · 1 message
Kept Clarify/verify EC reconstruction handles multiple simultaneous OST/stripe failures and document reconstruction-trigger timeout logic Andreas Dilger (change-level) PS15 med
Kept in patchset 113 — Merged commit 783c13bb0b8f explains how whole-RG recovery works and what triggers it, and the code handles up to pcount failures while skipping parity stripes that cannot be read
All four questions are covered in the merged patchset. (1) Page-at-a-time or whole stripe: the commit message of 783c13bb (history/0931272e_08e8154c.txt:886-900) says the inner EC read IO grows to cover the whole raid set, and that lov_ec_read_stripe_pages() and lov_ec_read_parity_stripe() read every page of a stripe in one submit call. So it reconstructs a whole RG, not one page at a time. (2) Several data stripes missing: change/lustre/lov/lov_io.c:2978-2996 counts every failed stripe in err_array and only gives up when err_nr > pcount (lines 3005-3011, warning 'too many unrecoverable stripes'). The decode matrix is built for unavail_nr erasures (lines 2368-2409). (3) One data stripe and one parity stripe missing: the parity read loop at lov_io.c:3041-3049 skips any parity stripe that fails to read ('skip, try next') and records the ones that worked in ecr_parity_used[]. The matrix inversion uses those recorded rows (lines 2381-2390), so a second parity stripe is used when the first is unavailable. (4) What starts recovery: the commit message says recovery starts 'When normal read fails', including DIO read failures. The switch is in lov_io.c, where ci_cross_ec sets ci_switch_ec_io and restarts the IO. There is no timeout-based trigger, and the change/lustre/lov grep found no timeout logic for this, so the requested explanation of the trigger is given. The thread is resolved and the commit message was rewritten to cover these points.
Thread · 1 message
Kept fix RAID-set math for short RAID sets Zhenyu Xu lustre/lov/lov_lock.c:172 PS2 med
Kept in patchset 113 — The parity-lock extent math was removed from lov_lock.c. Raid-set boundaries now come from ec_split_stripes(), which handles two raid-set sizes (k0 and k1), so short raid sets are covered.
At PS2, lov_lock.c:167-177 computed the parity lock range as stripe_size * dstripe_count. That assumes every raid set has the same size, which is what Patrick flagged. At PS113, lov_lock.c no longer has any LCT_PARITY or RGs math. Only the CIT_EC_RD lock-failure handling is left (lines 291-308). The EC range now expands to raid-set boundaries in change/lustre/lov/lov_object.c:2361-2460. That code calls ec_split_stripes(stripe_count, dstripe_count, &sc) and handles the full k0-sized sets (esc_k0*esc_n0) separately from the shorter k1 sets (esc_k1, esc_n1). It does this for both the start and the end of the range. lov_io.c:418-435 and 2822-2841 count raid sets as esc_n0 + esc_n1 in the same way. Zhenyu's promise to rework all the EC placement math is met by the current patchset, which is merged.
“ah right, I'd change all that involves the EC placement.”Zhenyu Xu, PS2
Thread · 3 messages
Code at the comment
167: if (comp_type == LCT_PARITY) {
168: unsigned long RGs;
169:
170: /* calculate parity lock extent range */
171: RGs = lle->lle_lsme->lsme_stripe_size *
>>> 172: lle->lle_lsme->lsme_dstripe_count;
173: start = ext.e_start - lle->lle_extent->e_start;
174: div64_u64(start, RGs);
175: start *= lle->lle_lsme->lsme_stripe_size;
176:
177: if (ext.e_end > lle->lle_extent->e_end) {
Kept Handle possible NULL 'ptrs' pointer in lov_io_ec_rd_start Misc Code Checks Robot (Gatekeeper helper) lustre/lov/lov_io.c:2029 PS4 med +3 repeats
Kept in patchset 113 — The cleanup path in lov_io_ec_rd_start now checks state.ecr_ptrs and state.ecr_recov_ptrs for NULL before indexing them.
At PS4 the out: loop indexed ptrs[k] with no NULL check on ptrs itself (lov_io.c:2029). At PS113, change/lustre/lov/lov_io.c:3181-3189 wraps both unmap loops in 'if (state.ecr_recov_ptrs)' and 'if (state.ecr_ptrs)'. Those pointers are set to NULL at 2508-2509 before allocation at 2528-2535. Any failure before allocation jumps to out_ptrs/out (3193-3195), which skips the loops, and the loops also check each slot for NULL.
“error: lov_io_ec_rd_start():we previously assumed 'ptrs' could be null (see line 1898)”Misc Code Checks Robot (Gatekeeper helper), PS4
Also raised in 3 other threads — Same static-analysis warning: possible NULL 'ptrs' pointer in lov_io_ec_rd_start, recurring across patchsets
lustre/lov/lov_io.c:1940 PS1 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():we previously assumed 'ptrs' could be null (see line 1845)
lustre/lov/lov_io.c:1940 PS2 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():we previously assumed 'ptrs' could be null (see line 1845)
lustre/lov/lov_io.c:2029 PS3 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():we previously assumed 'ptrs' could be null (see line 1898)
Thread · 1 message
Code at the comment
2024: } /* for each page in a stripe */
2025: } /* for each recovery group */
2026:
2027: out:
2028: for (k = 0; k < max_t(int, dcount, err_nr); k++) {
>>> 2029: if (k < dcount && ptrs[k] && ptrs[k] != zerobuf)
2030: kunmap_atomic(ptrs[k]);
2031: if (k < err_nr && recov_ptrs[k])
2032: kunmap_atomic(recov_ptrs[k]);
2033: }
2034: if (anchor)
Kept Handle possible NULL 'recov_ptrs' pointer Misc Code Checks Robot (Gatekeeper helper) lustre/lov/lov_io.c:2031 PS4 med +3 repeats
Kept in patchset 113 — The cleanup path now checks that state.ecr_recov_ptrs is non-NULL before it walks the array and unmaps entries.
In change/lustre/lov/lov_io.c:3181-3184 of PS113, the out_err cleanup does `if (state.ecr_recov_ptrs) for (k = pcount-1..0) if (state.ecr_recov_ptrs[k]) kunmap_local(...)`. That array is only allocated when pcount > 0 (lines 2533-2535) and is reset to NULL otherwise (lines 2475 and 2509). ecr_ptrs gets the same NULL check at line 3185. The pattern the robot flagged at PS3/PS4 line 2031 was `recov_ptrs[k]` being read with no NULL check on the array itself, and that code is gone after the refactor into lov_ec_recover_page_pos() and struct ecr state.
“error: lov_io_ec_rd_start():we previously assumed 'recov_ptrs' could be null (see line 1916)”Misc Code Checks Robot (Gatekeeper helper), PS4
Also raised in 3 other threads — Same static-analysis warning: possible NULL 'recov_ptrs' pointer in lov_io_ec_rd_start, recurring across patchsets
lustre/lov/lov_io.c:1942 PS1 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():we previously assumed 'recov_ptrs' could be null (see line 1848)
lustre/lov/lov_io.c:1942 PS2 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():we previously assumed 'recov_ptrs' could be null (see line 1848)
lustre/lov/lov_io.c:2031 PS3 Misc Code Checks Robot (Gatekeeper helper): error: lov_io_ec_rd_start():we previously assumed 'recov_ptrs' could be null (see line 1916)
Thread · 1 message
Code at the comment
2026:
2027: out:
2028: for (k = 0; k < max_t(int, dcount, err_nr); k++) {
2029: if (k < dcount && ptrs[k] && ptrs[k] != zerobuf)
2030: kunmap_atomic(ptrs[k]);
>>> 2031: if (k < err_nr && recov_ptrs[k])
2032: kunmap_atomic(recov_ptrs[k]);
2033: }
2034: if (anchor)
2035: OBD_FREE(anchor, sizeof(*anchor) * q_a_count);
2036: if (queue)
Kept Use separate tracking ticket(s) for excepted/skipped EC tests instead of the main LU-12668 tracker Andreas Dilger lustre/tests/sanity-ec.sh:29 PS110 low
Kept in patchset 113 — The excepted tests now use their own tickets: 12a→LU-19631, 41d→LU-20708, 53a→LU-20709; none use LU-12668 anymore.
In PS110, change/lustre/tests/sanity-ec.sh:29 was 'always_except LU-12668 41d 53a'. At PS113, lines 29-31 read 'always_except LU-19631 12a', 'always_except LU-20708 41d' and 'always_except LU-20709 53a'. The file's other always_except lines use LU-20566 (line 34) and LU-20435 (line 39). The only LU-12668 left in the file is in the comment at line 28, and it is not an except entry. master/lustre/tests/sanity-ec.sh has the same lines 29-39, so this is what landed.
“Please use a separate track ticket (or tickets) for excepted tests”Andreas Dilger, PS110
Thread · 2 messages
Code at the comment
24: # than failing, so it stays off until the retry loop is fixed.
25: # tests 53a: a recovery read at a non-zero offset intermittently
26: # reconstructs wrong data. Measured at 2 failures in 10 runs, and it
27: # still fails 1 in 10 with the proactive dead-OST detection applied, so
28: # it is not the reconstruction bug the other offset tests hit (LU-12668)
>>> 29: always_except LU-12668 41d 53a
30:
31: # aio EC recovery is not working yet
32: always_except LU-20566 41j 41k 41l 41m 41n
33:
34: if [[ "$ost1_FSTYPE" == "zfs" ]]; then
Kept Point excluded-test comments at LU-20566 (AIO EC recovery gap) instead of LU-12669 Gerrit AI review for Lustre lustre/tests/sanity-ec.sh:32 PS108 low
Kept in patchset 113 — sanity-ec.sh now uses always_except LU-20566 for 41j-41n; the change is merged to master
In change/lustre/tests/sanity-ec.sh:33-34 and master/lustre/tests/sanity-ec.sh:33-34 the exclusion now reads '# aio EC recovery is not working yet' followed by 'always_except LU-20566 41j 41k 41l 41m 41n'. On PS108 it was LU-12669. The review's aside about ll_file_io_generic() was only partly acted on. The comment at change/lustre/llite/file.c:2222-2225 still says 'AIO EC recovery is not yet implemented' and names no ticket, which is also true on master. That is a small gap. The main item, the ticket used by the test exclusions, is done.
Suggested action: Optionally add '(LU-20566)' to the 'AIO EC recovery is not yet implemented' comment in ll_file_io_generic() in lustre/llite/file.c.
“LU-20566 "FLR-EC: recover data from parity code for DIO+AIO" was filed for exactly this gap and is still Open -- would that be the better reference here”Gerrit AI review for Lustre, PS108
Thread · 2 messages
Code at the comment
27: # still fails 1 in 10 with the proactive dead-OST detection applied, so
28: # it is not the reconstruction bug the other offset tests hit (LU-12668)
29: always_except LU-12668 41d 53a
30:
31: # aio EC recovery is not working yet
>>> 32: always_except LU-12669 41j 41k 41l 41m 41n
33:
34: if [[ "$ost1_FSTYPE" == "zfs" ]]; then
35: # test 5b: EC resync hole-punches stale parity, but osd-zfs has no
36: # fallocate hole-punch, so resync fails with EOPNOTSUPP (LU-20435)
37: always_except LU-20435 5b 12b
Kept Avoid unbounded stack usage for large dcount arrays Zhenyu Xu lustre/lov/lov_io.c:2135 PS40 low
Kept in patchset 113 — The stack array err_array[MAX_OST_UNAVAIL] and all arrays sized by dcount now live in heap buffers allocated by lov_ec_bufs_alloc().
In change/lustre/lov/lov_io.c at PS113, the local array `int err_array[MAX_OST_UNAVAIL]` that was at PS40 line 2135 is gone. err_array is now the heap field state->ecr_err_array, allocated with OBD_ALLOC(sizeof * LOV_EC_MAX_CODING_STRIPES) in lov_ec_bufs_alloc() (lov_io.c:2518-2521) and freed at 2469-2470. Other arrays follow the same pattern: ecr_ptrs[dcount] (2528), ecr_recov_ptrs[pcount] (2533), encode_matrix (2543) and the ecr_parity_used array (2523) all use OBD_ALLOC. The per-page arrays all_pages and all_states (2949-2950) and par_vmpages and par_clpages (3026-3027) use OBD_ALLOC_PTR_ARRAY_LARGE. A search for stack arrays sized by dcount or MAX_OST_UNAVAIL finds nothing. This goes further than the reviewer's suggestion of a small default on the stack plus a heap fallback: stack use is now fixed no matter how large dcount is.
“Acknowledged”Zhenyu Xu, PS40
Thread · 2 messages
Code at the comment
2130: int stripe;
2131:
2132: /* error array index, don't expect more than 64 OSTs unavailable for
2133: * read
2134: */
>>> 2135: int err_array[MAX_OST_UNAVAIL];
2136: struct ec_split_comp sc = { 0 };
2137: int dcount; /* erasure parameters: data stripe count */
2138: int pcount; /* erasure parameters: parity stripe count */
2139: int raid_set = 0; /* raid set number we are reading from */
2140: unsigned long ss; /* stripe size */
Kept Remove redundant vvp_io_set_range() call/field already set in vvp_io_init Zhenyu Xu lustre/llite/vvp_io.c:1944 PS9 low
Kept in patchset 113 — vvp_io_set_range() was removed; vui_tot_bytes is now set only in vvp_io_init, including for CIT_EC_RD
A search for vvp_io_set_range across change/, master/, history/ and candidates/ finds nothing, so the function and all its callers are gone. In change/lustre/llite/vvp_io.c:2045-2058, vvp_io_init now handles CIT_EC_RD next to CIT_READ and CIT_WRITE and sets vio->vui_tot_bytes = io->u.ci_rw.crw_bytes itself. That makes the separate setter unnecessary, which is what Farrell pointed out and Xu agreed to.
“right, could get rid of vvp_io_set_range() then.”Zhenyu Xu, PS9
Thread · 2 messages
Code at the comment
1939: void vvp_io_set_range(const struct lu_env *env, struct cl_object *obj,
1940: struct cl_io *io)
1941: {
1942: struct vvp_io *vio = vvp_env_io(env);
1943:
>>> 1944: vio->vui_tot_bytes = io->u.ci_rw.crw_bytes;
1945: }
1946:
Set aside 1
Not needed Pull the sanity-ec ZFS seek-hole test skip out into a separate patch if the allocator dependency delays landing Marc Vef (change-level) PS82 low
Kept in change 63809 — The raidset-aware stripe allocator patch that carries the ZFS seek-hole skip for sanity-ec is MERGED, so splitting the skip out is moot
The promise only applied if the raid-aware stripe allocator patch held things up. That patch, 63809 ('LU-19852 lod: raidset aware stripe allocator'), is MERGED, and so is 62757 itself, so it never blocked landing. The ZFS lseek skip is on master at master/lustre/tests/sanity-ec.sh:2618-2619 ('[[ "$ost1_FSTYPE" != "zfs" ]] || skip "LU-14217: lseek holes unreliable on ZFS"'), with the same skip at :8999-9000. No separate patch is needed.
“if the raid aware stripe allocator patch is delaying things we should pull that test skip out”Marc Vef, PS82
Thread · 2 messages
Judged not a promise — spot-check them here.
| (change-level) | n/a - suggestion for a test, not agreed to explicitly | Patrick Farrell · resolved | Gerrit ↗ |
| (change-level) | n/a - KASAN crash closed as stale pending re-verification | Timothy Day · resolved | Gerrit ↗ |
| (change-level) | n/a - WIP/CI bot chatter | Patrick Farrell · resolved | Gerrit ↗ |
| (change-level) | n/a - patchset baseline discussion | Zhenyu Xu · resolved | Gerrit ↗ |
| (change-level) | n/a - build vote | Charlie Olmstead · resolved | Gerrit ↗ |
| (change-level) | n/a - praise comment | Andreas Dilger · resolved | Gerrit ↗ |
| (change-level) | n/a - patchset management note | Zhenyu Xu · resolved | Gerrit ↗ |
| (change-level) | n/a - build vote | Andreas Dilger · resolved | Gerrit ↗ |
| (change-level) | n/a - CI test failure chatter, attributed to prior patch | Marc Vef · resolved | Gerrit ↗ |
| (change-level) | n/a - NULL ptr deref already fixed | Marc Vef · resolved | Gerrit ↗ |
| (change-level) | n/a - intro to pasted AI comments | kg.xu · resolved | Gerrit ↗ |
| (change-level) | n/a - unaccepted design suggestion | Qian Yingjin · resolved | Gerrit ↗ |
| (change-level) | n/a - administrative note transferring comments | Marc Vef · resolved | Gerrit ↗ |
| (change-level) | n/a - crash attribution discussion resolved as stale comment | Zhenyu Xu · resolved | Gerrit ↗ |
| (change-level) | n/a - build vote | Marc Vef · resolved | Gerrit ↗ |
| (change-level) | n/a - rebase suggestion, superseded later | Andreas Dilger · resolved | Gerrit ↗ |
| (change-level) | n/a - question about stale parent patch | Andreas Dilger · resolved | Gerrit ↗ |
| (change-level) | n/a - build vote | Charlie Olmstead · resolved | Gerrit ↗ |
| (change-level) | n/a - general status/vote comment | Andreas Dilger · resolved | Gerrit ↗ |
| (change-level) | n/a - rebase rationale explanation | Marc Vef · resolved | Gerrit ↗ |
| (change-level) | n/a - EOF/fallocate test issues fixed in later patchset | Oleg Drokin · resolved | Gerrit ↗ |
| (change-level) | (the classifier returned nothing for this thread) | Marc Vef · resolved | Gerrit ↗ |
| (change-level) | n/a - test comment | Marc Vef · resolved | Gerrit ↗ |
| (change-level) | N/A - meta discussion on test cleanup status | Andreas Dilger · resolved | Gerrit ↗ |
| COMMIT_MSG:7 | n/a - intentional NULL-safe cleanup explained | Marc Vef · resolved | Gerrit ↗ |
| COMMIT_MSG:13 | n/a - wording nit acknowledged | Patrick Farrell · resolved | Gerrit ↗ |
| COMMIT_MSG:18 | n/a - commit message style nit acknowledged | Patrick Farrell · resolved | Gerrit ↗ |
| COMMIT_MSG:29 | n/a - commit message updated to cover hunks | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| COMMIT_MSG:31 | n/a - typo fixed | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| COMMIT_MSG:34 | n/a - commit message clarity acknowledged, no code change needed | Marc Vef · resolved | Gerrit ↗ |
| COMMIT_MSG:35 | n/a - Fixes tag suggestion acknowledged | Marc Vef · resolved | Gerrit ↗ |
| COMMIT_MSG:42 | n/a - typo fixed | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| COMMIT_MSG:50 | n/a - informational note on test exceptions | Zhenyu Xu · resolved | Gerrit ↗ |
| include/uapi/linux/lustre/lustre_user.h:827 | n/a - refactor done | Marc Vef · resolved | Gerrit ↗ |
| include/uapi/linux/lustre/lustre_user.h:851 | n/a - misleading comment wording acknowledged, not a bug | Marc Vef · resolved | Gerrit ↗ |
| lustre/include/cl_object.h:448 | n/a - minor coverage note acknowledged | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/include/cl_object.h:1125 | n/a - style question, done | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/include/cl_object.h:1126 | n/a - typo acknowledged | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/include/cl_object.h:1133 | n/a - style change done | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/include/cl_object.h:1135 | n/a - bitfield packing suggestion acknowledged | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/include/cl_object.h:1681 | n/a - stale comment fixed | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/include/cl_object.h:1836 | n/a - typos fixed | Marc Vef · resolved | Gerrit ↗ |
| lustre/include/cl_object.h:1838 | n/a - typo acknowledged | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/include/cl_object.h:2023 | n/a - comment clarified, done | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/include/cl_object.h:2066 | n/a - comment corrected, done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/file.c:2099 | n/a - stale line refs fixed in this patch | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/file.c:2133 | n/a - iot accounting bug fixed | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/file.c:2144 | n/a - fragile ci_switch_ec_io read, ultimately fixed | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/file.c:2227 | (minor) io->ci_dio_aio still points at the freed aio after this - only the local is cleared. | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/rw.c:1670 | (minor) io_start_index/io_end_index are only consumed by the | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/rw.c:2058 | (typo) stray `*` after "locks." in the comment. | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_internal.h:254 | vvp_io_set_range() has no definition anywhere in the tree and nothing references it, so this declaration is dead. It looks like the leftover of the earlier disc | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:490 | (defect) These descriptors are for the user buffer's file, not for the EC object being reconstructed - cld_obj is ll_i2info(inode)->lli_clob of vma->vm_file bel | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1446 | Is `mbo_size` usable here without looking at `body->mbo_valid`? | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1456 | [minor] If these are direct IO/transient pages, we do not need to lock them since they are not accessible by others anyway | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1482 | cl_page_list_disown() | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1482 | (defect) A negative return from generic_file_read_iter() looks like it is dropped on the floor here. | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1490 | Can you explain what this is doing here in ec read end? I'm a little confused by the flow | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1504 | lli_trunc_sem is only held around generic_file_read_iter() here. For CIT_READ it is taken in vvp_io_read_start() and dropped in vvp_io_rw_end(), so it covers th | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1507 | (style) This isn't a bug today, but the guard reads as if IOCB_DIRECT might be absent, and llite already uses it unconditionally - file.c:2096, file.c:2126, fil | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1513 | (defect) vvp_io_ec_rd_start() takes lli_trunc_sem for read, but this early return skips the matching trunc_sem_up_read() further down. | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1896 | This slot has no cio_lock, so vvp_io_read_lock() -> vvp_io_rw_lock() -> vvp_mmap_locks() never runs for an EC recovery read. lov_io_lock() replaces the extent l | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1907 | (style) This isn't a bug, but the function uses RETURN() without a matching ENTRY, so the D_TRACE entry/exit pair is unbalanced. The other new CIT_EC_RD handler | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1931 | (style) Extra blank line before the closing brace. | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1943 | Do we need this check? Shouldn't this not be called if !CIT_EC_RD? | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1947 | (defect) vvp_io_rw_lock() also runs vvp_io_update_iov(), i.e. iov_iter_truncate(vui_iter, io->u.ci_rw.crw_bytes). This slot only takes the mmap locks, so vui_it | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/llite/vvp_io.c:1990 | style | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:242 | (defect) Caching the VFS inode in lov_object and using it from lov crosses the client layering boundary: only llite is supposed to touch inode/page-cache object | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:383 | (style) Not a bug in the current callers, but returning 0 for a non-FLR object conflates a valid mirror index with the -ENOENT returned below for "not found". l | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:399 | Do we need all of these for EC? | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:425 | [Sashiko AI Review] [Low] lso_status is initialized implicitly to 0 (LSS_OK) by the object allocator, is that right? Might be worth a comment to make that assum | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:646 | (minor) data_mirror_index is not range-checked before it reaches lov_mirror_entry(), which only asserts the upper bound. | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:650 | (minor) This takes the link id from the mirror's first component (lre_start) whatever lio->lis_pos is. The parity->data direction does the opposite: lov_find_co | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:668 | (style) Not a bug, but there is a stray line continuation after the closing paren of the for(), so the blank line below is swallowed into the macro body. Droppi | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:792 | Wondering - why do we need these? | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:814 | Does this need to be a macro, or could it be a static inline function (better type checking, avoid variable re-evaluation, etc.)? | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:816 | (style) single space between variable type and name for local declarations | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:831 | (style) this should be declared above `LOV_2DISPATCH_MAYLOCK()` that is calling it? | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_cl_internal.h:841 | (style) the `/**` comment format should only be used by kerneldoc-style comments | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:351 | (defect?) I don't think this is a *requirement*, it is just commonly the case when the parity mirror is created at the same time as the data mirror. However, i | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:385 | (style) this is a plain `return true` in a function that uses ENTRY/RETURN everywhere else, so the exit trace is skipped on the success path. RETURN(true)? | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:565 | (typo) "crosses", "erasure" | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:566 | (style) `index` is still the -ENOENT initialiser on the common path here: lov_find_comp_with_ec() returns false without touching *mirror_index when no parity co | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:568 | (defect) The failure branch above now only warns; it used to `RETURN(-EIO)`. With `result` initialised to 0 this falls through ndelay_tried and returns 0, so th | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:801 | **[Marc Bot - AI review - opus]** | Marc Vef · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1009 | (minor) lov_io_sub_rid() is hard to read - "rid" does not say what it does. Something like lov_io_sub_release() or lov_io_sub_remove() would sort with the other | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1230 | (style) "nr" is not a very descriptive name. Is this the number of data stripes, the number of *missing* data stripes, something else? | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1280 | If there isn't one already, | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1429 | (defect) Is ec_inode_size populated yet at this point? | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1462 | (style) The comment says "beyond EOF", but the test is "does not intersect this cycle's data_ext". In a component whose stripes span more than one raid set, the | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1519 | **[Marc Bot - AI review - opus]** | Marc Vef · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1593 | (minor) Does this hold when a component contains more than one raid set per row? | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1596 | Is `sc` guaranteed to be non-zero by the time it reaches here? | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1668 | --ec_map 1+1 sets lsme_dstripe_count == 1, so this skips `ec_split_stripes()` and leaves sc = {0}. That gets passed to `lov_ecio_add_parity_sub()`, where `dswid | kg.xu · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1688 | (defect) lsme_stripe_size is u32 and lsme_stripe_count is u16, so this product is evaluated as unsigned int and wraps before div64_u64() widens it. This patch n | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1695 | `cnt` is u16 and `lsme_stripe_size` is u32, so `cnt * lsme->lsme_stripe_size` is 32-bit unsigned arithmetic. | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1726 | warning: sparse:symbol 'lov_alloc_sub_page' was not declared. Should it be static? | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1797 | The CIT_EC_RD extent lock is built here from scratch, so it loses the group-lock handling every other read path gets from vvp_io_one_lock_index(): | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1852 | (minor) `enum ec_data_pg_state` sits between this kernel-doc block and lov_ec_read_stripe_pages(), so the comment attaches to the enum rather than the function. | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1853 | error: lov_io_ec_rd_start():uninitialized symbol 'lsme'. | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1853 | error: lov_io_ec_rd_start():uninitialized symbol 'lsme'. | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1862 | [Sashiko AI Review] [Critical] On the read_err path, if we got here because lov_sub_get() returned an ERR_PTR, we'd be passing that error pointer into lov_io_su | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1868 | With `LOV_EC_MAX_CODING_STRIPES = 15` is there a reason why this needs to be 64? If there are more than 15 stripes missing in a single RAID Set then the data w | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1879 | (minor) `dcount` is never read in lov_ec_read_stripe_pages(); the stripe is derived from lov_stripe_number() + stripe_i instead. lov_ec_read_parity_stripe() has | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1908 | Fix page ref leak for EC_DPG_READ/ERROR pages | Marc Vef · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1940 | Guard get_folio_grab() result with IS_ERR_OR_NULL | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1955 | Fix degraded-read regression by updating ec_inode_size on EOF-overlapping IO | Maximilian Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1976 | Use cl_object_top(obj) for cl_page_alloc/cl_page_alloc_sub | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:1986 | Fix out-of-bounds access to non-array lti_cl2q via queue[i] | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2032 | Pack function arguments to fewer lines within 80 cols | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2060 | Shorten comment to one line | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2065 | Reword and shorten comment | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2069 | Shorten comment wording | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2069 | Fix remaining page-ref leak on error path (put_page for out_pages) | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2074 | Complexity warning: function too hairy to analyze | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2074 | Complexity warning: function too hairy to analyze | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2074 | Complexity warning: function too hairy to analyze | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2074 | Complexity warning: function too hairy to analyze | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2075 | Shorten comment wording | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2076 | Complexity warning: function too hairy to analyze | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2076 | Complexity warning: function too hairy to analyze | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2076 | Complexity warning: function too hairy to analyze | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2078 | Complexity warning: function too hairy to analyze | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2078 | Complexity warning: function too hairy to analyze | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2085 | Fix double page unlock in cleanup path | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2099 | Remove unneeded NULL check before OBD_FREE() | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2167 | Add ': rc = %d' to console message | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2169 | Wait unconditionally and clear cp_sync_io on leftover c2_qin pages | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2169 | Guard against pl_nr != 0 skipping cl_sync_io_wait | kg.xu · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2239 | Add ': rc = %d' to console message | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2274 | Add ': rc = %d' checkpatch console message format | wc-checkpatch · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2285 | Add ': rc = %d' to console message | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2345 | Add ': rc = %d' to console message | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2348 | Avoid hardcoding magic number in comment | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2371 | Remove guesswork for unavailable OST count vs EC stripes | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2400 | Fix infinite loop from missing crw_pos/crw_bytes advance on early return | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2412 | Free only buffers allocated in error path (avoid dangling erc_tbls) | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2427 | Use u64 for 'row' to avoid wraparound on large offsets | kg.xu · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2503 | Split cleanup into multiple labeled goto targets | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2534 | Fix component-wide vs per-raid-set failure count comparison | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2552 | Avoid sleeping allocation while holding kmap_local_page mappings | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2554 | Complexity warning: function too hairy to analyze | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2585 | Use DIV_ROUND_UP for pg_seq_end instead of floor shift | kg.xu · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2591 | Fix PAGE_SHIFT/PAGE_SIZE divisor mixup causing wrong npages | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2602 | static analysis complexity warning, no action | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2608 | static analysis complexity warning, no action | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2608 | static analysis complexity warning, no action | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2626 | fix bounds check ordering, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2635 | fix shared erc_tbls buffer causing wrong reconstruction, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2645 | fix err_array overflow bound check, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2663 | fix off-by-one in err_nr bound check, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2666 | NULL parity slots after unmap, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2669 | reuse existing constant for parity stripe bound, marked Done | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2669 | unwind kmap_local in reverse order, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2679 | set rc on ENOMEM before goto rg_cleanup, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2775 | reduce per-page stack usage, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2777 | remove dead NULL check, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2786 | set ci_continue=0 at EOF, fixed | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2793 | fix pg_seq_start/end window computation, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2865 | add retry/fallback for EIO bail-out, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2892 | use div64_u64 for 64-bit division, refuted as irrelevant for supported archs | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2914 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2914 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2918 | static analysis complexity warning, no reply | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2918 | static analysis complexity warning, no reply | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2941 | static analysis complexity warning, no reply | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2941 | static analysis complexity warning, no reply | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2941 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2942 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2942 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2942 | initialize stack vars defensively, suggestion not a current bug | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2945 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2947 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2947 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2949 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2949 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2949 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2949 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2953 | split large function into helpers, marked Done | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2953 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2957 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2964 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2964 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2970 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:2976 | static analysis complexity warning, acknowledged only | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:3066 | null-pointer analysis warning, no reply | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:3066 | null-pointer analysis warning, no reply | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:3075 | static analysis complexity warning, no reply | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:3075 | static analysis complexity warning, no reply | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:3086 | static analysis complexity warning, no reply | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:3109 | reconstructed pages lack DLM lock, explained/verified as intentional | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_io.c:3980 | add lower bound check on mirror_index, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_lock.c:136 | question on cld_heed_error scope, unanswered question | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/lov/lov_lock.c:143 | typo fix, marked Done | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/lov/lov_lock.c:183 | fix unsynchronized lso_status re-read race, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_lock.c:211 | fix sub-lock cleanup break vs continue leak, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_object.c:1066 | fix cat_size monotonic non-decreasing bug, marked Done | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_object.c:2279 | sparse label warning, no reply | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_object.c:2284 | sparse label warning, no reply | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_object.c:2284 | n/a | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/lov/lov_object.c:2337 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_object.c:2344 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_object.c:2369 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/lov/lov_object.c:2394 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/obdclass/cl_io.c:221 | n/a | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/obdclass/cl_io.c:221 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/obdclass/cl_io.c:222 | n/a | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/obdclass/cl_io.c:222 | n/a | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/obdclass/cl_io.c:224 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/obdclass/cl_io.c:466 | n/a | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/obdclass/cl_io.c:865 | n/a | Misc Code Checks Robot (Gatekeeper helper) · resolved | Gerrit ↗ |
| lustre/obdclass/cl_io.c:1205 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/obdclass/cl_io.c:1207 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/obdclass/cl_page.c:347 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/tests/llapi_layout_test.c:3740 | n/a | Patrick Farrell · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:40 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:118 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:2779 | n/a | Zhenyu Xu · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:3999 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:3999 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4006 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4016 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4017 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4027 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4029 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4035 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4044 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4049 | n/a | Gerrit AI review for Lustre · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4057 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4108 | n/a | wc-checkpatch · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4132 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4169 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4170 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4177 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4180 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4193 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4198 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4217 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4240 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4328 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4349 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4394 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4398 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4412 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4417 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4418 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4436 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4441 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4442 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4460 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4655 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4729 | n/a | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/tests/sanity-ec.sh:4993 | Fixed: removed unnecessary error numbering | Andreas Dilger · resolved | Gerrit ↗ |
| lustre/utils/lfs.c:2027 | Fixed: removed duplicate error message on error path | Marc Vef · resolved | Gerrit ↗ |
| lustre/utils/lfs.c:2406 | Fixed: removed duplicate error message in mirror extend | Marc Vef · resolved | Gerrit ↗ |
Verdicts are AI output, checked against the commits they cite — verify before acting on one.