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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox