On Sat, May 30, 2026 at 04:50:02PM +0000, John Groves wrote: > From: John Groves <[email protected]> > > This series applies bug fixes (mostly found via sashiko) to the dax/fsdev > series. This has been soaking in the famfs CI pipeline for 2+ weeks and > 1) won't affect anything that doesn't use drivers/dax/fsdev.c, and 2) > doesn't affect any known workloads - although the bugs would have > manifested when multi-range DCD dax devices are a thing (soon-ish).
Hi John, I'm here to give my Reviewed-by: on all but Patch 7. I'll come back around after you rev for the Patch 7 feedback from DaveJ. I did a DaveJ vs Sashiko comparison. (guessing you may have already done that) DaveJ came out "righter", with a crisper description, suggested fix, and omits the overstated first issue Sashiko made about the unconditional write corrupting state. And since patch 7 shows this set does affect XFS, please update the above cover letter intro to something like: "Most of the series is confined to drivers/dax/fsdev.c. Two patches touch shared DAX core: patch 7 changes fs_put_dax() in drivers/dax/super.c (used by ext2/ext4/erofs/xfs, though only holder-passing callers, like XFS in-tree, see a behavior change), and patch 8 adjusts the dax_dev lookup API in super.c / dax.h." FWIW I'm overwhelmed with the Sashiko pre-existing defect reports. I am hoping that patch authors can read thru those and will call out anything that MUST be fixed for their set, or appear otherwise urgent. -- Alison > > Changes since V2: > > * Patch 1 (comment fix): No change. Responded to Dave's question about > the dropped precondition -- the new comment correctly covers both > callers; fsdev_clear_folio_state() does not guarantee share==0 before > calling, so the old precondition was no longer universally true. > * V2 patch 2 (three fixes): Split into three separate patches (patches > 2-4) per Dave's review. > * V2 patch 3 (two fixes): Split into two separate patches (patches 5-6) > per Dave's review. > * V2 patch 4 (clamp direct_access / remove cached_size): Dropped. > Dave's analysis correctly showed the claimed bug does not exist -- > dax_pgoff_to_phys() already enforces that the full requested size fits > within a single range before returning, making the clamp a no-op in > every reachable path. > * V2 patch 5 (holder_ops race): Use WRITE_ONCE() for the holder_ops > store; add WARN_ON() on the cmpxchg result to catch wrong-holder and > double-put API contract violations; fix the inline comment, which > incorrectly claimed dax_holder_notify_failure() consults holder_ops > only when holder_data is non-NULL. > * V2 patch 6 (dax_dev_find): Add dax_alive() check under dax_read_lock() > after ilookup5() to prevent returning a device that is concurrently > being torn down by kill_dax(). > * V2 patch 7 (formatting cleanup): Drop incorrect Fixes: tag; add > Dave's Reviewed-by. > * The series grows from 7 to 9 patches. > > Changes since v1: > * Dropped modes from patch 6 to fs/fuse/famfs.c and > fs/famfs/famfs_inode.c, which are not upstream so it broke > attempts to apply the series. Oops... > * Added patch 7, which addresses a previously-missed review comment > from Jonathan - minor cleanup > > John Groves (9): > dax: fix misleading comment about share/index union in > dax_folio_reset_order() > dax/fsdev: fix multi-range offset in memory_failure handler > dax/fsdev: clear vmemmap_shift when binding static pgmap > dax/fsdev: clear dev_dax->pgmap on probe failure > dax/fsdev: use __va(phys) for kaddr in direct_access > dax/fsdev: fail probe on invalid pgmap offset > dax: fix holder_ops race in fs_put_dax() > dax: replace exported dax_dev_get() with non-allocating dax_dev_find() > dax: fsdev.c minor formatting cleanup > > drivers/dax/fsdev.c | 81 +++++++++++++++++++++++++++++++-------------- > drivers/dax/super.c | 73 +++++++++++++++++++++++++++++++++++++--- > fs/dax.c | 12 +++---- > include/linux/dax.h | 6 +++- > 4 files changed, 136 insertions(+), 36 deletions(-) > > -- > 2.53.0 >
