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.

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

vabackend: fix alignment of codecs #72

Merged
merged 1 commit into from
Apr 11, 2022
Merged

Conversation

pobrn
Copy link
Contributor

@pobrn pobrn commented Apr 11, 2022

Making the NVCodec structs maximally aligned was an erroneous move because the gcc may increase the alignment which can lead to undesirable behaviour (e.g. #71).

In the aforementioned case the size of the struct was 496 bytes, which was properly aligned to the maximum alignment (__BIGGEST_ALIGNMENT__) on the platform (16 bytes - x86-64), but gcc chose to align the objects at a 32 byte boundary, which resulted in 16 bytes of padding between each element that was not accounted for at runtime.

Specifying the alignment on the object itself and not the type prevents gcc from increasing the alignment.

See: https://lore.kernel.org/lkml/[email protected]/

Fixes: 73ddb63 ("vabackend: make NVCodec struct aligned")

Making the `NVCodec` structs maximally aligned was an erroneous
move because the gcc may increase the alignment which can lead
to undesirable behaviour (e.g. elFarto#71).

In the aforementioned case the size of the struct was 496 bytes,
which was properly aligned to the maximum alignment
(`__BIGGEST_ALIGNMENT__`) on the platform (16 bytes - x86-64),
but gcc chose to align the objects at a 32 byte boundary,
which resulted in 16 bytes of padding between each element
that was not accounted for at runtime.

Specifying the alignment on the object itself and not
the type prevents gcc from increasing the alignment.

See: https://lore.kernel.org/lkml/[email protected]/

Fixes: 73ddb63 ("vabackend: make NVCodec struct aligned")
@pobrn
Copy link
Contributor Author

pobrn commented Apr 11, 2022

Sorry again, I hope this will be last installment of the alignment saga.

@elFarto elFarto merged commit 51aaf5e into elFarto:master Apr 11, 2022
@elFarto
Copy link
Owner

elFarto commented Apr 11, 2022

Thanks for the patch! Let's hope this fixes it once and for all.

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.

2 participants