From: Gustavo F. Padovan <padovan@profusion.mobi>
To: ofono@ofono.org
Subject: Re: [PATCH 1/4] Add write ring buffer for non-blocking write
Date: Wed, 10 Feb 2010 15:23:37 -0200 [thread overview]
Message-ID: <20100210172337.GC6797@vigoh> (raw)
In-Reply-To: <1265789623-30146-1-git-send-email-zhenhua.zhang@intel.com>
[-- Attachment #1: Type: text/plain, Size: 7054 bytes --]
* Zhenhua Zhang <zhenhua.zhang@intel.com> [2010-02-10 16:13:40 +0800]:
> Use two layers to cache server side response data to client.
> 1. A fixed-length ring buffer.
> 2. A list of free ring buffers and a list of empty full ring buffer.
>
> If current ring buffer is full, put it into full buffer list and fetch
> a free buffer frome free list.
> ---
> gatchat/gatserver.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 104 insertions(+), 7 deletions(-)
>
> diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
> index bf7e847..6b40084 100644
> --- a/gatchat/gatserver.c
> +++ b/gatchat/gatserver.c
> @@ -32,6 +32,9 @@
> #include "ringbuffer.h"
> #include "gatserver.h"
>
> +#define MAX_BUFFER_NUM 5
> +/* #define WRITE_SCHEDULER_DEBUG 1 */
> +
> enum ParserState {
> PARSER_STATE_IDLE,
> PARSER_STATE_A,
> @@ -90,17 +93,54 @@ struct _GAtServer {
> struct v250_settings v250; /* V.250 command setting */
> GIOChannel *channel; /* Server IO */
> guint read_watch; /* GSource read id, 0 if none */
> + guint write_watch; /* GSource write id, 0 if none */
> guint read_so_far; /* Number of bytes processed */
> GAtDisconnectFunc user_disconnect; /* User disconnect func */
> gpointer user_disconnect_data; /* User disconnect data */
> GAtDebugFunc debugf; /* Debugging output function */
> gpointer debug_data; /* Data to pass to debug func */
> struct ring_buffer *read_buf; /* Current read buffer */
> + struct ring_buffer *write_buf; /* Current write buffer */
> + GSList *full_list; /* List of full ring buffer */
> + GSList *free_list; /* List of free ring buffer */
> guint max_read_attempts; /* Max reads per select */
> enum ParserState parser_state;
> gboolean destroyed; /* Re-entrancy guard */
> };
>
> +static void g_at_server_wakeup_writer(GAtServer *server);
> +
> +static gboolean alloc_free_list(GAtServer *server)
> +{
> + struct ring_buffer *buf;
> + guint i;
> +
> + for (i = 0; i < MAX_BUFFER_NUM; i++) {
> +#ifdef WRITE_SCHEDULER_DEBUG
> + buf = ring_buffer_new(4);
> +#else
> + buf = ring_buffer_new(4096);
> +#endif
Why not a macro to define buf size? So you don't need to use ifdef
everytime
#ifdef WRITE_SCHEDULER_DEBUG
BUF_SIZE = 4
#else
BUF_SIZE = 4096
#endif
> + if (!buf)
> + return FALSE;
> +
> + server->free_list = g_slist_prepend(server->free_list, buf);
> + }
> +
> + return TRUE;
> +}
> +
> +static void send_common(GAtServer *server, const char *buf)
> +{
> + gsize avail = ring_buffer_avail(server->write_buf);
> + gsize len = strlen(buf);
> +
> + if (avail >= len)
> + ring_buffer_write(server->write_buf, buf, len);
> +
> + g_at_server_wakeup_writer(server);
> +}
> +
> static void g_at_server_send_result(GAtServer *server, GAtServerResult result)
> {
> struct v250_settings v250 = server->v250;
> @@ -108,7 +148,6 @@ static void g_at_server_send_result(GAtServer *server, GAtServerResult result)
> char buf[1024];
> char t = v250.s3;
> char r = v250.s4;
> - gsize wbuf;
>
> if (v250.quiet)
> return;
> @@ -125,8 +164,7 @@ static void g_at_server_send_result(GAtServer *server, GAtServerResult result)
> g_at_util_debug_chat(FALSE, buf, strlen(buf),
> server->debugf, server->debug_data);
>
> - g_io_channel_write(server->channel, (char *) buf, strlen(buf),
> - &wbuf);
> + send_common(server, buf);
> }
>
> static inline gboolean is_at_command_prefix(const char c)
> @@ -432,12 +470,35 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond,
> return TRUE;
> }
>
> +static gboolean can_write_data(GIOChannel *channel, GIOCondition cond,
> + gpointer data)
> +{
> + return FALSE;
> +}
> +
> +static void free_ring_buffer(struct ring_buffer *buf)
> +{
> + ring_buffer_free(buf);
> +}
This seems pointless to me.
> +
> static void g_at_server_cleanup(GAtServer *server)
> {
> /* Cleanup all received data */
> ring_buffer_free(server->read_buf);
> server->read_buf = NULL;
>
> + /* Cleanup pending data to write */
> + ring_buffer_free(server->write_buf);
> + server->write_buf = NULL;
> +
> + if (server->full_list)
> + g_slist_foreach(server->full_list, (GFunc)free_ring_buffer,
> + NULL);
> +
> + if (server->free_list)
> + g_slist_foreach(server->free_list, (GFunc)free_ring_buffer,
> + NULL);
> +
> server->channel = NULL;
> }
>
> @@ -446,8 +507,6 @@ static void read_watcher_destroy_notify(GAtServer *server)
> g_at_server_cleanup(server);
> server->read_watch = 0;
>
> - server->channel = NULL;
> -
> if (server->user_disconnect)
> server->user_disconnect(server->user_disconnect_data);
>
> @@ -455,6 +514,23 @@ static void read_watcher_destroy_notify(GAtServer *server)
> g_free(server);
> }
>
> +static void write_watcher_destroy_notify(GAtServer *server)
> +{
> + server->write_watch = 0;
> +}
> +
> +static void g_at_server_wakeup_writer(GAtServer *server)
> +{
> + if (server->write_watch != 0)
> + return;
> +
> + server->write_watch = g_io_add_watch_full(server->channel,
> + G_PRIORITY_DEFAULT,
> + G_IO_OUT | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> + can_write_data, server,
> + (GDestroyNotify)write_watcher_destroy_notify);
> +}
> +
> static void v250_settings_create(struct v250_settings *v250)
> {
> v250->s3 = '\r';
> @@ -483,11 +559,22 @@ GAtServer *g_at_server_new(GIOChannel *io)
> v250_settings_create(&server->v250);
> server->channel = io;
> server->read_buf = ring_buffer_new(4096);
> - server->max_read_attempts = 3;
> -
> if (!server->read_buf)
> goto error;
>
> +#ifdef WRITE_SCHEDULER_DEBUG
> + server->write_buf = ring_buffer_new(4);
> +#else
> + server->write_buf = ring_buffer_new(4096);
> +#endif
> + if (!server->write_buf)
> + goto error;
> +
> + if (!alloc_free_list(server))
> + goto error;
> +
> + server->max_read_attempts = 3;
> +
> if (!g_at_util_setup_io(server->channel, G_IO_FLAG_NONBLOCK))
> goto error;
>
> @@ -502,6 +589,13 @@ error:
> if (server->read_buf)
> ring_buffer_free(server->read_buf);
>
> + if (server->write_buf)
> + ring_buffer_free(server->write_buf);
> +
> + if (server->free_list)
> + g_slist_foreach(server->free_list, (GFunc)free_ring_buffer,
> + NULL);
> +
> if (server)
> g_free(server);
>
> @@ -552,6 +646,9 @@ gboolean g_at_server_shutdown(GAtServer *server)
> server->user_disconnect = NULL;
> server->user_disconnect_data = NULL;
>
> + if (server->write_watch)
> + g_source_remove(server->write_watch);
> +
> if (server->read_watch)
> g_source_remove(server->read_watch);
>
> --
> 1.6.6.1
>
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
--
Gustavo F. Padovan
http://padovan.org
ProFUSION embedded systems - http://profusion.mobi
next prev parent reply other threads:[~2010-02-10 17:23 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-10 8:13 [PATCH 1/4] Add write ring buffer for non-blocking write Zhenhua Zhang
2010-02-10 8:13 ` [PATCH 2/4] Add handle the case when write buffer is full Zhenhua Zhang
2010-02-10 8:13 ` [PATCH 3/4] Add ring_buffer_read_advance for partial write Zhenhua Zhang
2010-02-10 8:13 ` [PATCH 4/4] Add write server response into non blocking IO Zhenhua Zhang
2010-02-10 15:36 ` [PATCH 3/4] Add ring_buffer_read_advance for partial write Denis Kenzior
2010-02-10 17:23 ` Gustavo F. Padovan [this message]
2010-02-11 1:40 ` [PATCH 1/4] Add write ring buffer for non-blocking write Zhenhua Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100210172337.GC6797@vigoh \
--to=padovan@profusion.mobi \
--cc=ofono@ofono.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.