qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: Anthony Liguori <aliguori@us.ibm.com>
Cc: qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: [PATCH 1/4] Add a scatter-gather list type and accessors
Date: Wed, 04 Feb 2009 22:30:05 +0200	[thread overview]
Message-ID: <4989FACD.6090309@redhat.com> (raw)
In-Reply-To: <4989EC29.9050705@us.ibm.com>

Anthony Liguori wrote:
> Avi Kivity wrote:
>> Scatter-gather lists are used extensively in dma-capable devices; a
>> single data structure allows more code reuse later on.
>>
>> diff --git a/dma-helpers.c b/dma-helpers.c
>> new file mode 100644
>> index 0000000..315834e
>> --- /dev/null
>> +++ b/dma-helpers.c
>> @@ -0,0 +1,29 @@
>>   
>
> Needs copyright/license.

Sure.

Is it possible to have a blanket license for files which don't have 
explicit terms?  I don't much like boilerplate.

>
>> +#include "dma.h"
>> +
>> +
>> +void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint)
>> +{
>> +    qsg->sg = qemu_malloc(alloc_hint * sizeof(ScatterGatherEntry));
>>   
>
> Would be nice to check for malloc failures and fail gracefully at least.

Do you mean an exit(1)?  If so we could just put it in qemu_malloc().

Propagation is usually not possible since hardware is not expected to 
run out of memory.  It will also be a never-tested code path.

>>
>> +void qemu_sglist_add(QEMUSGList *qsg, target_phys_addr_t base,
>> +                     target_phys_addr_t len)
>> +{
>> +    if (qsg->nsg == qsg->nalloc) {
>> +        qsg->nalloc = 2 * qsg->nalloc + 1;
>>   
>
> Do you really want exponential growth verses linear growth?  The sg 
> lists should be relatively small so linear growth should be fine.
>

I expect this to trigger rarely since the allocation hint should suffice 
nearly 100% of the time.  But in case we miss, it's better to reallocate 
as little as possible.

(what I really want is std::vector<>)

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

  reply	other threads:[~2009-02-04 20:29 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-04 12:25 [Qemu-devel] [PATCH 0/4] Block DMA helpers Avi Kivity
2009-02-04 12:25 ` [Qemu-devel] [PATCH 1/4] Add a scatter-gather list type and accessors Avi Kivity
2009-02-04 19:27   ` [Qemu-devel] " Anthony Liguori
2009-02-04 20:30     ` Avi Kivity [this message]
2009-02-04 20:36       ` Anthony Liguori
2009-02-04 20:46         ` Avi Kivity
2009-02-04 20:50           ` Anthony Liguori
2009-02-04 21:03             ` Avi Kivity
2009-02-04 23:58           ` Paul Brook
2009-02-05  7:25             ` Avi Kivity
2009-02-05  0:29         ` M. Warner Losh
2009-02-05  1:56           ` Anthony Liguori
2009-02-04 23:49     ` Paul Brook
2009-02-04 12:25 ` [Qemu-devel] [PATCH 2/4] Add qemu_iovec_reset() Avi Kivity
2009-02-04 12:25 ` [Qemu-devel] [PATCH 3/4] Introduce block dma helpers Avi Kivity
2009-02-04 19:29   ` [Qemu-devel] " Anthony Liguori
2009-02-04 12:25 ` [Qemu-devel] [PATCH 4/4] Convert IDE to use new " Avi Kivity

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=4989FACD.6090309@redhat.com \
    --to=avi@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=qemu-devel@nongnu.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;
as well as URLs for NNTP newsgroup(s).