From: Zhenhua Zhang <zhenhua.zhang@intel.com>
To: ofono@ofono.org
Subject: Re: [PATCH 1/4] Add write ring buffer for non-blocking write
Date: Thu, 11 Feb 2010 09:40:33 +0800 [thread overview]
Message-ID: <4B736011.90805@intel.com> (raw)
In-Reply-To: <20100210172337.GC6797@vigoh>
[-- Attachment #1: Type: text/plain, Size: 4116 bytes --]
Hi Padovan,
On 02/11/2010 01:23 AM, Gustavo F. Padovan wrote:
> * Zhenhua Zhang<zhenhua.zhang@intel.com> [2010-02-10 16:13:40 +0800]:
>> +
>> + 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
Good suggestion. We can use a macro to define the buffer size.
>> + if (!buf)
>> + return FALSE;
>> +
>> + server->free_list = g_slist_prepend(server->free_list, buf);
>> + }
>> +
>> + return TRUE;
>> +}
>> +
>> +static void free_ring_buffer(struct ring_buffer *buf)
>> +{
>> + ring_buffer_free(buf);
>> +}
>
> This seems pointless to me.
It just remind me to call ring_buffer_free. Yes. I can remove it and
call ring_buffer_free in g_slist_foreach() directly.
>> +
>> 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
>
prev parent reply other threads:[~2010-02-11 1:40 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 ` [PATCH 1/4] Add write ring buffer for non-blocking write Gustavo F. Padovan
2010-02-11 1:40 ` Zhenhua Zhang [this message]
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=4B736011.90805@intel.com \
--to=zhenhua.zhang@intel.com \
--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