Could we just not support user's preset rsp buffer for function
rpc_clnt_submit, which means remove some relevant parameters of
function rpc_clnt_submit such like rsphdr, rsphdr_count, rsp_payload,
rsp_payload_count, rsp_iobref.
The reasons as follow:
1.We not only should put rsp info to saved_frame, but also should
duplicate iovec rsp.payload. So we should GF_FREE(
saved_frame.rsp.rsp_payload) before mem_put(saved_frame) . It seem's
ugly.
wind:
saved_frame->rsp = *rsp (this rsp is a pointer to local parameter);
saved_frame->rsp.rsp_payload = iov_dup (rsp.rsp_payload,
rsp.rsp_payload_count);
unwind:
GF_FREE (saved_frame->rsp.rsp_payload);
memput (saved_frame);
2.If we finish the patch as the way mentioned above, and what about
rsphdr ? According to the note of rpc_clnt_submit, the rsp_hdr and
rsp_payload both could preset by users. But we even can't get
saved_frame before we use rsphdr in socket read state machine, because
we need rsphdr's content to get saved_frame.
3.I can't figure out the benefit of presetting rsphdr and rsp_payload.
2016-07-12 12:38 GMT+08:00 Raghavendra Gowdappa <rgowdapp@xxxxxxxxxx>:
>
>
> ----- Original Message -----
>> From: "Zhengping Zhou" <johnzzpcrystal@xxxxxxxxx>
>> To: gluster-devel@xxxxxxxxxxx
>> Sent: Tuesday, July 12, 2016 9:28:01 AM
>> Subject: [puzzle] readv operation allocate iobuf twice
>>
>> Hi all:
>>
>> It is a puzzle to me that we allocate rsp buffers for rspond
>> content in function client3_3_readv, but these rsp parameters hasn't
>> ever been saved to struct saved_frame in submit procedure.
>
> Good catch :). We were aware of this issue, but the fix wasn't prioritized. Can you please file a bug on this? If you want to send a fix (which essentially stores the rsp payload ptr in saved-frame and passes it down during rpc_clnt_fill_request_info - as part of handling RPC_TRANSPORT_MAP_XID_REQUEST event in rpc-clnt), please post a patch to gerrit and I'll accept it. If you don't have bandwidth, one of us can send out a fix too.
>
> Again, thanks for the effort :).
>
> regards,
> Raghavendra
>
>> Which means
>> the iobuf will reallocated by transport layer in function
>> __socket_read_accepted_successful_reply.
>> According to the commnet of fucntion rpc_clnt_submit :
>> 1. Both @rsp_hdr and @rsp_payload are optional.
>> 2. The user of rpc_clnt_submit, if wants response hdr and payload in its
>> own
>> buffers, then it has to populate @rsphdr and @rsp_payload.
>> ....
>> The rsp_payload is optional, ransport layer will not reallocate
>> rsp buffers if
>> it populated. But the fact is readv operation will allocate rsp buffer twice.
>>
>> Thanks
>> Zhengping
>> _______________________________________________
>> Gluster-devel mailing list
>> Gluster-devel@xxxxxxxxxxx
>> http://www.gluster.org/mailman/listinfo/gluster-devel
>>
_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
http://www.gluster.org/mailman/listinfo/gluster-devel
