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

nautilus: common/buffer: adjust align before calling posix_memalign() #41246

Merged
merged 2 commits into from Jun 3, 2021

Conversation

idryomov
Copy link
Contributor

posix_memalign() requires alignment argument to be a multiple of
sizeof(void *).  Since it is an implementation detail of buffer,
it needs to be adjusted there -- buffer consumers have no way of
knowing that passing e.g. align == 4 is incorrect.

One place already does the adjustment, but only for align == 0.
The other just asserts.  Fix both and remove the "power of two"
assertion.  Let posix_memalign() return EINVAL and handle that
by throwing buffer::bad_alloc, as expected by the consumers.

Fixes: https://tracker.ceph.com/issues/50646
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit aa31ddf)

Conflicts:
	src/common/buffer.cc [ commit ea32d80 ("common: drop
	  sharing of buffer::raw outside bufferlist.") not in nautilus ]
We want buffer::bad_alloc, not std::bad_alloc.  Otheriwise, we end
up with a confusing error

  failed decoding of frame header: Bad allocation

from ProtocolV2::run_continuation(), printed after frame header is
successfully decoded.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit 67bb6cf)
@idryomov idryomov added bug-fix core common messenger Issues involving one of the Ceph messenger implementations labels May 10, 2021
@idryomov idryomov added this to the nautilus milestone May 10, 2021
@idryomov
Copy link
Contributor Author

jenkins retest this please

@idryomov idryomov requested a review from rzarzynski May 10, 2021 20:28
@jdurgin jdurgin changed the base branch from nautilus to nautilus-saved May 14, 2021 21:58
@jdurgin jdurgin changed the base branch from nautilus-saved to nautilus May 14, 2021 21:58
@tchaikov tchaikov added nautilus-batch-1 nautilus point releases needs-qa labels Jun 2, 2021
Copy link
Member

@ideepika ideepika left a comment

Choose a reason for hiding this comment

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

@yuriw yuriw merged commit 57619d2 into ceph:nautilus Jun 3, 2021
@idryomov idryomov deleted the wip-posix-memalign-fix-nautilus branch July 4, 2022 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix common core messenger Issues involving one of the Ceph messenger implementations nautilus-batch-1 nautilus point releases needs-qa wip-yuri7-testing wip-yuri-testing
Projects
None yet
4 participants