| Message ID | 51560237.2040000@inktank.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Reviewed-by: Josh Durgin <josh.durgin@inktank.com> On 03/29/2013 02:05 PM, Alex Elder wrote: > In ceph_osdc_build_request() there is a call to cpu_to_le16() which > provides a 64-bit value as its argument. Because of the implied > byte swapping going on it looked pretty suspect to me. > > At the moment it turns out the behavior is well defined, but masking > off those bottom bits explicitly eliminates this distraction, and is > in fact more directly related to the purpose of the message header's > data_off field. > > This resolves: > http://tracker.ceph.com/issues/4125 > > Signed-off-by: Alex Elder <elder@inktank.com> > --- > net/ceph/osd_client.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > index 3b6657f..015bf9f 100644 > --- a/net/ceph/osd_client.c > +++ b/net/ceph/osd_client.c > @@ -419,8 +419,18 @@ void ceph_osdc_build_request(struct > ceph_osd_request *req, > p += 4; > > /* data */ > - if (flags & CEPH_OSD_FLAG_WRITE) > - req->r_request->hdr.data_off = cpu_to_le16(off); > + if (flags & CEPH_OSD_FLAG_WRITE) { > + u16 data_off; > + > + /* > + * The header "data_off" is a hint to the receiver > + * allowing it to align received data into its > + * buffers such that there's no need to re-copy > + * it before writing it to disk (direct I/O). > + */ > + data_off = (u16) (off & 0xffff); > + req->r_request->hdr.data_off = cpu_to_le16(data_off); > + } > req->r_request->hdr.data_len = cpu_to_le32(data_len); > > BUG_ON(p > msg->front.iov_base + msg->front.iov_len); > -- 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/net/ceph/osd_client.c b/net/ceph/osd_client.c index 3b6657f..015bf9f 100644 --- a/net/ceph/osd_client.c +++ b/net/ceph/osd_client.c @@ -419,8 +419,18 @@ void ceph_osdc_build_request(struct ceph_osd_request *req, p += 4; /* data */ - if (flags & CEPH_OSD_FLAG_WRITE) - req->r_request->hdr.data_off = cpu_to_le16(off); + if (flags & CEPH_OSD_FLAG_WRITE) { + u16 data_off; + + /* + * The header "data_off" is a hint to the receiver + * allowing it to align received data into its + * buffers such that there's no need to re-copy + * it before writing it to disk (direct I/O). + */ + data_off = (u16) (off & 0xffff); + req->r_request->hdr.data_off = cpu_to_le16(data_off); + } req->r_request->hdr.data_len = cpu_to_le32(data_len);
In ceph_osdc_build_request() there is a call to cpu_to_le16() which provides a 64-bit value as its argument. Because of the implied byte swapping going on it looked pretty suspect to me. At the moment it turns out the behavior is well defined, but masking off those bottom bits explicitly eliminates this distraction, and is in fact more directly related to the purpose of the message header's data_off field. This resolves: http://tracker.ceph.com/issues/4125 Signed-off-by: Alex Elder <elder@inktank.com> --- net/ceph/osd_client.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) BUG_ON(p > msg->front.iov_base + msg->front.iov_len);