MAJOR: channel: remove the BF_OUT_EMPTY flag
This flag was very problematic because it was composite in that both changes
to the pipe or to the buffer had to cause this flag to be updated, which is
not always simple (eg: there may not even be a channel attached to a buffer
at all).
There were not that many users of this flags, mostly setters. So the flag got
replaced with a macro which reports whether the channel is empty or not, by
checking both the pipe and the buffer.
One part of the change is sensible : the flag was also part of BF_MASK_STATIC,
which is used by process_session() to rescan all analysers in case the flag's
status changes. At first glance, none of the analysers seems to change its
mind base on this flag when it is subject to change, so it seems fine not to
add variation checks here. Otherwise it's possible that checking the buffer's
output size is more useful than checking the flag's replacement.
diff --git a/include/proto/channel.h b/include/proto/channel.h
index 6b5478d..83881ba 100644
--- a/include/proto/channel.h
+++ b/include/proto/channel.h
@@ -49,7 +49,7 @@
int buffer_insert_line2(struct channel *b, char *pos, const char *str, int len);
unsigned long long buffer_forward(struct channel *buf, unsigned long long bytes);
-/* Initialize all fields in the buffer. The BF_OUT_EMPTY flags is set. */
+/* Initialize all fields in the buffer. */
static inline void buffer_init(struct channel *buf)
{
buf->buf.o = 0;
@@ -60,13 +60,23 @@
buf->pipe = NULL;
buf->analysers = 0;
buf->cons = NULL;
- buf->flags = BF_OUT_EMPTY;
+ buf->flags = 0;
}
/*****************************************************************/
/* These functions are used to compute various buffer area sizes */
/*****************************************************************/
+/* Reports non-zero if the channel is empty, which means both its
+ * buffer and pipe are empty. The construct looks strange but is
+ * jump-less and much more efficient on both 32 and 64-bit than
+ * the boolean test.
+ */
+static inline unsigned int channel_is_empty(struct channel *c)
+{
+ return !(c->buf.o | (long)c->pipe);
+}
+
/* Return the number of reserved bytes in the buffer, which ensures that once
* all pending data are forwarded, the buffer still has global.tune.maxrewrite
* bytes free. The result is between 0 and global.maxrewrite, which is itself
@@ -151,14 +161,12 @@
/* Advances the buffer by <adv> bytes, which means that the buffer
* pointer advances, and that as many bytes from in are transferred
* to out. The caller is responsible for ensuring that adv is always
- * smaller than or equal to b->i. The BF_OUT_EMPTY flag is updated.
+ * smaller than or equal to b->i.
*/
static inline void b_adv(struct channel *b, unsigned int adv)
{
b->buf.i -= adv;
b->buf.o += adv;
- if (b->buf.o)
- b->flags &= ~BF_OUT_EMPTY;
b->buf.p = b_ptr(&b->buf, adv);
}
@@ -170,8 +178,6 @@
{
b->buf.i += adv;
b->buf.o -= adv;
- if (!b->buf.o && !b->pipe)
- b->flags |= BF_OUT_EMPTY;
b->buf.p = b_ptr(&b->buf, (int)-adv);
}
@@ -225,8 +231,6 @@
buf->buf.p = buffer_wrap_add(&buf->buf, buf->buf.p + buf->buf.i);
buf->buf.o += buf->buf.i;
buf->buf.i = 0;
- if (buf->buf.o)
- buf->flags &= ~BF_OUT_EMPTY;
}
/* Erase any content from buffer <buf> and adjusts flags accordingly. Note
@@ -239,9 +243,7 @@
buf->buf.i = 0;
buf->to_forward = 0;
buf->buf.p = buf->buf.data;
- buf->flags &= ~(BF_FULL | BF_OUT_EMPTY);
- if (!buf->pipe)
- buf->flags |= BF_OUT_EMPTY;
+ buf->flags &= ~BF_FULL;
}
/* Cut the "tail" of the buffer, which means strip it to the length of unsent
@@ -350,8 +352,6 @@
static inline void bo_skip(struct channel *buf, int len)
{
buf->buf.o -= len;
- if (!buf->buf.o && !buf->pipe)
- buf->flags |= BF_OUT_EMPTY;
if (buffer_len(&buf->buf) == 0)
buf->buf.p = buf->buf.data;
@@ -403,7 +403,7 @@
static inline int bo_getchr(struct channel *buf)
{
/* closed or empty + imminent close = -2; empty = -1 */
- if (unlikely(buf->flags & (BF_OUT_EMPTY|BF_SHUTW))) {
+ if (unlikely((buf->flags & BF_SHUTW) || channel_is_empty(buf))) {
if (buf->flags & (BF_SHUTW|BF_SHUTW_NOW))
return -2;
return -1;
@@ -415,8 +415,8 @@
* buffer <b>, and moves <end> just after the end of <str>. <b>'s parameters
* (l, r, lr) are updated to be valid after the shift. the shift value
* (positive or negative) is returned. If there's no space left, the move is
- * not done. The function does not adjust ->o nor BF_OUT_EMPTY because
- * it does not make sense to use it on data scheduled to be sent.
+ * not done. The function does not adjust ->o because it does not make sense
+ * to use it on data scheduled to be sent.
*/
static inline int buffer_replace(struct channel *b, char *pos, char *end, const char *str)
{
diff --git a/include/types/channel.h b/include/types/channel.h
index cd16d9c..67016d1 100644
--- a/include/types/channel.h
+++ b/include/types/channel.h
@@ -70,7 +70,7 @@
#define BF_WRITE_ERROR 0x000800 /* unrecoverable error on consumer side */
#define BF_WRITE_ACTIVITY (BF_WRITE_NULL|BF_WRITE_PARTIAL|BF_WRITE_ERROR)
-#define BF_OUT_EMPTY 0x001000 /* out and pipe are empty. Set by last change. */
+/* unused: 0x001000 */
#define BF_SHUTW 0x002000 /* consumer has already shut down */
#define BF_SHUTW_NOW 0x004000 /* the consumer must shut down for writes ASAP */
#define BF_AUTO_CLOSE 0x008000 /* producer can forward shutdown to other side */
@@ -129,7 +129,7 @@
#define BF_MASK_ANALYSER (BF_READ_ATTACHED|BF_READ_ACTIVITY|BF_READ_TIMEOUT|BF_ANA_TIMEOUT|BF_WRITE_ACTIVITY|BF_WAKE_ONCE)
/* Mask for static flags which cause analysers to be woken up when they change */
-#define BF_MASK_STATIC (BF_OUT_EMPTY|BF_FULL|BF_SHUTR|BF_SHUTW|BF_SHUTR_NOW|BF_SHUTW_NOW)
+#define BF_MASK_STATIC (BF_FULL|BF_SHUTR|BF_SHUTW|BF_SHUTR_NOW|BF_SHUTW_NOW)
/* Analysers (channel->analysers).
diff --git a/src/channel.c b/src/channel.c
index 2789e89..825ed7b 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -127,7 +127,7 @@
buf->buf.p = b_ptr(&buf->buf, len);
buf->total += len;
- buf->flags &= ~(BF_OUT_EMPTY|BF_FULL);
+ buf->flags &= ~BF_FULL;
if (bi_full(buf))
buf->flags |= BF_FULL;
@@ -242,7 +242,7 @@
max = len;
/* closed or empty + imminent close = -1; empty = 0 */
- if (unlikely(buf->flags & (BF_OUT_EMPTY|BF_SHUTW))) {
+ if (unlikely((buf->flags & BF_SHUTW) || channel_is_empty(buf))) {
if (buf->flags & (BF_SHUTW|BF_SHUTW_NOW))
ret = -1;
goto out;
@@ -314,11 +314,11 @@
* buffer <b>, and moves <end> just after the end of <str>. <b>'s parameters
* <l> and <r> are updated to be valid after the shift. The shift value
* (positive or negative) is returned. If there's no space left, the move is
- * not done. The function does not adjust ->o nor BF_OUT_EMPTY because it
- * does not make sense to use it on data scheduled to be sent. For the same
- * reason, it does not make sense to call this function on unparsed data, so
- * <orig> is not updated. The string length is taken from parameter <len>. If
- * <len> is null, the <str> pointer is allowed to be null.
+ * not done. The function does not adjust ->o because it does not make sense to
+ * use it on data scheduled to be sent. For the same reason, it does not make
+ * sense to call this function on unparsed data, so <orig> is not updated. The
+ * string length is taken from parameter <len>. If <len> is null, the <str>
+ * pointer is allowed to be null.
*/
int buffer_replace2(struct channel *b, char *pos, char *end, const char *str, int len)
{
diff --git a/src/proto_http.c b/src/proto_http.c
index a7e7b6e..f38e378 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3955,7 +3955,7 @@
/* if we've just closed an output, let's switch */
buf->cons->flags |= SI_FL_NOLINGER; /* we want to close ASAP */
- if (!(buf->flags & BF_OUT_EMPTY)) {
+ if (!channel_is_empty(buf)) {
txn->req.msg_state = HTTP_MSG_CLOSING;
goto http_msg_closing;
}
@@ -3972,7 +3972,7 @@
/* nothing else to forward, just waiting for the output buffer
* to be empty and for the shutw_now to take effect.
*/
- if (buf->flags & BF_OUT_EMPTY) {
+ if (channel_is_empty(buf)) {
txn->req.msg_state = HTTP_MSG_CLOSED;
goto http_msg_closed;
}
@@ -4076,7 +4076,7 @@
if (buf->flags & (BF_SHUTW|BF_SHUTW_NOW)) {
/* if we've just closed an output, let's switch */
- if (!(buf->flags & BF_OUT_EMPTY)) {
+ if (!channel_is_empty(buf)) {
txn->rsp.msg_state = HTTP_MSG_CLOSING;
goto http_msg_closing;
}
@@ -4093,7 +4093,7 @@
/* nothing else to forward, just waiting for the output buffer
* to be empty and for the shutw_now to take effect.
*/
- if (buf->flags & BF_OUT_EMPTY) {
+ if (channel_is_empty(buf)) {
txn->rsp.msg_state = HTTP_MSG_CLOSED;
goto http_msg_closed;
}
diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index bdab4ab..488d737 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -473,7 +473,7 @@
fdtab[fd].iocb = conn_fd_handler;
fd_insert(fd);
conn_sock_want_send(&si->conn); /* for connect status */
- if (!(si->ob->flags & BF_OUT_EMPTY))
+ if (!channel_is_empty(si->ob))
conn_data_want_send(&si->conn); /* prepare to send data if any */
si->state = SI_ST_CON;
diff --git a/src/session.c b/src/session.c
index db6dec4..1aec3bf 100644
--- a/src/session.c
+++ b/src/session.c
@@ -563,7 +563,7 @@
/* OK, maybe we want to abort */
if (unlikely((rep->flags & BF_SHUTW) ||
((req->flags & BF_SHUTW_NOW) && /* FIXME: this should not prevent a connection from establishing */
- (((req->flags & (BF_OUT_EMPTY|BF_WRITE_ACTIVITY)) == BF_OUT_EMPTY) ||
+ ((!(req->flags & BF_WRITE_ACTIVITY) && channel_is_empty(req)) ||
s->be->options & PR_O_ABRT_CLOSE)))) {
/* give up */
si_shutw(si);
@@ -829,7 +829,7 @@
/* Connection remains in queue, check if we have to abort it */
if ((si->ob->flags & (BF_READ_ERROR)) ||
((si->ob->flags & BF_SHUTW_NOW) && /* empty and client aborted */
- (si->ob->flags & BF_OUT_EMPTY || s->be->options & PR_O_ABRT_CLOSE))) {
+ (channel_is_empty(si->ob) || s->be->options & PR_O_ABRT_CLOSE))) {
/* give up */
si->exp = TICK_ETERNITY;
s->logs.t_queue = tv_ms_elapsed(&s->logs.tv_accept, &now);
@@ -849,7 +849,7 @@
/* Connection request might be aborted */
if ((si->ob->flags & (BF_READ_ERROR)) ||
((si->ob->flags & BF_SHUTW_NOW) && /* empty and client aborted */
- (si->ob->flags & BF_OUT_EMPTY || s->be->options & PR_O_ABRT_CLOSE))) {
+ (channel_is_empty(si->ob) || s->be->options & PR_O_ABRT_CLOSE))) {
/* give up */
si->exp = TICK_ETERNITY;
si_shutr(si);
@@ -1893,7 +1893,8 @@
buffer_shutw_now(s->req);
/* shutdown(write) pending */
- if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_OUT_EMPTY)) == (BF_SHUTW_NOW|BF_OUT_EMPTY)))
+ if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW)) == BF_SHUTW_NOW &&
+ channel_is_empty(s->req)))
si_shutw(s->req->cons);
/* shutdown(write) done on server side, we must stop the client too */
@@ -1915,7 +1916,7 @@
*/
if (s->req->cons->state == SI_ST_INI) {
if (!(s->req->flags & BF_SHUTW)) {
- if ((s->req->flags & (BF_AUTO_CONNECT|BF_OUT_EMPTY)) != BF_OUT_EMPTY) {
+ if ((s->req->flags & BF_AUTO_CONNECT) || !channel_is_empty(s->req)) {
/* If we have an applet without a connect method, we immediately
* switch to the connected state, otherwise we perform a connection
* request.
@@ -2038,7 +2039,8 @@
buffer_shutw_now(s->rep);
/* shutdown(write) pending */
- if (unlikely((s->rep->flags & (BF_SHUTW|BF_OUT_EMPTY|BF_SHUTW_NOW)) == (BF_OUT_EMPTY|BF_SHUTW_NOW)))
+ if (unlikely((s->rep->flags & (BF_SHUTW|BF_SHUTW_NOW)) == BF_SHUTW_NOW &&
+ channel_is_empty(s->rep)))
si_shutw(s->rep->cons);
/* shutdown(write) done on the client side, we must stop the server too */
diff --git a/src/stream_interface.c b/src/stream_interface.c
index fe80b91..2066b06 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -146,7 +146,8 @@
if (si->state != SI_ST_EST)
return;
- if ((si->ob->flags & (BF_OUT_EMPTY|BF_SHUTW|BF_HIJACK|BF_SHUTW_NOW)) == (BF_OUT_EMPTY|BF_SHUTW_NOW))
+ if ((si->ob->flags & (BF_SHUTW|BF_HIJACK|BF_SHUTW_NOW)) == BF_SHUTW_NOW &&
+ channel_is_empty(si->ob))
si_shutw(si);
if ((si->ob->flags & (BF_FULL|BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK)) == 0)
@@ -155,7 +156,7 @@
/* we're almost sure that we need some space if the buffer is not
* empty, even if it's not full, because the applets can't fill it.
*/
- if ((si->ib->flags & (BF_SHUTR|BF_OUT_EMPTY|BF_DONT_READ)) == 0)
+ if ((si->ib->flags & (BF_SHUTR|BF_DONT_READ)) == 0 && !channel_is_empty(si->ib))
si->flags |= SI_FL_WAIT_ROOM;
if (si->ob->flags & BF_WRITE_ACTIVITY) {
@@ -175,7 +176,7 @@
(si->ob->prod->flags & SI_FL_WAIT_ROOM)))
si_chk_rcv(si->ob->prod);
- if (((si->ib->flags & (BF_READ_PARTIAL|BF_OUT_EMPTY)) == BF_READ_PARTIAL) &&
+ if (((si->ib->flags & BF_READ_PARTIAL) && !channel_is_empty(si->ib)) &&
(si->ib->cons->flags & SI_FL_WAIT_DATA)) {
si_chk_snd(si->ib->cons);
/* check if the consumer has freed some space */
@@ -205,7 +206,7 @@
((si->ob->flags & BF_WRITE_ACTIVITY) &&
((si->ob->flags & BF_SHUTW) ||
si->ob->prod->state != SI_ST_EST ||
- ((si->ob->flags & BF_OUT_EMPTY) && !si->ob->to_forward)))) {
+ (channel_is_empty(si->ob) && !si->ob->to_forward)))) {
if (!(si->flags & SI_FL_DONT_WAKE) && si->owner)
task_wakeup(si->owner, TASK_WOKEN_IO);
}
@@ -394,7 +395,7 @@
return;
if (!(si->flags & SI_FL_WAIT_DATA) || /* not waiting for data */
- (ob->flags & BF_OUT_EMPTY)) /* called with nothing to send ! */
+ channel_is_empty(ob)) /* called with nothing to send ! */
return;
/* Otherwise there are remaining data to be sent in the buffer,
@@ -577,7 +578,7 @@
}
/* process consumer side */
- if (si->ob->flags & BF_OUT_EMPTY) {
+ if (channel_is_empty(si->ob)) {
if (((si->ob->flags & (BF_SHUTW|BF_HIJACK|BF_SHUTW_NOW)) == BF_SHUTW_NOW) &&
(si->state == SI_ST_EST))
stream_int_shutw(si);
@@ -590,7 +591,8 @@
if (si->ob->flags & BF_WRITE_ACTIVITY) {
/* update timeouts if we have written something */
- if ((si->ob->flags & (BF_OUT_EMPTY|BF_SHUTW|BF_WRITE_PARTIAL)) == BF_WRITE_PARTIAL)
+ if ((si->ob->flags & (BF_SHUTW|BF_WRITE_PARTIAL)) == BF_WRITE_PARTIAL &&
+ !channel_is_empty(si->ob))
if (tick_isset(si->ob->wex))
si->ob->wex = tick_add_ifset(now_ms, si->ob->wto);
@@ -610,7 +612,7 @@
* immediately afterwards once the following data is parsed (eg:
* HTTP chunking).
*/
- if (((si->ib->flags & (BF_READ_PARTIAL|BF_OUT_EMPTY)) == BF_READ_PARTIAL) &&
+ if (((si->ib->flags & BF_READ_PARTIAL) && !channel_is_empty(si->ib)) &&
(si->ib->pipe /* always try to send spliced data */ ||
(si->ib->buf.i == 0 && (si->ib->cons->flags & SI_FL_WAIT_DATA)))) {
int last_len = si->ib->pipe ? si->ib->pipe->data : 0;
@@ -647,7 +649,7 @@
((si->ob->flags & BF_WRITE_ACTIVITY) &&
((si->ob->flags & BF_SHUTW) ||
si->ob->prod->state != SI_ST_EST ||
- ((si->ob->flags & BF_OUT_EMPTY) && !si->ob->to_forward)))) {
+ (channel_is_empty(si->ob) && !si->ob->to_forward)))) {
task_wakeup(si->owner, TASK_WOKEN_IO);
}
if (si->ib->flags & BF_READ_ACTIVITY)
@@ -691,10 +693,8 @@
/* At this point, the pipe is empty, but we may still have data pending
* in the normal buffer.
*/
- if (!b->buf.o) {
- b->flags |= BF_OUT_EMPTY;
+ if (!b->buf.o)
return 0;
- }
/* when we're in this loop, we already know that there is no spliced
* data left, and that there are sendable buffered data.
@@ -733,8 +733,6 @@
if (!b->buf.o) {
/* Always clear both flags once everything has been sent, they're one-shot */
b->flags &= ~(BF_EXPECT_MORE | BF_SEND_DONTWAIT);
- if (likely(!b->pipe))
- b->flags |= BF_OUT_EMPTY;
break;
}
@@ -799,7 +797,7 @@
/* Check if we need to close the write side */
if (!(ob->flags & BF_SHUTW)) {
/* Write not closed, update FD status and timeout for writes */
- if (ob->flags & BF_OUT_EMPTY) {
+ if (channel_is_empty(ob)) {
/* stop writing */
if (!(si->flags & SI_FL_WAIT_DATA)) {
if ((ob->flags & (BF_FULL|BF_HIJACK|BF_SHUTW_NOW)) == 0)
@@ -882,7 +880,7 @@
return;
}
- if (unlikely((ob->flags & BF_OUT_EMPTY))) /* called with nothing to send ! */
+ if (unlikely(channel_is_empty(ob))) /* called with nothing to send ! */
return;
if (!ob->pipe && /* spliced data wants to be forwarded ASAP */
@@ -904,7 +902,7 @@
/* OK, so now we know that some data might have been sent, and that we may
* have to poll first. We have to do that too if the buffer is not empty.
*/
- if (ob->flags & BF_OUT_EMPTY) {
+ if (channel_is_empty(ob)) {
/* the connection is established but we can't write. Either the
* buffer is empty, or we just refrain from sending because the
* ->o limit was reached. Maybe we just wrote the last
@@ -933,7 +931,8 @@
if (likely(ob->flags & BF_WRITE_ACTIVITY)) {
/* update timeout if we have written something */
- if ((ob->flags & (BF_OUT_EMPTY|BF_SHUTW|BF_WRITE_PARTIAL)) == BF_WRITE_PARTIAL)
+ if ((ob->flags & (BF_SHUTW|BF_WRITE_PARTIAL)) == BF_WRITE_PARTIAL &&
+ !channel_is_empty(ob))
ob->wex = tick_add_ifset(now_ms, ob->wto);
if (tick_isset(si->ib->rex) && !(si->flags & SI_FL_INDEP_STR)) {
@@ -953,7 +952,7 @@
* have to notify the task.
*/
if (likely((ob->flags & (BF_WRITE_NULL|BF_WRITE_ERROR|BF_SHUTW)) ||
- ((ob->flags & BF_OUT_EMPTY) && !ob->to_forward) ||
+ (channel_is_empty(ob) && !ob->to_forward) ||
si->state != SI_ST_EST)) {
out_wakeup:
if (!(si->flags & SI_FL_DONT_WAKE) && si->owner)
@@ -1031,7 +1030,6 @@
b->total += ret;
cur_read += ret;
b->flags |= BF_READ_PARTIAL;
- b->flags &= ~BF_OUT_EMPTY;
}
if (conn_data_read0_pending(conn))