On Sat, 28 Nov 2009 05:20:35 pm Amit Shah wrote:
> The console could be flooded with data from the host; handle
> this situation by buffering the data.
All this complexity makes me really wonder if we should just
have the host say the max # ports it will ever use, and just do this
really dumbly. Yes, it's a limitation, but it'd be much simpler.
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -65,6 +65,23 @@ struct ports_device {
> * interrupt
> */
> struct work_struct rx_work;
> +
> + struct list_head unused_read_head;
You should name lists after plurals, rather than using "head" which is
an implementation detail. eg. "queued_inbufs" and below "used_inbufs".
Though Shirly Ma was working on a "destroy_bufs" patch which would avoid
your need for this list at all, AFAICT.
> + /* Return the number of bytes actually copied */
> + ret = copy_size;
> + buf->offset += ret;
> + out_offset += ret;
> + out_count -= ret;
We don't actually use ret.
> + if (buf->len - buf->offset == 0) {
I prefer the simpler "if (buf->offset == buf->len)" myself.
> + spin_lock_irqsave(&port->readbuf_list_lock, flags);
> + list_del(&buf->list);
> + spin_unlock_irqrestore(&port->readbuf_list_lock, flags);
> + kfree(buf->buf);
> + kfree(buf);
Does it become cleaner later to have this in a separate function? Usually
I prefer matching alloc and free fns.
> +static struct port_buffer *get_buf(size_t buf_size)
> +{
> + struct port_buffer *buf;
> +
> + buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> + if (!buf)
> + goto out;
> + buf->buf = kzalloc(buf_size, GFP_KERNEL);
> + if (!buf->buf) {
> + kfree(buf);
> + goto out;
No, that would return non-NULL. I'd stick with the standard multi-part exit:
if (!buf)
goto fail;
buf->buf = kzalloc(buf_size, GFP_KERNEL);
if (!buf->buf)
goto fail_free_buf;
buf->len = buf_size;
return buf;
fail_free_buf:
kfree(buf);
fail:
return NULL;
Thanks,
Rusty.
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/virtualization
