Re: [PATCH] ntfs: read WOF chunks outside the decompression lock

[Namjae Jeon]

On Fri, Aug 28, 2026 at 1:58 PM Zhan Xusheng <zhanxusheng1024@xxxxxxxxx> wrote:
>
> From: Zhan Xusheng <zhanxusheng1024@xxxxxxxxx>
>
> From: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>
>
> WOF decompression uses four module-global workspaces, one per compression
> format, each with a static mutex.  ntfs_read_wof_compressed_block() takes
> that mutex once and holds it across the whole chunk loop, so both block
> reads run inside it:
>
>   mutex_lock(ws->lock);
>   for each chunk {
>           parse_wof_chunk_table(..., ws->input, ...);   /* reads disk */
>           ntfs_read_wof_chunk(..., ws->input, ...);     /* reads disk */
>           decompress into ws->output;
>   }
>   mutex_unlock(ws->lock);
>
> Readers of system-compressed files then serialise system-wide on the disk
> waits, not just on the decompressor scratch the lock exists for.  One
> reader sleeping in submit_bio_wait() blocks all the rest.
>
> The waits dominate.  Reading an 8 MiB xpress4k file (2048 chunks at a 48%
> compressed ratio, so 2048 acquisitions and 4096 block reads) and timing
> ws->lock against the part of it spent in ntfs_bdev_read():
>
>   backing store                held     of that in I/O   held after
>   virtio, host page cache      348 ms   321 ms  (92%)    24.6 ms
>   virtio, throttled 100 MB/s   978 ms   948 ms  (96%)    36.6 ms
>
> The page-cache row is a lower bound, having no seek cost at all, and the
> share still grows with slower storage because only the wait scales while
> decompression stays near 26 ms.
>
> The reads are inside the lock only because they land in ws->input, a
> buffer shared through the workspace.  Nothing else requires it:
> parse_wof_chunk_table() and ntfs_read_wof_chunk() already take the buffer
> as a parameter and both set *chunk_mem to a pointer inside it, so a
> caller-owned buffer works unchanged.
>
> Allocate that buffer per call, do both reads without the lock, and take
> the lock only around decompression, which is the step needing ws->output
> and ws->scratch.  squashfs is arranged this way already: its
> squashfs_decompress() is handed a bio that has been read, and locks only
> for the CPU work.
>
> Block reads are unchanged in number, they just no longer run under the
> lock, and hold time stops tracking device speed.
>
> This also unnests two per-inode locks from the global one, runlist->lock
> taken by both reads and base_ni->mrec_lock taken for a resident stream.
> A resident chunk needs no I/O at all, yet used to queue behind a reader
> blocked in submit_bio_wait() and then take mrec_lock inside the global
> mutex.
>
> The buffer is 4608 bytes for xpress4k and at most 33280 for lzx32k.  This
> path already does GFP_NOFS allocations per call in ntfs_attr_iget(), and
> in ntfs_attr_get_search_ctx() for a resident stream, so one more does not
> change how it behaves under memory pressure.  The workspace keeps output
> and scratch, 4 KiB to 32 KiB and 6224 bytes (xpress) or 10240 (lzx), and
> its "already allocated" test moves from ws->input to ws->output.
>
> The lock is now taken per chunk rather than per call, which differs only
> for a folio spanning several chunks: a few more uncontended mutex
> operations in exchange for not holding it across the reads between them.
>
> Verified under QEMU against an uncompressed copy of the same data, on an
> 8 MiB file and a 100000 byte one, the latter covering the tail chunk that
> is not a full comp_unit.
>
> Signed-off-by: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>
Applied it to #ntfs-next.
Thanks!