At Thu, 20 Oct 2011 19:02:50 +0400, Konstantin Ozerkov wrote:

This patch intended to improve performance in virtualized environments like Parallels Desktop or VirtualBox/QEMU (virtual ICH/AC97 audio).

I/O access is very time-expensive operation in virtual world: VCPU can be rescheduled and in the worst case we got more than 10ms delay on each I/O access.

In the original code normal loop exit rule (old_civ == current_civ && old_picb == current_picb) was never satisfied, because old_picb was never the same as current_picb due delay inspired by reading current_civ. As a result loop ended by timeout and we got 10x more I/O operations.

You mean in the virtual environment, right? On the real machines, this condition can be satisfied normally because the read is much faster than the sample rate. This check is needed to avoid the broken behavior around the boundary.

To prevent glitch on buffer swapping the rule (old_civ == current_civ) is enough.

Not for real (thus less logical) machines, unfortunately. Both checks were needed in the real tests.

I don't mind to implement some bypass if the driver knows that it's running in a VM. But, removing blindly would cause definitely a regression.

thanks,

Takashi

Experimental data from Prallels Desktop 7, RHEL6 guest (I/O ops per second):

Original code: In Port Counter Callback f014 41550 fffff00000179d00 ac97_bm_read_civ+0x000 f018 41387 fffff0000017a580 ac97_bm_read_picb+0x000

With patch: In Port Counter Callback f014 4090 fffff00000179d00 ac97_bm_read_civ+0x000 f018 1964 fffff0000017a580 ac97_bm_read_picb+0x000

Signed-off-by: Konstantin Ozerkov kozerkov@parallels.com Signed-off-by: Denis V. Lunev den@parallels.com

diff --git a/pci/intel8x0.c b/pci/intel8x0.c index 6a5b387..bad4a65 100644 --- a/pci/intel8x0.c +++ b/pci/intel8x0.c @@ -1065,8 +1065,7 @@ static snd_pcm_uframes_t snd_intel8x0_pcm_pointer(struct snd_pcm_substream *subs udelay(10); continue; }

• if (civ == igetbyte(chip, ichdev->reg_offset + ICH_REG_OFF_CIV) &&
  

•     ptr1 == igetword(chip, ichdev->reg_offset + ichdev->roff_picb))
  

• if (civ == igetbyte(chip, ichdev->reg_offset + ICH_REG_OFF_CIV))
  break;
  

  } while (timeout--); ptr = ichdev->last_pos;

-- 1.7.7