On Tue, 24 Jun 2025 07:33:51 +0000
Dongsheng Yang <dongsheng.yang@xxxxxxxxx> wrote:
> Introduce segment.{c,h}, an internal abstraction that encapsulates
> everything related to a single pcache *segment* (the fixed-size
> allocation unit stored on the cache-device).
>
> * On-disk metadata (`struct pcache_segment_info`)
> - Embedded `struct pcache_meta_header` for CRC/sequence handling.
> - `flags` field encodes a “has-next” bit and a 4-bit *type* class
> (`CACHE_DATA` added as the first type).
>
> * Initialisation
> - `pcache_segment_init()` populates the in-memory
> `struct pcache_segment` from a given segment id, data offset and
> metadata pointer, computing the usable `data_size` and virtual
> address within the DAX mapping.
>
> * IO helpers
> - `segment_copy_to_bio()` / `segment_copy_from_bio()` move data
> between pmem and a bio, using `_copy_mc_to_iter()` and
> `_copy_from_iter_flushcache()` to tolerate hw memory errors and
> ensure durability.
> - `segment_pos_advance()` advances an internal offset while staying
> inside the segment’s data area.
>
> These helpers allow upper layers (cache key management, write-back
> logic, GC, etc.) to treat a segment as a contiguous byte array without
> knowing about DAX mappings or persistence details.
>
> Signed-off-by: Dongsheng Yang <dongsheng.yang@xxxxxxxxx>
Hi
Just one trivial comment.
> diff --git a/drivers/md/dm-pcache/segment.h b/drivers/md/dm-pcache/segment.h
> new file mode 100644
> index 000000000000..9675951ffaf8
> --- /dev/null
> +++ b/drivers/md/dm-pcache/segment.h
> @@ -0,0 +1,73 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef _PCACHE_SEGMENT_H
> +#define _PCACHE_SEGMENT_H
> +
> +#include <linux/bio.h>
> +
> +#include "pcache_internal.h"
> +
> +struct pcache_segment_info {
> + struct pcache_meta_header header; /* Metadata header for the segment */
The comment is fairly obvious given the type of the field. I'd drop the comment.
> + __u32 flags;
> + __u32 next_seg;
> +};
