[tipc-discussion] [PATCH net-next v2 1/3] tipc: unify tipc_wait_for_sndpkt() and tipc_wait_for_sndmsg() functions
The functions tipc_wait_for_sndpkt() and tipc_wait_for_sndmsg() are very
similar. The latter function is also called from two locations, and
there will be more in the coming commits, which will all need to test on
different conditions.
Instead of making yet another duplicates of the function, we now
introduce a new macro tipc_wait_for_cond() where the wakeup condition
can be stated as an argument to the call. This macro replaces all
current and future uses of the two functions, which can now be
eliminated.
Signed-off-by: Jon Maloy
---
net/tipc/socket.c | 110 +-
1 file changed, 51 insertions(+), 59 deletions(-)
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 333c5da..30732a8 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -110,7 +110,6 @@ static void tipc_write_space(struct sock *sk);
static void tipc_sock_destruct(struct sock *sk);
static int tipc_release(struct socket *sock);
static int tipc_accept(struct socket *sock, struct socket *new_sock, int
flags);
-static int tipc_wait_for_sndmsg(struct socket *sock, long *timeo_p);
static void tipc_sk_timeout(unsigned long data);
static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
struct tipc_name_seq const *seq);
@@ -334,6 +333,51 @@ static int tipc_set_sk_state(struct sock *sk, int state)
return res;
}
+static int tipc_sk_sock_err(struct socket *sock, long *timeout)
+{
+ struct sock *sk = sock->sk;
+ int err = sock_error(sk);
+ int typ = sock->type;
+
+ if (err)
+ return err;
+ if (typ == SOCK_STREAM || typ == SOCK_SEQPACKET) {
+ if (sk->sk_state == TIPC_DISCONNECTING)
+ return -EPIPE;
+ else if (!tipc_sk_connected(sk))
+ return -ENOTCONN;
+ } else if (sk->sk_shutdown & SEND_SHUTDOWN) {
+ return -EPIPE;
+ }
+ if (!*timeout)
+ return -EAGAIN;
+ if (signal_pending(current))
+ return sock_intr_errno(*timeout);
+
+ return 0;
+}
+
+#define tipc_wait_for_cond(sock_, timeout_, condition_)
\
+({ \
+ int rc_ = 0;\
+ int done_ = 0; \
+ \
+ while (!(condition_) && !done_) { \
+ struct sock *sk_ = sock->sk;\
+ DEFINE_WAIT_FUNC(wait_, woken_wake_function); \
+ \
+ rc_ = tipc_sk_sock_err(sock_, timeout_);\
+ if (rc_)\
+ break; \
+ prepare_to_wait(sk_sleep(sk_), &wait_, \
+ TASK_INTERRUPTIBLE);\
+ done_ = sk_wait_event(sk_, timeout_,\
+ (condition_), &wait_);\
+ remove_wait_queue(sk_sleep(sk), &wait_);\
+ } \
+ rc_;\
+})
+
/**
* tipc_sk_create - create a TIPC socket
* @net: network namespace (must be default network)
@@ -719,7 +763,7 @@ static int tipc_sendmcast(struct socket *sock, struct
tipc_name_seq *seq,
if (rc == -ELINKCONG) {
tsk->link_cong = 1;
- rc = tipc_wait_for_sndmsg(sock, &timeo);
+ rc = tipc_wait_for_cond(sock, &timeo, !tsk->link_cong);
if (!rc)
continue;
}
@@ -828,31 +872,6 @@ static void tipc_sk_proto_rcv(struct tipc_sock *tsk,
struct sk_buff *skb,
kfree_skb(skb);
}
-static int tipc_wait_for_sndmsg(struct socket *sock, long *timeo_p)
-{
- DEFINE_WAIT_FUNC(wait, woken_wake_function);
- struct sock *sk = sock->sk;
- struct tipc_sock *tsk = tipc_sk(sk);
- int done;
-
- do {
- int err = sock_error(sk);
- if (err)
- return err;
- if (sk->sk_shutdown & SEND_SHUTDOWN)
- return -EPIPE;
- if (!*timeo_p)
- return -EAGAIN;
- if (signal_pending(current))
- return sock_intr_errno(*timeo_p);
-
- add_wait_queue(sk_sleep(sk), &wait);
- done = sk_wait_event(sk, timeo_p, !tsk->link_cong, &wait);
- remove_wait_queue(sk_sleep(sk), &wait);
- } while (
Re: [tipc-discussion] [PATCH net-next v2 1/3] tipc: unify tipc_wait_for_sndpkt() and tipc_wait_for_sndmsg() functions
On 11/29/2016 06:03 PM, Jon Maloy wrote:
> The functions tipc_wait_for_sndpkt() and tipc_wait_for_sndmsg() are very
> similar. The latter function is also called from two locations, and
> there will be more in the coming commits, which will all need to test on
> different conditions.
>
> Instead of making yet another duplicates of the function, we now
> introduce a new macro tipc_wait_for_cond() where the wakeup condition
> can be stated as an argument to the call. This macro replaces all
> current and future uses of the two functions, which can now be
> eliminated.
>
> Signed-off-by: Jon Maloy
> ---
> net/tipc/socket.c | 110
> +-
> 1 file changed, 51 insertions(+), 59 deletions(-)
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 333c5da..30732a8 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -110,7 +110,6 @@ static void tipc_write_space(struct sock *sk);
> static void tipc_sock_destruct(struct sock *sk);
> static int tipc_release(struct socket *sock);
> static int tipc_accept(struct socket *sock, struct socket *new_sock, int
> flags);
> -static int tipc_wait_for_sndmsg(struct socket *sock, long *timeo_p);
> static void tipc_sk_timeout(unsigned long data);
> static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
> struct tipc_name_seq const *seq);
> @@ -334,6 +333,51 @@ static int tipc_set_sk_state(struct sock *sk, int state)
> return res;
> }
>
> +static int tipc_sk_sock_err(struct socket *sock, long *timeout)
> +{
> + struct sock *sk = sock->sk;
> + int err = sock_error(sk);
> + int typ = sock->type;
> +
> + if (err)
> + return err;
> + if (typ == SOCK_STREAM || typ == SOCK_SEQPACKET) {
> + if (sk->sk_state == TIPC_DISCONNECTING)
> + return -EPIPE;
> + else if (!tipc_sk_connected(sk))
> + return -ENOTCONN;
> + } else if (sk->sk_shutdown & SEND_SHUTDOWN) {
> + return -EPIPE;
The else is for connection less sockets, so returning EPIPE does not
make sense.
/Partha
> + }
> + if (!*timeout)
> + return -EAGAIN;
> + if (signal_pending(current))
> + return sock_intr_errno(*timeout);
> +
> + return 0;
> +}
> +
> +#define tipc_wait_for_cond(sock_, timeout_, condition_)
> \
> +({ \
> + int rc_ = 0;\
> + int done_ = 0; \
> + \
> + while (!(condition_) && !done_) { \
> + struct sock *sk_ = sock->sk;\
> + DEFINE_WAIT_FUNC(wait_, woken_wake_function); \
> + \
> + rc_ = tipc_sk_sock_err(sock_, timeout_);\
> + if (rc_)\
> + break; \
> + prepare_to_wait(sk_sleep(sk_), &wait_, \
> + TASK_INTERRUPTIBLE);\
> + done_ = sk_wait_event(sk_, timeout_,\
> + (condition_), &wait_);\
> + remove_wait_queue(sk_sleep(sk), &wait_);\
> + } \
> + rc_;\
> +})
> +
> /**
> * tipc_sk_create - create a TIPC socket
> * @net: network namespace (must be default network)
> @@ -719,7 +763,7 @@ static int tipc_sendmcast(struct socket *sock, struct
> tipc_name_seq *seq,
>
> if (rc == -ELINKCONG) {
> tsk->link_cong = 1;
> - rc = tipc_wait_for_sndmsg(sock, &timeo);
> + rc = tipc_wait_for_cond(sock, &timeo, !tsk->link_cong);
> if (!rc)
> continue;
> }
> @@ -828,31 +872,6 @@ static void tipc_sk_proto_rcv(struct tipc_sock *tsk,
> struct sk_buff *skb,
> kfree_skb(skb);
> }
>
> -static int tipc_wait_for_sndmsg(struct socket *sock, long *timeo_p)
> -{
> - DEFINE_WAIT_FUNC(wait, woken_wake_function);
> - struct sock *sk = sock->sk;
> - struct tipc_sock *tsk = tipc_sk(sk);
> - int done;
> -
> - do {
> - int err = sock_error(sk);
> - if (err)
> - return err;
> - if (sk->sk_shutdown & SEND_SHUTDOWN)
> - return -EPIPE;
> - if (!*timeo_p)
> - return -EAGAIN;
> - if (signal_pending(current))
> - retur
Re: [tipc-discussion] [PATCH net-next v2 1/3] tipc: unify tipc_wait_for_sndpkt() and tipc_wait_for_sndmsg() functions
On 12/06/2016 03:49 AM, Parthasarathy Bhuvaragan wrote:
> On 11/29/2016 06:03 PM, Jon Maloy wrote:
>> The functions tipc_wait_for_sndpkt() and tipc_wait_for_sndmsg() are very
>> similar. The latter function is also called from two locations, and
>> there will be more in the coming commits, which will all need to test on
>> different conditions.
>>
>> Instead of making yet another duplicates of the function, we now
>> introduce a new macro tipc_wait_for_cond() where the wakeup condition
>> can be stated as an argument to the call. This macro replaces all
>> current and future uses of the two functions, which can now be
>> eliminated.
>>
>> Signed-off-by: Jon Maloy
>> ---
>> net/tipc/socket.c | 110
>> +-
>> 1 file changed, 51 insertions(+), 59 deletions(-)
>>
>> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
>> index 333c5da..30732a8 100644
>> --- a/net/tipc/socket.c
>> +++ b/net/tipc/socket.c
>> @@ -110,7 +110,6 @@ static void tipc_write_space(struct sock *sk);
>> static void tipc_sock_destruct(struct sock *sk);
>> static int tipc_release(struct socket *sock);
>> static int tipc_accept(struct socket *sock, struct socket *new_sock,
>> int flags);
>> -static int tipc_wait_for_sndmsg(struct socket *sock, long *timeo_p);
>> static void tipc_sk_timeout(unsigned long data);
>> static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
>> struct tipc_name_seq const *seq);
>> @@ -334,6 +333,51 @@ static int tipc_set_sk_state(struct sock *sk,
>> int state)
>> return res;
>> }
>>
>> +static int tipc_sk_sock_err(struct socket *sock, long *timeout)
>> +{
>> +struct sock *sk = sock->sk;
>> +int err = sock_error(sk);
>> +int typ = sock->type;
>> +
>> +if (err)
>> +return err;
>> +if (typ == SOCK_STREAM || typ == SOCK_SEQPACKET) {
>> +if (sk->sk_state == TIPC_DISCONNECTING)
>> +return -EPIPE;
>> +else if (!tipc_sk_connected(sk))
>> +return -ENOTCONN;
>> +} else if (sk->sk_shutdown & SEND_SHUTDOWN) {
>> +return -EPIPE;
> The else is for connection less sockets, so returning EPIPE does not
> make sense.
This is how it is currently done in tipc_wait_for_snd_msg(), probably a
leftover from a time when it was used even for connections. Should I
just leave it out, or do you suggest any other code?
///jon
>
> /Partha
>> +}
>> +if (!*timeout)
>> +return -EAGAIN;
>> +if (signal_pending(current))
>> +return sock_intr_errno(*timeout);
>> +
>> +return 0;
>> +}
>> +
>> +#define tipc_wait_for_cond(sock_, timeout_, condition_)\
>> +({\
>> +int rc_ = 0;\
>> +int done_ = 0;\
>> +\
>> +while (!(condition_) && !done_) {\
>> +struct sock *sk_ = sock->sk;\
>> +DEFINE_WAIT_FUNC(wait_, woken_wake_function);\
>> +\
>> +rc_ = tipc_sk_sock_err(sock_, timeout_);\
>> +if (rc_)\
>> +break;\
>> +prepare_to_wait(sk_sleep(sk_), &wait_,\
>> +TASK_INTERRUPTIBLE);\
>> +done_ = sk_wait_event(sk_, timeout_,\
>> + (condition_), &wait_);\
>> +remove_wait_queue(sk_sleep(sk), &wait_);\
>> +}\
>> +rc_;\
>> +})
>> +
>> /**
>> * tipc_sk_create - create a TIPC socket
>> * @net: network namespace (must be default network)
>> @@ -719,7 +763,7 @@ static int tipc_sendmcast(struct socket *sock,
>> struct tipc_name_seq *seq,
>>
>> if (rc == -ELINKCONG) {
>> tsk->link_cong = 1;
>> -rc = tipc_wait_for_sndmsg(sock, &timeo);
>> +rc = tipc_wait_for_cond(sock, &timeo, !tsk->link_cong);
>> if (!rc)
>> continue;
>> }
>> @@ -828,31 +872,6 @@ static void tipc_sk_proto_rcv(struct tipc_sock
>> *tsk, struct sk_buff *skb,
>> kfree_skb(skb);
>> }
>>
>> -static int tipc_wait_for_sndmsg(struct socket *sock, long *timeo_p)
>> -{
>> -DEFINE_WAIT_FUNC(wait, woken_wake_function);
>> -struct sock *sk = sock->sk;
>> -struct tipc_sock *tsk = tipc_sk(sk);
>> -int done;
>> -
>> -do {
>> -int err = sock_error(sk);
>> -if (err)
>> -return err;
>> -if (sk->sk_shutdown & SEND_SHUTDOWN)
>> -return -EPIPE;
>> -if (!*timeo_p)
>> -return -EAGAIN;
>> -if (signal_pending(current))
>> -return sock_intr_errno(*timeo_p);
>> -
>> -add_wait_queue(sk_sleep(sk), &wait);
>> -done = sk_wait_event(sk, timeo_p, !tsk->link_cong, &wait);
>> -remove_wait_queue(sk_sleep(sk), &wait
Re: [tipc-discussion] [PATCH net-next v2 1/3] tipc: unify tipc_wait_for_sndpkt() and tipc_wait_for_sndmsg() functions
On 12/06/2016 12:49 PM, Jon Maloy wrote:
>
>
> On 12/06/2016 03:49 AM, Parthasarathy Bhuvaragan wrote:
>> On 11/29/2016 06:03 PM, Jon Maloy wrote:
>>> The functions tipc_wait_for_sndpkt() and tipc_wait_for_sndmsg() are very
>>> similar. The latter function is also called from two locations, and
>>> there will be more in the coming commits, which will all need to test on
>>> different conditions.
>>>
>>> Instead of making yet another duplicates of the function, we now
>>> introduce a new macro tipc_wait_for_cond() where the wakeup condition
>>> can be stated as an argument to the call. This macro replaces all
>>> current and future uses of the two functions, which can now be
>>> eliminated.
>>>
>>> Signed-off-by: Jon Maloy
>>> ---
>>> net/tipc/socket.c | 110
>>> +-
>>> 1 file changed, 51 insertions(+), 59 deletions(-)
>>>
>>> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
>>> index 333c5da..30732a8 100644
>>> --- a/net/tipc/socket.c
>>> +++ b/net/tipc/socket.c
>>> @@ -110,7 +110,6 @@ static void tipc_write_space(struct sock *sk);
>>> static void tipc_sock_destruct(struct sock *sk);
>>> static int tipc_release(struct socket *sock);
>>> static int tipc_accept(struct socket *sock, struct socket *new_sock,
>>> int flags);
>>> -static int tipc_wait_for_sndmsg(struct socket *sock, long *timeo_p);
>>> static void tipc_sk_timeout(unsigned long data);
>>> static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
>>> struct tipc_name_seq const *seq);
>>> @@ -334,6 +333,51 @@ static int tipc_set_sk_state(struct sock *sk,
>>> int state)
>>> return res;
>>> }
>>>
>>> +static int tipc_sk_sock_err(struct socket *sock, long *timeout)
>>> +{
>>> +struct sock *sk = sock->sk;
>>> +int err = sock_error(sk);
>>> +int typ = sock->type;
>>> +
>>> +if (err)
>>> +return err;
>>> +if (typ == SOCK_STREAM || typ == SOCK_SEQPACKET) {
>>> +if (sk->sk_state == TIPC_DISCONNECTING)
>>> +return -EPIPE;
>>> +else if (!tipc_sk_connected(sk))
>>> +return -ENOTCONN;
>>> +} else if (sk->sk_shutdown & SEND_SHUTDOWN) {
>>> +return -EPIPE;
>> The else is for connection less sockets, so returning EPIPE does not
>> make sense.
>
> This is how it is currently done in tipc_wait_for_snd_msg(), probably a
> leftover from a time when it was used even for connections. Should I
> just leave it out, or do you suggest any other code?
You can remove it, as SEND_SHUTDOWN is always in sync with
TIPC_DISCONNECTING.
/partha
>
> ///jon
>
>>
>> /Partha
>>> +}
>>> +if (!*timeout)
>>> +return -EAGAIN;
>>> +if (signal_pending(current))
>>> +return sock_intr_errno(*timeout);
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +#define tipc_wait_for_cond(sock_, timeout_, condition_)\
>>> +({\
>>> +int rc_ = 0;\
>>> +int done_ = 0;\
>>> +\
>>> +while (!(condition_) && !done_) {\
>>> +struct sock *sk_ = sock->sk;\
>>> +DEFINE_WAIT_FUNC(wait_, woken_wake_function);\
>>> +\
>>> +rc_ = tipc_sk_sock_err(sock_, timeout_);\
>>> +if (rc_)\
>>> +break;\
>>> +prepare_to_wait(sk_sleep(sk_), &wait_,\
>>> +TASK_INTERRUPTIBLE);\
>>> +done_ = sk_wait_event(sk_, timeout_,\
>>> + (condition_), &wait_);\
>>> +remove_wait_queue(sk_sleep(sk), &wait_);\
>>> +}\
>>> +rc_;\
>>> +})
>>> +
>>> /**
>>> * tipc_sk_create - create a TIPC socket
>>> * @net: network namespace (must be default network)
>>> @@ -719,7 +763,7 @@ static int tipc_sendmcast(struct socket *sock,
>>> struct tipc_name_seq *seq,
>>>
>>> if (rc == -ELINKCONG) {
>>> tsk->link_cong = 1;
>>> -rc = tipc_wait_for_sndmsg(sock, &timeo);
>>> +rc = tipc_wait_for_cond(sock, &timeo, !tsk->link_cong);
>>> if (!rc)
>>> continue;
>>> }
>>> @@ -828,31 +872,6 @@ static void tipc_sk_proto_rcv(struct tipc_sock
>>> *tsk, struct sk_buff *skb,
>>> kfree_skb(skb);
>>> }
>>>
>>> -static int tipc_wait_for_sndmsg(struct socket *sock, long *timeo_p)
>>> -{
>>> -DEFINE_WAIT_FUNC(wait, woken_wake_function);
>>> -struct sock *sk = sock->sk;
>>> -struct tipc_sock *tsk = tipc_sk(sk);
>>> -int done;
>>> -
>>> -do {
>>> -int err = sock_error(sk);
>>> -if (err)
>>> -return err;
>>> -if (sk->sk_shutdown & SEND_SHUTDOWN)
>>> -return -EPIPE;
>>> -if (!*timeo_p)
>>> -return -EAGAIN;
>>> -
