Skip to content

obj: make sure footers are written for all huge memory blocks on chunk reinit#24

Merged
janekmi merged 1 commit intomasterfrom
DAOS-18195-reinit-all-footers
Nov 22, 2025
Merged

obj: make sure footers are written for all huge memory blocks on chunk reinit#24
janekmi merged 1 commit intomasterfrom
DAOS-18195-reinit-all-footers

Conversation

@janekmi
Copy link
Contributor

@janekmi janekmi commented Nov 20, 2025

Requires

Issue

Huge memory blocks with the header chunk of type CHUNK_TYPE_FREE had not reinitialized their footer chunk.

Context

  • Allocator divides the available space into zones and zones into smaller chunks.
  • All chunk headers belonging to a single zone are stored in an array.
  • Chunk header describes its: type, size_idx and flags.
  • Chunk can be of a few types. The types important for this patch are: CHUNK_TYPE_FOOTER, CHUNK_TYPE_FREE, and CHUNK_TYPE_USED.
  • CHUNK_TYPE_FREE and _USED are chunk types marking the beginning of so called huge memory block. This means this chunk and a number of chunks following it (size_idx) constitues a single allocation or a free memory.
  • The last chunk belonging to a huge memory block ought to be of type CHUNK_TYPE_FOOTER. Its size_idx allows to easily find the first chunk belonging to this huge memory block and determine the huge memory block type.
  • Huge memory blocks' footers are written immediately but persited lazily. It is not a problem at runtime since the footers are there at runtime. But in case of a crash the footers may be not persisted properly and missing on open. So, when the allocator is booted up it recreates footers just in case any of them is missing.

Note: The huge memory block's first chunk header and the last chunk header (footer) are not written nor persisted in any way transactionally. The first chunk header occupies only 8 bytes so it is written and persisted atomically. But its footer is written independently and not explicitly persisted.

Note: The first chunk header contains all the required info to recreate footer. The last chunk header is there only to make huge memory blocks coalescing easier to compute. Reading the footer allows to immediately find the memory block leaving just before the memory block which neighbours one might want to find.

This patch makes sure the huge memory block's footer is recreated no matter whather its type is CHUNK_TYPE_FREE or CHUNK_TYPE_USED.

The patch is inspired by work done for DAOS' DAV allocator (DAOS-18195) which is heavily based on the PMEMOBJ allocator.

Inspired-by: Sherin T George sherin-t.george@hpe.com


This change is Reviewable

@janekmi janekmi requested review from grom72 and osalyk November 20, 2025 20:09
@janekmi janekmi force-pushed the DAOS-18195-reinit-all-footers branch from 97e659c to 5d27fac Compare November 20, 2025 20:23
Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grom72 reviewed 1 of 2 files at r1.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @osalyk)


a discussion (no related file):
Why do you want this change merged into stable-2.1 and not to master?

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @janekmi and @osalyk)


a discussion (no related file):
I think we should explain why we can have the first chunk of type CHUNK_TYPE_FREE.

@osalyk osalyk changed the base branch from stable-2.1 to master November 21, 2025 11:08
@janekmi janekmi force-pushed the DAOS-18195-reinit-all-footers branch from 5d27fac to 8d7a434 Compare November 21, 2025 12:03
Copy link
Contributor Author

@janekmi janekmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @grom72 and @osalyk)


a discussion (no related file):

Previously, grom72 (Tomasz Gromadzki) wrote…

Why do you want this change merged into stable-2.1 and not to master?

Done.


a discussion (no related file):

Previously, grom72 (Tomasz Gromadzki) wrote…

I think we should explain why we can have the first chunk of type CHUNK_TYPE_FREE.

Done.

@janekmi janekmi force-pushed the DAOS-18195-reinit-all-footers branch from 8d7a434 to 54a830b Compare November 21, 2025 12:05
@janekmi janekmi changed the title obj: make sure footers are written for all huge memory blocks on... obj: make sure footers are written for all huge memory blocks on chunk reinit Nov 21, 2025
@janekmi janekmi force-pushed the DAOS-18195-reinit-all-footers branch from 54a830b to 2b6e2a2 Compare November 21, 2025 12:49
Copy link
Contributor

@osalyk osalyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@osalyk reviewed 1 of 2 files at r1, 5 of 5 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72)

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grom72 reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi)


