Re: [systemd-devel] [PATCH kdbus] handle: Return POLLOUT | POLLWRNORM mask when no messages are pending
On Fri, 15.08.14 14:16, Marcel Holtmann (mar...@holtmann.org) wrote: Hi Daniel, To facility the feature of doing an asynchronous sending of messages when the bus is idle, make sure to return POLLOUT | POLLWRNORM from kdbus_handle_poll. Signed-off-by: Marcel Holtmann mar...@holtmann.org --- handle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/handle.c b/handle.c index ac6868133280..fc15d28351b3 100644 --- a/handle.c +++ b/handle.c @@ -884,6 +884,8 @@ static unsigned int kdbus_handle_poll(struct file *file, mask |= POLLERR | POLLHUP; else if (!list_empty(conn-msg_list)) mask |= POLLIN | POLLRDNORM; + else + mask |= POLLOUT | POLLWRNORM; Hmm, what's your use case here? list_empty(conn-msg_list) only checks the incoming list of the current connection for pending messages. That doesn't tell you whether the bus is idle, or if your receiving end has messages pending ... if you are a good client citizen, then you only send messages when POLLOUT gets signaled. That is what this is allowing now. Blindly sending messages is never a good idea. You want to poll for POLLOUT first. This does not make a big difference for current kernel, but it allows future extensions when the clients are well behaving and just waiting for POLLOUT. Meaning once the kernel does not signal POLLOUT, no new messages will come from the client. Current code does not do this POLLOUT before sending a messages, but our kdbus client actually does that. It is the right thing to do. And we have been doing this with all of our protocols that are using asynchronous IO. No point in kdbus being any different. Well, kdbus keeps per-reciever buffers only, hence signalling on the kdbus fd when you are able to write is not really possible, since this information is not bound to the sender fd but only to the receiever of which there are many... If I understand you correctly you hence want the kdbus fd to always return EPOLLOUT then, because if a client wants to send something it can do that at any time? If that's the case then POLLOUT should really be ORed into the mask unconditionally, not just in some cases... So, I can sympathize with what you are trying to do. However, I think your patch doesn't do the right thing... It should really OR the POLLOUT into all masks always. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH kdbus] handle: Return POLLOUT | POLLWRNORM mask when no messages are pending
Hi Lennart, To facility the feature of doing an asynchronous sending of messages when the bus is idle, make sure to return POLLOUT | POLLWRNORM from kdbus_handle_poll. Signed-off-by: Marcel Holtmann mar...@holtmann.org --- handle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/handle.c b/handle.c index ac6868133280..fc15d28351b3 100644 --- a/handle.c +++ b/handle.c @@ -884,6 +884,8 @@ static unsigned int kdbus_handle_poll(struct file *file, mask |= POLLERR | POLLHUP; else if (!list_empty(conn-msg_list)) mask |= POLLIN | POLLRDNORM; + else + mask |= POLLOUT | POLLWRNORM; Hmm, what's your use case here? list_empty(conn-msg_list) only checks the incoming list of the current connection for pending messages. That doesn't tell you whether the bus is idle, or if your receiving end has messages pending ... if you are a good client citizen, then you only send messages when POLLOUT gets signaled. That is what this is allowing now. Blindly sending messages is never a good idea. You want to poll for POLLOUT first. This does not make a big difference for current kernel, but it allows future extensions when the clients are well behaving and just waiting for POLLOUT. Meaning once the kernel does not signal POLLOUT, no new messages will come from the client. Current code does not do this POLLOUT before sending a messages, but our kdbus client actually does that. It is the right thing to do. And we have been doing this with all of our protocols that are using asynchronous IO. No point in kdbus being any different. Well, kdbus keeps per-reciever buffers only, hence signalling on the kdbus fd when you are able to write is not really possible, since this information is not bound to the sender fd but only to the receiever of which there are many... If I understand you correctly you hence want the kdbus fd to always return EPOLLOUT then, because if a client wants to send something it can do that at any time? If that's the case then POLLOUT should really be ORed into the mask unconditionally, not just in some cases... So, I can sympathize with what you are trying to do. However, I think your patch doesn't do the right thing... It should really OR the POLLOUT into all masks always. always returning POLLOUT is also fine. As long as the fd signals POLLOUT and not just swallows it. Regards Marcel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH kdbus] handle: Return POLLOUT | POLLWRNORM mask when no messages are pending
Hi On Mon, Aug 18, 2014 at 4:15 PM, Marcel Holtmann mar...@holtmann.org wrote: Hi Lennart, To facility the feature of doing an asynchronous sending of messages when the bus is idle, make sure to return POLLOUT | POLLWRNORM from kdbus_handle_poll. Signed-off-by: Marcel Holtmann mar...@holtmann.org --- handle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/handle.c b/handle.c index ac6868133280..fc15d28351b3 100644 --- a/handle.c +++ b/handle.c @@ -884,6 +884,8 @@ static unsigned int kdbus_handle_poll(struct file *file, mask |= POLLERR | POLLHUP; else if (!list_empty(conn-msg_list)) mask |= POLLIN | POLLRDNORM; + else + mask |= POLLOUT | POLLWRNORM; Hmm, what's your use case here? list_empty(conn-msg_list) only checks the incoming list of the current connection for pending messages. That doesn't tell you whether the bus is idle, or if your receiving end has messages pending ... if you are a good client citizen, then you only send messages when POLLOUT gets signaled. That is what this is allowing now. Blindly sending messages is never a good idea. You want to poll for POLLOUT first. This does not make a big difference for current kernel, but it allows future extensions when the clients are well behaving and just waiting for POLLOUT. Meaning once the kernel does not signal POLLOUT, no new messages will come from the client. Current code does not do this POLLOUT before sending a messages, but our kdbus client actually does that. It is the right thing to do. And we have been doing this with all of our protocols that are using asynchronous IO. No point in kdbus being any different. Well, kdbus keeps per-reciever buffers only, hence signalling on the kdbus fd when you are able to write is not really possible, since this information is not bound to the sender fd but only to the receiever of which there are many... If I understand you correctly you hence want the kdbus fd to always return EPOLLOUT then, because if a client wants to send something it can do that at any time? If that's the case then POLLOUT should really be ORed into the mask unconditionally, not just in some cases... So, I can sympathize with what you are trying to do. However, I think your patch doesn't do the right thing... It should really OR the POLLOUT into all masks always. always returning POLLOUT is also fine. As long as the fd signals POLLOUT and not just swallows it. Yes, we should add POLLOUT unconditionally. I will push the fix. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH kdbus] handle: Return POLLOUT | POLLWRNORM mask when no messages are pending
On 08/18/2014 04:02 PM, Lennart Poettering wrote: On Fri, 15.08.14 14:16, Marcel Holtmann (mar...@holtmann.org) wrote: Blindly sending messages is never a good idea. You want to poll for POLLOUT first. This does not make a big difference for current kernel, but it allows future extensions when the clients are well behaving and just waiting for POLLOUT. Meaning once the kernel does not signal POLLOUT, no new messages will come from the client. Current code does not do this POLLOUT before sending a messages, but our kdbus client actually does that. It is the right thing to do. And we have been doing this with all of our protocols that are using asynchronous IO. No point in kdbus being any different. Well, kdbus keeps per-reciever buffers only, hence signalling on the kdbus fd when you are able to write is not really possible, since this information is not bound to the sender fd but only to the receiever of which there are many... Exactly. If I understand you correctly you hence want the kdbus fd to always return EPOLLOUT then, because if a client wants to send something it can do that at any time? If that's the case then POLLOUT should really be ORed into the mask unconditionally, not just in some cases... Yes, except when the connection is dead. I've pushed a patch to do that, please check. Thanks, Daniel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH kdbus] handle: Return POLLOUT | POLLWRNORM mask when no messages are pending
To facility the feature of doing an asynchronous sending of messages when the bus is idle, make sure to return POLLOUT | POLLWRNORM from kdbus_handle_poll. Signed-off-by: Marcel Holtmann mar...@holtmann.org --- handle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/handle.c b/handle.c index ac6868133280..fc15d28351b3 100644 --- a/handle.c +++ b/handle.c @@ -884,6 +884,8 @@ static unsigned int kdbus_handle_poll(struct file *file, mask |= POLLERR | POLLHUP; else if (!list_empty(conn-msg_list)) mask |= POLLIN | POLLRDNORM; + else + mask |= POLLOUT | POLLWRNORM; mutex_unlock(conn-lock); return mask; -- 1.9.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH kdbus] handle: Return POLLOUT | POLLWRNORM mask when no messages are pending
Hi, On 08/15/2014 09:43 PM, Marcel Holtmann wrote: To facility the feature of doing an asynchronous sending of messages when the bus is idle, make sure to return POLLOUT | POLLWRNORM from kdbus_handle_poll. Signed-off-by: Marcel Holtmann mar...@holtmann.org --- handle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/handle.c b/handle.c index ac6868133280..fc15d28351b3 100644 --- a/handle.c +++ b/handle.c @@ -884,6 +884,8 @@ static unsigned int kdbus_handle_poll(struct file *file, mask |= POLLERR | POLLHUP; else if (!list_empty(conn-msg_list)) mask |= POLLIN | POLLRDNORM; + else + mask |= POLLOUT | POLLWRNORM; Hmm, what's your use case here? list_empty(conn-msg_list) only checks the incoming list of the current connection for pending messages. That doesn't tell you whether the bus is idle, or if your receiving end has messages pending ... Anyway, I don't see a problem in that change, so I've applied it now. Thanks! Daniel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH kdbus] handle: Return POLLOUT | POLLWRNORM mask when no messages are pending
Hi Daniel, To facility the feature of doing an asynchronous sending of messages when the bus is idle, make sure to return POLLOUT | POLLWRNORM from kdbus_handle_poll. Signed-off-by: Marcel Holtmann mar...@holtmann.org --- handle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/handle.c b/handle.c index ac6868133280..fc15d28351b3 100644 --- a/handle.c +++ b/handle.c @@ -884,6 +884,8 @@ static unsigned int kdbus_handle_poll(struct file *file, mask |= POLLERR | POLLHUP; else if (!list_empty(conn-msg_list)) mask |= POLLIN | POLLRDNORM; +else +mask |= POLLOUT | POLLWRNORM; Hmm, what's your use case here? list_empty(conn-msg_list) only checks the incoming list of the current connection for pending messages. That doesn't tell you whether the bus is idle, or if your receiving end has messages pending ... if you are a good client citizen, then you only send messages when POLLOUT gets signaled. That is what this is allowing now. Blindly sending messages is never a good idea. You want to poll for POLLOUT first. This does not make a big difference for current kernel, but it allows future extensions when the clients are well behaving and just waiting for POLLOUT. Meaning once the kernel does not signal POLLOUT, no new messages will come from the client. Current code does not do this POLLOUT before sending a messages, but our kdbus client actually does that. It is the right thing to do. And we have been doing this with all of our protocols that are using asynchronous IO. No point in kdbus being any different. Regards Marcel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
