| Message ID | 511FBCD4.5040709@inktank.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Reviewed-by: Josh Durgin <josh.durgin@inktank.com> On 02/16/2013 09:07 AM, Alex Elder wrote: > 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. > > This resolves: > http://tracker.ceph.com/issues/4166 > > Signed-off-by: Alex Elder <elder@inktank.com> > --- > fs/ceph/file.c | 18 +++++------------- > 1 file changed, 5 insertions(+), 13 deletions(-) > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > index 9c4325e..6123ad4 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -331,10 +331,7 @@ static int striped_read(struct inode *inode, > io_align = off & ~PAGE_MASK; > > more: > - if (o_direct) > - page_align = (pos - io_align + buf_align) & ~PAGE_MASK; > - else > - page_align = pos & ~PAGE_MASK; > + page_align = o_direct ? buf_align : io_align; > this_len = left; > ret = ceph_osdc_readpages(&fsc->client->osdc, ceph_vino(inode), > &ci->i_layout, pos, &this_len, > @@ -526,15 +523,10 @@ more: > io_align = pos & ~PAGE_MASK; > buf_align = (unsigned long)data & ~PAGE_MASK; > len = left; > - if (file->f_flags & O_DIRECT) { > - /* write from beginning of first page, regardless of > - io alignment */ > - page_align = (pos - io_align + buf_align) & ~PAGE_MASK; > - num_pages = calc_pages_for((unsigned long)data, len); > - } else { > - page_align = pos & ~PAGE_MASK; > - num_pages = calc_pages_for(pos, len); > - } > + > + /* write from beginning of first page, regardless of io alignment */ > + page_align = file->f_flags & O_DIRECT ? buf_align : io_align; > + num_pages = calc_pages_for(page_align, len); > req = ceph_osdc_new_request(&fsc->client->osdc, &ci->i_layout, > ceph_vino(inode), pos, &len, > CEPH_OSD_OP_WRITE, flags, > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 9c4325e..6123ad4 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -331,10 +331,7 @@ static int striped_read(struct inode *inode, io_align = off & ~PAGE_MASK; more: - if (o_direct) - page_align = (pos - io_align + buf_align) & ~PAGE_MASK; - else - page_align = pos & ~PAGE_MASK; + page_align = o_direct ? buf_align : io_align; this_len = left; ret = ceph_osdc_readpages(&fsc->client->osdc, ceph_vino(inode), &ci->i_layout, pos, &this_len, @@ -526,15 +523,10 @@ more: io_align = pos & ~PAGE_MASK; buf_align = (unsigned long)data & ~PAGE_MASK; len = left; - if (file->f_flags & O_DIRECT) { - /* write from beginning of first page, regardless of - io alignment */ - page_align = (pos - io_align + buf_align) & ~PAGE_MASK; - num_pages = calc_pages_for((unsigned long)data, len); - } else { - page_align = pos & ~PAGE_MASK; - num_pages = calc_pages_for(pos, len); - } + + /* write from beginning of first page, regardless of io alignment */ + page_align = file->f_flags & O_DIRECT ? buf_align : io_align; + num_pages = calc_pages_for(page_align, len); req = ceph_osdc_new_request(&fsc->client->osdc, &ci->i_layout, ceph_vino(inode), pos, &len, CEPH_OSD_OP_WRITE, flags,
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. This resolves: http://tracker.ceph.com/issues/4166 Signed-off-by: Alex Elder <elder@inktank.com> --- fs/ceph/file.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-)