Cleanup #4166
closedceph: simplify ceph_sync_write() page_align
0%
Description
(This work is done.)
ceph: simplify ceph_sync_write() page_align calculation
In ceph_sync_write() there is some magic that computes page_align
for an osd request. But a little analysis shows it can be
simplified.
First, we have:
io_align = pos & ~PAGE_MASK;
which is used here:
page_align = (pos - io_align + buf_align) & ~PAGE_MASK;
Note (pos - io_align) simply rounds "pos" down to the nearest multiple
of the page size.
We also have:
buf_align = (unsigned long)data & ~PAGE_MASK;
Adding buf_align to that rounded-down "pos" value will stay within
the same page; the result will just be offset by the page offset for
the "data" pointer. The final mask therefore leaves just the value
of "buf_align".
The same simplification can be done in striped_read().
One more simplification. Note that the result of calc_pages_for()
is invariant of which page the offset starts in--the only thing that
matters is the offset within the starting page. We will have
put the proper page offset to use into "page_align", so just use
that in calculating num_pages.
Signed-off-by: Alex Elder <elder@inktank.com>
Updated by Alex Elder about 11 years ago
The following has been posted for review to resolve
this issue:
[PATCH] ceph: simplify ceph_sync_write() page_align
(Apparently a "cleanup" can't be marked for "Need Review")
Updated by Alex Elder about 11 years ago
- Status changed from In Progress to Resolved
commit 29c3c8b721a96b4a82f4224527c7103e5e910b80
Author: Alex Elder <elder@inktank.com>
Date: Fri Feb 15 22:10:16 2013 -0600
ceph: simplify ceph_sync_write() page_align calculationUpdated by Alex Elder about 11 years ago
This commit caused regressions. I pulled it out and
I'm not going to re-open it again.
Updated by Alex Elder about 11 years ago
- Status changed from Resolved to In Progress
I'm reopening this after all.
It turns out that the original patch was fine. The only part
that was bad was due to this:
The same simplification can be done in striped_read().That was an ill-advised throw-in that caused the problems,
and which made me close this in a fit of shame.
I have added this to the description, to explain why it's
useful and maybe important to include:
(This is being reposted. The first one had a problem because it
erroneously added a similar change elsewhere; that change has been
dropped.)
The next patch in this series points out that the calculation for
the number of pages in an osd request is getting done twice. It
is not obvious, but the result of both calculations is identical.
This patch simplifies one of them--as a separate step--to make
it clear that the transformation in the next patch is valid.
Updated by Alex Elder about 11 years ago
- Status changed from In Progress to 4
This patch (2/3 below) has been posted for review, along with
a few others I include here for context. Marking for "feedback"
for now since I can't mark it "needs review."
[PATCH 0/3] ceph: assign message data fields consistently
[PATCH 1/3] ceph: use calc_pages_for() in start_read()
[PATCH 2/3] ceph: simplify ceph_sync_write() page_align calculation
[PATCH 3/3] libceph: don't assign page info in ceph_osdc_new_request()
Updated by Alex Elder about 11 years ago
- Status changed from 4 to Resolved
This has been committed to the ceph-client testing branch.
038832c ceph: simplify ceph_sync_write() page_align calculation