On 07/15/2016 12:44 AM, Zhengping Zhou wrote: > 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); I agree with you. With current code base, it doesn't seems to be helping much. But we when we use libgfapi, we could implement zero copy read by changing the iobuf_get in client3_3_readv, so that the user given address can be passed as rsp buffer, and if we could give the data in the same memory , that will help to reduce one in memory copy. The same can also implement by giving the gluster memory to the user as a response to libgfapi read. Apart from this, I don't see any benefit for allocating the memory from client3_3_readv and giving in rsp struct. Regards Rafi KC > > 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 _______________________________________________ Gluster-devel mailing list Gluster-devel@xxxxxxxxxxx http://www.gluster.org/mailman/listinfo/gluster-devel
