On Wed, Feb 14, 2018 at 1:50 PM, Santosh Shilimkar
<santosh.shilim...@oracle.com> wrote:
> On 2/14/2018 2:28 AM, Sowmini Varadhan wrote:
>>
>> RDS removes a datagram (rds_message) from the retransmit queue when
>> an ACK is received. The ACK indicates that the receiver has queued
>> the RDS datagram, so that the sender can safely forget the datagram.
>> When all references to the rds_message are quiesced, rds_message_purge
>> is called to release resources used by the rds_message
>>
>> If the datagram to be removed had pinned pages set up, add
>> an entry to the rs->rs_znotify_queue so that the notifcation
>> will be sent up via rds_rm_zerocopy_callback() when the
>> rds_message is eventually freed by rds_message_purge.
>>
>> rds_rm_zerocopy_callback() attempts to batch the number of cookies
>> sent with each notification to a max of SO_EE_ORIGIN_MAX_ZCOOKIES.
>> This is achieved by checking the tail skb in the sk_error_queue:
>> if this has room for one more cookie, the cookie from the
>> current notification is added; else a new skb is added to the
>> sk_error_queue. Every invocation of rds_rm_zerocopy_callback() will
>> trigger a ->sk_error_report to notify the application.
>>
>> Signed-off-by: Sowmini Varadhan <sowmini.varad...@oracle.com>
>> ---
>> v2:
>> - make sure to always sock_put m_rs even if there is no znotifier.
>> - major rewrite of notification, resulting in much simplification.
>>
>> include/uapi/linux/errqueue.h | 2 +
>> net/rds/af_rds.c | 2 +
>> net/rds/message.c | 83
>> +++++++++++++++++++++++++++++++++++++---
>> net/rds/rds.h | 14 +++++++
>> net/rds/recv.c | 2 +
>> 5 files changed, 96 insertions(+), 7 deletions(-)
>>
> generic comment and please update it where it is applicable
> in terms of variable names, notifiers etc.
>
> RDS support true zero copy already with RDMA transport so some of
> this code can easily get confused.
>
> So I suggest something like below.
> s/zerocopy/zeromsgcopy
> s/zcopy/zmsgcopy
> s/zcookie/zmsgcpycookie
> s/znotifier/zmsgcpynotifier
>
>
>> diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
>> index dc64cfa..28812ed 100644
>> --- a/include/uapi/linux/errqueue.h
>> +++ b/include/uapi/linux/errqueue.h
>> @@ -20,11 +20,13 @@ struct sock_extended_err {
>> #define SO_EE_ORIGIN_ICMP6 3
>> #define SO_EE_ORIGIN_TXSTATUS 4
>> #define SO_EE_ORIGIN_ZEROCOPY 5
>> +#define SO_EE_ORIGIN_ZCOOKIE 6
>> #define SO_EE_ORIGIN_TIMESTAMPING SO_EE_ORIGIN_TXSTATUS
>> #define SO_EE_OFFENDER(ee) ((struct sockaddr*)((ee)+1))
>> #define SO_EE_CODE_ZEROCOPY_COPIED 1
>> +#define SO_EE_ORIGIN_MAX_ZCOOKIES 8
>
>
> This error change might need to go though other subsystem tree. May
> be you can seperate it and also copy "linux-...@vger.kernel.org"
Previous changes to this file also went in through net-next, so this is
fine. The error queue is a socket, so network, datastructure.
As for naming, zerocopy is unfortunately an overused term. I did not
help matters when adding msg_zerocopy. That said, let's try to keep
consistent naming across socket families, so no zmsg prefix only in
RDS.
