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

os/bluestore: s/align_down/p2align/ #29379

Merged
merged 1 commit into from
Aug 2, 2019
Merged

Conversation

tchaikov
Copy link
Contributor

we have two implementations of align_{up,down}(). let's consolidate
them.

  • s/align_up/p2roundup/
  • s/align_down/p2align/
  • drop common/align.h

Signed-off-by: Kefu Chai kchai@redhat.com

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

we have two implementations of align_{up,down}(). let's consolidate
them.

* s/align_up/p2roundup/
* s/align_down/p2align/
* drop common/align.h

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov
Copy link
Contributor Author

but i think align_up() and align_down() are better names than p2roundup() and p2align(). can we rename the latter two ? what do you think?

if yes, i will add a commit in this PR to do this rename.

@xiexingguo
Copy link
Member

I am not sure the re-naming proposal since the p2* wrappers only work well if the "align" is a power of 2, e.g., often times it is a block, sector, or page.

@tchaikov
Copy link
Contributor Author

i see. put in other words, i am using this property of blocksize to replace a more general form of align_{up,down} with their special cases. let's keep it as is then.

@liewegas liewegas merged commit 7326e0d into ceph:master Aug 2, 2019
liewegas added a commit that referenced this pull request Aug 2, 2019
* refs/pull/29379/head:
	os/bluestore: s/align_down/p2align/

Reviewed-by: xie xingguo <xie.xingguo@zte.com.cn>
@tchaikov tchaikov deleted the wip-align-up branch August 2, 2019 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants