qemu write cacheing and DMA IDE writes

qemu write cacheing and DMA IDE writes

[Ian Jackson]

I've been doing some merge work between tools/ioemu and qemu
upstream.  I came across this commit:

  changeset:   11209:9bb6c1c1890a07885265bbc59f4dbb660312974e
  date:        Sun Aug 20 23:59:34 2006 +0100
  files:       [...]
  description:

  [qemu] hdparm tunable IDE write cache for HVM

  qemu 0.8.2 has a flush callback to the storage backends, so now it is
  possible to implement hdparm tunable IDE write cache enable/disable for
  guest domains, allowing people to pick speed or data consistency on a
  case by case basis.

  As an added benefit, really large LBA48 IOs will now no longer be broken
  up into smaller IOs on the host side.

  From: Rik van Riel <riel@redhat.com>
  Signed-off-by: Christian Limpach <Christian.Limpach@xensource.com>

However there seems to me to a be a bug in it: it does not take effect
for DMA writes, which are handled by a separate set of functions.
Since most guest operating systems will be using (emulated) DMA, it
seems that the result is that we advertise configurable write cacheing
but in fact in most cases always cache.

To implement configurable write cacheing for DMA would be possible but
would involve introducing various new complications to arrange to call
an aio fsync, or pass the cacheing flag into the underlying block
implementations.

Also, according to the ATA spec the `turn off write cache' command
must also flush the cache, and that wasn't done.

My question is: given how long this has been like this, do we care ?
It seems likely that HVM guests switching from emulated IDE to PV
drivers may make use of some of the flushing facilities but I'm not
aware of the details.

My options wrt the qemu merge are to drop all of these related
changes, retain what we have but leave DMA transfers always cached, or
to fix it properly.

Ian.

Re: qemu write cacheing and DMA IDE writes

[Daniel P. Berrange]

On Mon, Feb 25, 2008 at 04:48:21PM +0000, Ian Jackson wrote:
> I've been doing some merge work between tools/ioemu and qemu
> upstream.  I came across this commit:
> 
>   changeset:   11209:9bb6c1c1890a07885265bbc59f4dbb660312974e
>   date:        Sun Aug 20 23:59:34 2006 +0100
>   files:       [...]
>   description:
> 
>   [qemu] hdparm tunable IDE write cache for HVM
> 
>   qemu 0.8.2 has a flush callback to the storage backends, so now it is
>   possible to implement hdparm tunable IDE write cache enable/disable for
>   guest domains, allowing people to pick speed or data consistency on a
>   case by case basis.
> 
>   As an added benefit, really large LBA48 IOs will now no longer be broken
>   up into smaller IOs on the host side.
> 
>   From: Rik van Riel <riel@redhat.com>
>   Signed-off-by: Christian Limpach <Christian.Limpach@xensource.com>
> 
> However there seems to me to a be a bug in it: it does not take effect
> for DMA writes, which are handled by a separate set of functions.
> Since most guest operating systems will be using (emulated) DMA, it
> seems that the result is that we advertise configurable write cacheing
> but in fact in most cases always cache.

Are you sure it doesn't apply for DMA writes ?

AFAICT, the DMA write is done by ide_write_dma_cb() in hw/ide.c. At the
point the DMA transfer completes that method does

       if (!s->write_cache)
            bdrv_flush(s->bs);

Which should flush the data to disk, if the guest has done hdparm -W 0
on their device.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

Re: qemu write cacheing and DMA IDE writes

[Ian Jackson]

Daniel P. Berrange writes ("Re: [Xen-devel] qemu write cacheing and DMA IDE writes"):
> AFAICT, the DMA write is done by ide_write_dma_cb() in hw/ide.c. At the
> point the DMA transfer completes that method does
> 
>        if (!s->write_cache)
>             bdrv_flush(s->bs);
> 
> Which should flush the data to disk, if the guest has done hdparm -W 0
> on their device.

Oh, yes, sorry, I missed that.  I'll add an error check for it to my
merge, though.

Ian.