ChangeLog line 5 at r3 (raw file):

	* Version X.X.X

	- Fix a very unlikely issue in the PMEMOBJ allocator with a potential to corrupt the allocator's metadata (daos-stack/pmdk#24, DAOS-18195).

Do we fix something or not?

Suggestion:

Fix an issue 

Copy link
Contributor Author

@janekmi janekmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72)


ChangeLog line 5 at r3 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

Do we fix something or not?

We do. Moreover, “very unlikely,” despite being vague, is a qualifier that gives the reader more insight into the problem. Considering there is no more detailed information on how frequently the problem occurs, it is the most accurate explanation we can provide.

Why exactly do you think we should not provide this information?

... chunk reinit

Issue

Huge memory blocks with the header chunk of type CHUNK_TYPE_FREE had not
reinitialized their footer chunk.

Context

- Allocator divides the available space into zones and zones into
  smaller chunks.
- All chunk headers belonging to a single zone are stored in an array.
- Chunk header describes its: type, size_idx and flags.
- Chunk can be of a few types. The types important for this patch are:
  CHUNK_TYPE_FOOTER, CHUNK_TYPE_FREE, and CHUNK_TYPE_USED.
- CHUNK_TYPE_FREE and _USED are chunk types marking the beginning of so
  called huge memory block. This means this chunk and a number of chunks
  following it (size_idx) constitues a single allocation or a free
  memory.
- The last chunk belonging to a huge memory block ought to be of type
  CHUNK_TYPE_FOOTER. Its size_idx allows to easily find the first chunk
  belonging to this huge memory block and determine the huge memory
  block type.
- Huge memory blocks' footers are written immediately but persited
  lazily. It is not a problem at runtime since the footers are there at
  runtime. But in case of a crash the footers may be not persisted
  properly and missing on open. So, when the allocator is booted up it
  recreates footers just in case any of them is missing.

Note: The huge memory block's first chunk header and the last chunk
header (footer) are not written nor persisted in any way
transactionally. The first chunk header occupies only 8 bytes so it is
written and persisted atomically. But its footer is written
independently and not explicitly persisted.

Note: The first chunk header contains all the required info to recreate
footer. The last chunk header is there only to make huge memory blocks
coalescing easier to compute. Reading the footer allows to immediately
find the memory block leaving just before the memory block which
neighbours one might want to find.

This patch makes sure the huge memory block's footer is recreated no
matter whather its type is CHUNK_TYPE_FREE or CHUNK_TYPE_USED.

The patch is inspired by work done for DAOS' DAV allocator (DAOS-18195)
which is heavily based on the PMEMOBJ allocator.

Signed-off-by: Jan Michalski <jan-marian.michalski@hpe.com>
Inspired-by: Sherin T George <sherin-t.george@hpe.com>
@janekmi janekmi force-pushed the DAOS-18195-reinit-all-footers branch from 2b6e2a2 to f09bc1d Compare November 21, 2025 14:00
Copy link
Contributor Author

@janekmi janekmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion (waiting on @grom72 and @osalyk)


ChangeLog line 5 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

We do. Moreover, “very unlikely,” despite being vague, is a qualifier that gives the reader more insight into the problem. Considering there is no more detailed information on how frequently the problem occurs, it is the most accurate explanation we can provide.

Why exactly do you think we should not provide this information?

We agreed:

  • "very unlikely" does not assume any kind of randomness. Rather it means a number of scenarios causing the problems is relatively small compared to the number of all possible scenarios.
  • We also agreed since these scenarios include power-failures/crash they are very important. Since they constitute the key deliverable of the PMDK libraries as such.

Copy link
Contributor Author

@janekmi janekmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion (waiting on @grom72 and @osalyk)


ChangeLog line 5 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

We agreed:

  • "very unlikely" does not assume any kind of randomness. Rather it means a number of scenarios causing the problems is relatively small compared to the number of all possible scenarios.
  • We also agreed since these scenarios include power-failures/crash they are very important. Since they constitute the key deliverable of the PMDK libraries as such.

Done.

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@grom72 reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi)

Copy link
Contributor Author

@janekmi janekmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@janekmi reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi)

@janekmi janekmi merged commit 7e1db7c into master Nov 22, 2025
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants