[Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size.

[Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size.

[Ian Jackson]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 450 bytes --]

In this patch we increase the DMA buffers in the ide emulation from 8k
to 132k, and make it configurable separately from the MAX_MULT_SECTORS
for maximum multi-mode transfers.  This can improve the performance in
some conditions.

The patch is cross-ported from xen-unstable hg changeset
 f4a92f0db20fda98a633c149e3396c005a759a77

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>


[-- Attachment #2: increase ide DMA buffers --]
[-- Type: text/plain, Size: 2446 bytes --]

>From 4eaa19458262cce92f0b891c51433ccee0a19b79 Mon Sep 17 00:00:00 2001
From: Ian Jackson <iwj@mariner.uk.xensource.com>
Date: Wed, 26 Mar 2008 16:09:35 +0000
Subject: [PATCH] Improve DMA transfers by increasing the size of DMA buffers.

This involves a new constant IDE_DMA_BUF_SIZE which is separate from
MAX_MULT_SECTORS.

Cross-ported from xen-unstable
 17267:f4a92f0db20fda98a633c149e3396c005a759a77

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>
---
 hw/ide.c |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/hw/ide.c b/hw/ide.c
index 56a1cda..b73dba2 100644
--- a/hw/ide.c
+++ b/hw/ide.c
@@ -202,6 +202,12 @@
 /* set to 1 set disable mult support */
 #define MAX_MULT_SECTORS 16
 
+#define IDE_DMA_BUF_SIZE 131072
+
+#if (IDE_DMA_BUF_SIZE < MAX_MULT_SECTORS * 512)
+#error "IDE_DMA_BUF_SIZE must be bigger or equal to MAX_MULT_SECTORS * 512"
+#endif
+
 /* ATAPI defines */
 
 #define ATAPI_PACKET_SIZE 12
@@ -920,8 +926,8 @@ static void ide_read_dma_cb(void *opaque, int ret)
 
     /* launch next transfer */
     n = s->nsector;
-    if (n > MAX_MULT_SECTORS)
-        n = MAX_MULT_SECTORS;
+    if (n > IDE_DMA_BUF_SIZE / 512)
+        n = IDE_DMA_BUF_SIZE / 512;
     s->io_buffer_index = 0;
     s->io_buffer_size = n * 512;
 #ifdef DEBUG_AIO
@@ -1028,8 +1034,8 @@ static void ide_write_dma_cb(void *opaque, int ret)
 
     /* launch next transfer */
     n = s->nsector;
-    if (n > MAX_MULT_SECTORS)
-        n = MAX_MULT_SECTORS;
+    if (n > IDE_DMA_BUF_SIZE / 512)
+        n = IDE_DMA_BUF_SIZE / 512;
     s->io_buffer_index = 0;
     s->io_buffer_size = n * 512;
 
@@ -1323,8 +1329,8 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
         data_offset = 16;
     } else {
         n = s->packet_transfer_size >> 11;
-        if (n > (MAX_MULT_SECTORS / 4))
-            n = (MAX_MULT_SECTORS / 4);
+        if (n > (IDE_DMA_BUF_SIZE / 2048))
+            n = (IDE_DMA_BUF_SIZE / 2048);
         s->io_buffer_size = n * 2048;
         data_offset = 0;
     }
@@ -2557,7 +2563,7 @@ static void ide_init2(IDEState *ide_state,
 
     for(i = 0; i < 2; i++) {
         s = ide_state + i;
-        s->io_buffer = qemu_memalign(512, MAX_MULT_SECTORS*512 + 4);
+        s->io_buffer = qemu_memalign(512, IDE_DMA_BUF_SIZE + 4);
         if (i == 0)
             s->bs = hd0;
         else
-- 
1.4.4.4

Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size.

[Aurelien Jarno]

On Wed, Mar 26, 2008 at 04:35:00PM +0000, Ian Jackson wrote:
Content-Description: message body text
> In this patch we increase the DMA buffers in the ide emulation from 8k
> to 132k, and make it configurable separately from the MAX_MULT_SECTORS
> for maximum multi-mode transfers.  This can improve the performance in
> some conditions.
> 
> The patch is cross-ported from xen-unstable hg changeset
>  f4a92f0db20fda98a633c149e3396c005a759a77
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>
> 

Content-Description: increase ide DMA buffers
> From 4eaa19458262cce92f0b891c51433ccee0a19b79 Mon Sep 17 00:00:00 2001
> From: Ian Jackson <iwj@mariner.uk.xensource.com>
> Date: Wed, 26 Mar 2008 16:09:35 +0000
> Subject: [PATCH] Improve DMA transfers by increasing the size of DMA buffers.
> 
> This involves a new constant IDE_DMA_BUF_SIZE which is separate from
> MAX_MULT_SECTORS.
> 
> Cross-ported from xen-unstable
>  17267:f4a92f0db20fda98a633c149e3396c005a759a77
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>
> ---
>  hw/ide.c |   20 +++++++++++++-------
>  1 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ide.c b/hw/ide.c
> index 56a1cda..b73dba2 100644
> --- a/hw/ide.c
> +++ b/hw/ide.c
> @@ -202,6 +202,12 @@
>  /* set to 1 set disable mult support */
>  #define MAX_MULT_SECTORS 16
>  
> +#define IDE_DMA_BUF_SIZE 131072

Wouldn't it be better to define this value in number of sectors? This
would avoid a few divisions in the code, and anyway the code handling
DMA transfers is working with a number of sectors, not a number of 
bytes.

> +#if (IDE_DMA_BUF_SIZE < MAX_MULT_SECTORS * 512)
> +#error "IDE_DMA_BUF_SIZE must be bigger or equal to MAX_MULT_SECTORS * 512"
> +#endif
> +
>  /* ATAPI defines */
>  
>  #define ATAPI_PACKET_SIZE 12
> @@ -920,8 +926,8 @@ static void ide_read_dma_cb(void *opaque, int ret)
>  
>      /* launch next transfer */
>      n = s->nsector;
> -    if (n > MAX_MULT_SECTORS)
> -        n = MAX_MULT_SECTORS;
> +    if (n > IDE_DMA_BUF_SIZE / 512)
> +        n = IDE_DMA_BUF_SIZE / 512;
>      s->io_buffer_index = 0;
>      s->io_buffer_size = n * 512;
>  #ifdef DEBUG_AIO
> @@ -1028,8 +1034,8 @@ static void ide_write_dma_cb(void *opaque, int ret)
>  
>      /* launch next transfer */
>      n = s->nsector;
> -    if (n > MAX_MULT_SECTORS)
> -        n = MAX_MULT_SECTORS;
> +    if (n > IDE_DMA_BUF_SIZE / 512)
> +        n = IDE_DMA_BUF_SIZE / 512;
>      s->io_buffer_index = 0;
>      s->io_buffer_size = n * 512;
>  
> @@ -1323,8 +1329,8 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
>          data_offset = 16;
>      } else {
>          n = s->packet_transfer_size >> 11;
> -        if (n > (MAX_MULT_SECTORS / 4))
> -            n = (MAX_MULT_SECTORS / 4);
> +        if (n > (IDE_DMA_BUF_SIZE / 2048))
> +            n = (IDE_DMA_BUF_SIZE / 2048);
>          s->io_buffer_size = n * 2048;
>          data_offset = 0;
>      }
> @@ -2557,7 +2563,7 @@ static void ide_init2(IDEState *ide_state,
>  
>      for(i = 0; i < 2; i++) {
>          s = ide_state + i;
> -        s->io_buffer = qemu_memalign(512, MAX_MULT_SECTORS*512 + 4);
> +        s->io_buffer = qemu_memalign(512, IDE_DMA_BUF_SIZE + 4);
>          if (i == 0)
>              s->bs = hd0;
>          else
> -- 
> 1.4.4.4
> 


-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size.

[Samuel Thibault]

Aurelien Jarno, le Fri 28 Mar 2008 00:53:40 +0100, a écrit :
> > +#define IDE_DMA_BUF_SIZE 131072
> 
> Wouldn't it be better to define this value in number of sectors? This
> would avoid a few divisions in the code, and anyway the code handling
> DMA transfers is working with a number of sectors, not a number of 
> bytes.

Right.

Samuel

Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size.

[Ian Jackson]

Aurelien Jarno writes ("Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size."):
> On Wed, Mar 26, 2008 at 04:35:00PM +0000, Ian Jackson wrote:
> Content-Description: message body text
> > +#define IDE_DMA_BUF_SIZE 131072
> 
> Wouldn't it be better to define this value in number of sectors? This
> would avoid a few divisions in the code, and anyway the code handling
> DMA transfers is working with a number of sectors, not a number of 
> bytes.

I don't have much of an opinion about what units it should be in.  The
divisions are just at compile-time of course.

If you prefer I'll send another version with it done in sectors :-).

Ian.

Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size.

[Aurelien Jarno]

On Fri, Mar 28, 2008 at 11:35:52AM +0000, Ian Jackson wrote:
> Aurelien Jarno writes ("Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size."):
> > On Wed, Mar 26, 2008 at 04:35:00PM +0000, Ian Jackson wrote:
> > Content-Description: message body text
> > > +#define IDE_DMA_BUF_SIZE 131072
> > 
> > Wouldn't it be better to define this value in number of sectors? This
> > would avoid a few divisions in the code, and anyway the code handling
> > DMA transfers is working with a number of sectors, not a number of 
> > bytes.
> 
> I don't have much of an opinion about what units it should be in.  The
> divisions are just at compile-time of course.

I agree that the generated assembly code should not be different, but
this makes the code less readable.

> If you prefer I'll send another version with it done in sectors :-).

Please send it, I will commit it. 

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size.

[Aurelien Jarno]

On Fri, Mar 28, 2008 at 05:22:30PM +0100, Aurelien Jarno wrote:
> On Fri, Mar 28, 2008 at 11:35:52AM +0000, Ian Jackson wrote:
> > Aurelien Jarno writes ("Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size."):
> > > On Wed, Mar 26, 2008 at 04:35:00PM +0000, Ian Jackson wrote:
> > > Content-Description: message body text
> > > > +#define IDE_DMA_BUF_SIZE 131072
> > > 
> > > Wouldn't it be better to define this value in number of sectors? This
> > > would avoid a few divisions in the code, and anyway the code handling
> > > DMA transfers is working with a number of sectors, not a number of 
> > > bytes.
> > 
> > I don't have much of an opinion about what units it should be in.  The
> > divisions are just at compile-time of course.
> 
> I agree that the generated assembly code should not be different, but
> this makes the code less readable.
> 
> > If you prefer I'll send another version with it done in sectors :-).
> 
> Please send it, I will commit it. 
> 

I have just done the changes myself and committed the result to the CVS.

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size.

[Avi Kivity]

Ian Jackson wrote:
> Aurelien Jarno writes ("Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size."):
>   
>> On Wed, Mar 26, 2008 at 04:35:00PM +0000, Ian Jackson wrote:
>> Content-Description: message body text
>>     
>>> +#define IDE_DMA_BUF_SIZE 131072
>>>       
>> Wouldn't it be better to define this value in number of sectors? This
>> would avoid a few divisions in the code, and anyway the code handling
>> DMA transfers is working with a number of sectors, not a number of 
>> bytes.
>>     
>
> I don't have much of an opinion about what units it should be in.  The
> divisions are just at compile-time of course.
>
>   

I prefer bytes myself, this way it is easy to have all constants using 
the same scale factor (1) and avoid worrying about conversions, except 
at the last moment when using them.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size.

[Ian Jackson]

Aurelien Jarno writes ("Re: [Qemu-devel] [PATCH] Improve DMA transfers by increasing the buffer size."):
> I have just done the changes myself and committed the result to the CVS.

Thanks, that's great.

Ian.