dm vdo: update struct allocation and add flexible-array annotations#94
dm vdo: update struct allocation and add flexible-array annotations#94raeburn merged 3 commits intodm-vdo:mainfrom
Conversation
github-actions
bot
added
private testing running
and removed
private testing running
labels
lorelei-sakai
left a comment
lorelei-sakai
left a comment
There was a problem hiding this comment.
Code merge looks correct to me.
Some questions about commit messages and arrangement.
a) why squash two commits together? They don't really overlap. (I mean it's fine, other than being more of a pain to review, but why did you do it?)
b) Why did you remove the commit message for the second commit (relative to what's in vdo-devel)?
c) Is the main PR description the cover letter draft? I would like to have ticket or PR references in the description. This is effectively a merge, so it would be nice to be able to easily find the original versions. (Cover letter draft can be set off as a separate section.)
That may be fine, but I'd like to get these answers, at least.
All of VDO's "extended" allocations use a flexible array field at the end of the allocated structure. We can infer the struct type from the supplied pointer. Replacing the array field type with the field name lets us use struct_size from overflow.h to compute the size instead of the local __vdo_do_allocation version. One allocation of bio structures doesn't conform to this pattern, since the removal of bi_inline_vecs; directly compute the total size for that case. Signed-off-by: Ken Raeburn <raeburn@redhat.com>
We can infer the type needed from the supplied pointer argument. A couple invocation sites needed fixing to supply the proper type of pointer. Use overflow.h's size_mul, and we can remove the __vdo_do_allocation wrapper which did the same overflow check. Signed-off-by: Ken Raeburn <raeburn@redhat.com>
This attribute allows the compiler to refine compile-time diagnostics and run-time sanitizer features with information about the size of the flexible arrays. Signed-off-by: Ken Raeburn <raeburn@redhat.com>
github-actions
bot
added
private testing running
and removed
private testing running
labels
lorelei-sakai
left a comment
lorelei-sakai
left a comment
There was a problem hiding this comment.
Yes, I think these all look good.
I do think having three commits looks better, too, so changing each macro gets its own patch.
Commit messages edits make sense and the resulting messages look fine. The PR description feels a bit sparse as a cover letter? But it may be fine, too. Anyway cover letter contents don't matter as much as long as you cover the main motivations. So this is good.
I edited the PR description to add the references for which PRs got merged rather than ask you to do it again. Obviously not something for the cover letter.
3 participants
These patches update VDO’s structure management to better align with
other kernel behavior: using overflow.h macros when computing
allocation sizes, and adding __counted_by annotations to structures
where appropriate.
This PR merges commits from vdo-devel PRs 333, 352, and 429.