On 2/8/2011 3:50 PM, Venkateswararao Jujjuri (JV) wrote:
> On 2/8/2011 1:09 PM, Eric Van Hensbergen wrote:
>> One thing I wonder is if we always want to zero copy for payload. In
>> the extreme, do we want to take the overhead of pinning an extra page
>> if we are only reading/writing a byte? memcpy is expensive for large
>> packets, but may actually be more efficient for small packets.
> I am not a memory expert, but I would assume memcpy also need to do
> same thing similar to get_user_pages() short of pinning pages. But I see the point.
>
>>
>> Have we done any performance measurements of this code with various
>> payload sizes versus non-zero-copy? Of course that may not really
>> show the impact of pinning the extra pages....
>
> All our testing is with large buffers. Did not test with small buffers.
>
>>
>> In any case, if such a tradeoff did exist, we might choose to not do
>> zero copy for requests smaller than some size -- and that might
>> alleviate some of the problems with the legacy protocols (such as the
>> Rerror issue) -- killing two birds with one stone. In any case, given
>
> I think it is a wise decision to avoid zero copy if iosize+hdr_size <= pagesize.
> But It doesn't change any of today's complexity. Except may be saving an if
> condition.
>
>> the implementations it should be really easy to shut me up with
>> comparison data of zc and non-zc for 1, 64, 128, 256, 512, 1024, 2048,
>> 4192 byte payloads (without caches enabled of course).
>
> I think this is good experiment will publish data.
BTW, unless we have bigger msize with differentiating pdu sizes these experiments
may not make sense.
- JV
>
>>
>> -eric
>>
>>
>> On Mon, Feb 7, 2011 at 12:57 AM, Venkateswararao Jujjuri (JV)
>> <jvrao@xxxxxxxxxxxxxxxxxx> wrote:
>>> On 2/6/2011 11:21 PM, Venkateswararao Jujjuri (JV) wrote:
>>>> Signed-off-by: Venkateswararao Jujjuri <jvrao@xxxxxxxxxxxxxxxxxx>
>>>> ---
>>>> net/9p/client.c | 45 +++++++++++++++++++++++++++++++--------------
>>>> net/9p/protocol.c | 38 ++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 69 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/net/9p/client.c b/net/9p/client.c
>>>> index a848bca..f939edf 100644
>>>> --- a/net/9p/client.c
>>>> +++ b/net/9p/client.c
>>>> @@ -1270,7 +1270,14 @@ p9_client_read(struct p9_fid *fid, char *data, char __user *udata, u64 offset,
>>>> if (count < rsize)
>>>> rsize = count;
>>>>
>>>> - req = p9_client_rpc(clnt, P9_TREAD, "dqd", fid->fid, offset, rsize);
>>>> + if ((clnt->trans_mod->pref & P9_TRANS_PREF_PAYLOAD_MASK) ==
>>>> + P9_TRANS_PREF_PAYLOAD_SEP) {
>>>> + req = p9_client_rpc(clnt, P9_TREAD, "dqE", fid->fid, offset,
>>>> + rsize, data ? data : udata);
>>>> + } else {
>>>> + req = p9_client_rpc(clnt, P9_TREAD, "dqd", fid->fid, offset,
>>>> + rsize);
>>>> + }
>>>> if (IS_ERR(req)) {
>>>> err = PTR_ERR(req);
>>>> goto error;
>>>> @@ -1284,13 +1291,15 @@ p9_client_read(struct p9_fid *fid, char *data, char __user *udata, u64 offset,
>>>>
>>>> P9_DPRINTK(P9_DEBUG_9P, "<<< RREAD count %d\n", count);
>>>>
>>>> - if (data) {
>>>> - memmove(data, dataptr, count);
>>>> - } else {
>>>> - err = copy_to_user(udata, dataptr, count);
>>>> - if (err) {
>>>> - err = -EFAULT;
>>>> - goto free_and_error;
>>>> + if (!req->tc->pbuf_size) {
>>>> + if (data) {
>>>> + memmove(data, dataptr, count);
>>>> + } else {
>>>> + err = copy_to_user(udata, dataptr, count);
>>>> + if (err) {
>>>> + err = -EFAULT;
>>>> + goto free_and_error;
>>>> + }
>>>> }
>>>> }
>>>> p9_free_req(clnt, req);
>>>> @@ -1323,12 +1332,20 @@ p9_client_write(struct p9_fid *fid, char *data, const char __user *udata,
>>>>
>>>> if (count < rsize)
>>>> rsize = count;
>>>> - if (data)
>>>> - req = p9_client_rpc(clnt, P9_TWRITE, "dqD", fid->fid, offset,
>>>> - rsize, data);
>>>> - else
>>>> - req = p9_client_rpc(clnt, P9_TWRITE, "dqU", fid->fid, offset,
>>>> - rsize, udata);
>>>> +
>>>> + if ((clnt->trans_mod->pref & P9_TRANS_PREF_PAYLOAD_MASK) ==
>>>> + P9_TRANS_PREF_PAYLOAD_SEP) {
>>>> + req = p9_client_rpc(clnt, P9_TWRITE, "dqF", fid->fid, offset,
>>>> + rsize, data ? data : udata);
>>>> + } else {
>>>> + if (data)
>>>> + req = p9_client_rpc(clnt, P9_TWRITE, "dqD", fid->fid,
>>>> + offset, rsize, data);
>>>> + else
>>>> + req = p9_client_rpc(clnt, P9_TWRITE, "dqU", fid->fid,
>>>> + offset, rsize, udata);
>>>> + }
>>>> +
>>>> if (IS_ERR(req)) {
>>>> err = PTR_ERR(req);
>>>> goto error;
>>>> diff --git a/net/9p/protocol.c b/net/9p/protocol.c
>>>> index dfc358f..ea778dd 100644
>>>> --- a/net/9p/protocol.c
>>>> +++ b/net/9p/protocol.c
>>>> @@ -114,6 +114,24 @@ pdu_write_u(struct p9_fcall *pdu, const char __user *udata, size_t size)
>>>> return size - len;
>>>> }
>>>>
>>>> +static size_t
>>>> +pdu_write_uw(struct p9_fcall *pdu, const char *udata, size_t size)
>>>> +{
>>>> + size_t len = min(pdu->capacity - pdu->size, size);
>>>> + pdu->pbuf = udata;
>>>> + pdu->pbuf_size = len;
>>>> + return size - len;
>>>> +}
>>>> +
>>>> +static size_t
>>>> +pdu_write_ur(struct p9_fcall *pdu, const char *udata, size_t size)
>>>> +{
>>>> + size_t len = min(pdu->capacity - pdu->size, size);
>>>> + pdu->pbuf = udata;
>>>> + pdu->pbuf_size = len;
>>>> + return size - len;
>>>> +}
>>>> +
>>>> /*
>>>> b - int8_t
>>>> w - int16_t
>>>> @@ -445,6 +463,26 @@ p9pdu_vwritef(struct p9_fcall *pdu, int proto_version, const char *fmt,
>>>> errcode = -EFAULT;
>>>> }
>>>> break;
>>>> + case 'E':{
>>>> + int32_t count = va_arg(ap, int32_t);
>>>> + const char *udata = va_arg(ap, const void *);
>>>> + errcode = p9pdu_writef(pdu, proto_version, "d",
>>>> + count);
>>>> + if (!errcode && pdu_write_ur(pdu, udata,
>>>> + count))
>>>> + errcode = -EFAULT;
>>>> + }
>>>> + break;
>>>> + case 'F':{
>>>> + int32_t count = va_arg(ap, int32_t);
>>>> + const char *udata = va_arg(ap, const void *);
>>>> + errcode = p9pdu_writef(pdu, proto_version, "d",
>>>> + count);
>>>> + if (!errcode && pdu_write_uw(pdu, udata,
>>>> + count))
>>>> + errcode = -EFAULT;
>>>> + }
>>>> + break;
>>>> case 'U':{
>>>> int32_t count = va_arg(ap, int32_t);
>>>> const char __user *udata =
>>>
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> The modern datacenter depends on network connectivity to access resources
>>> and provide services. The best practices for maximizing a physical server's
>>> connectivity to a physical network are well understood - see how these
>>> rules translate into the virtual world?
>>> http://p.sf.net/sfu/oracle-sfdevnlfb
>>> _______________________________________________
>>> V9fs-developer mailing list
>>> V9fs-developer@xxxxxxxxxxxxxxxxxxxxx
>>> https://lists.sourceforge.net/lists/listinfo/v9fs-developer
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
