Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mm: Add pending configuration for mm node struct and precding #15484

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+42 −8
Draft

mm: Add pending configuration for mm node struct and precding #15484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions mm/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,14 @@ config MM_DEFAULT_ALIGNMENT
memory default alignment is equal to sizoef(uintptr), if this value
is not 0, this value must be 2^n and at least sizeof(uintptr).

config MM_NODE_PENDING
bool "Enable pending memory node"
default n
---help---
Pending memory block node precding and the entire struct,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Pending memory block node precding and the entire struct,
Pending memory block node preceding and the entire struct,

After turning on this bit, the size of each memory block
will be aligned to MM_ALIGN

config MM_SMALL
bool "Small memory model"
default n
Expand Down
40 changes: 33 additions & 7 deletions mm/mm_heap/mm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,15 @@
* previous freenode
*/

#define MM_ALLOCNODE_OVERHEAD (MM_SIZEOF_ALLOCNODE - sizeof(mmsize_t))
#ifdef CONFIG_MM_NODE_PENDING
# define MM_NODE_PENDING aligned_data(MM_ALIGN)
# define MMSIZE_T_LEN MM_ALIGN
#else
# define MM_NODE_PENDING
# define MMSIZE_T_LEN sizeof(mmsize_t)
#endif

#define MM_ALLOCNODE_OVERHEAD (MM_SIZEOF_ALLOCNODE - MMSIZE_T_LEN)

/* Get the node size */

Expand Down Expand Up @@ -173,23 +181,41 @@ typedef size_t mmsize_t;

struct mm_allocnode_s
{
mmsize_t preceding; /* Physical preceding chunk size */
mmsize_t size; /* Size of this chunk */
union
{
mmsize_t preceding; /* Physical preceding chunk size */
Copy link
Contributor

Choose a reason for hiding this comment

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

why not simplify to:

mmsize_t preceding aligned_data(MM_ALIGN);

Copy link
Contributor Author

@W-M-R W-M-R Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It is invalid to do aligned_data(MM_ALIGN) on precding alone.

#include <stdio.h>


struct st0
{
    char a;
    char b;
}__attribute__((aligned(16)));

struct st1
{
    char a __attribute__((aligned(16)));
    char b;
};

struct st2
{
    char a;
    char b __attribute__((aligned(16)));
};

struct st3
{
    char a __attribute__((aligned(16)));
    char b __attribute__((aligned(16)));
    char d[15];
}__attribute__((aligned(16)));

struct st4
{
    union
    {
        char a;
        char a_align[16];
    };

    union
    {
        char b;
        char b_align[16];
    };
};

#define OFFSET(struct, member) (&(((struct *)0)->member))

#define PRINTF_STRUCT(struct) \
    printf("%s: ", #struct); \
    printf("size: %d\n", sizeof(struct)); \
    printf("member a: %d\n", OFFSET(struct, a)); \
    printf("member b: %d\n", OFFSET(struct, b));

int main()
{
    PRINTF_STRUCT(struct st0);
    PRINTF_STRUCT(struct st1);
    PRINTF_STRUCT(struct st2);
    PRINTF_STRUCT(struct st3);
    PRINTF_STRUCT(struct st4);
}



struct st0: size: 16
member a: 0
member b: 1
struct st1: size: 16
member a: 0
member b: 1
struct st2: size: 32
member a: 0
member b: 16
struct st3: size: 32
member a: 0
member b: 16
struct st4: size: 32
member a: 0
member b: 16

My understanding is that the alignment of attribute((aligned(16))) is more about the alignment of the size of the entire structure, and the alignment of a single member is limited to the alignment of access, not size.Therefore, using union is the only effective way to align the size

Copy link
Contributor

Choose a reason for hiding this comment

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

but anonymous union doesn't support by all c compiler.

uint8_t preceding_align[MMSIZE_T_LEN];
};
union
{
mmsize_t size; /* Physical preceding chunk size */
uint8_t size_align[MMSIZE_T_LEN];
};

#if CONFIG_MM_BACKTRACE >= 0
pid_t pid; /* The pid for caller */
unsigned long seqno; /* The sequence of memory malloc */
# if CONFIG_MM_BACKTRACE > 0
FAR void *backtrace[CONFIG_MM_BACKTRACE]; /* The backtrace buffer for caller */
# endif
#endif
};
}MM_NODE_PENDING;

/* This describes a free chunk */

struct mm_freenode_s
{
mmsize_t preceding; /* Physical preceding chunk size */
mmsize_t size; /* Size of this chunk */
union
{
mmsize_t preceding; /* Physical preceding chunk size */
W-M-R marked this conversation as resolved.
Show resolved Hide resolved
uint8_t preceding_align[MMSIZE_T_LEN];
};
union
{
mmsize_t size; /* Physical preceding chunk size */
uint8_t size_align[MMSIZE_T_LEN];
};

#if CONFIG_MM_BACKTRACE >= 0
pid_t pid; /* The pid for caller */
unsigned long seqno; /* The sequence of memory malloc */
Expand All @@ -199,7 +225,7 @@ struct mm_freenode_s
#endif
FAR struct mm_freenode_s *flink; /* Supports a doubly linked list */
FAR struct mm_freenode_s *blink;
};
}MM_NODE_PENDING;

static_assert(MM_SIZEOF_ALLOCNODE <= MM_MIN_CHUNK,
"Error size for struct mm_allocnode_s\n");
Expand Down
2 changes: 1 addition & 1 deletion mm/mm_heap/mm_realloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ FAR void *mm_realloc(FAR struct mm_heap_s *heap, FAR void *oldmem,
heap->mm_curused += newsize - oldsize;
mm_shrinkchunk(heap, oldnode, newsize);
kasan_poison((FAR char *)oldnode + MM_SIZEOF_NODE(oldnode) +
sizeof(mmsize_t), oldsize - MM_SIZEOF_NODE(oldnode));
MMSIZE_T_LEN, oldsize - MM_SIZEOF_NODE(oldnode));
}

/* Then return the original address */
Expand Down
Loading