linux-kernel - Re: virtio: Add memory statistics reporting to the balloon driver (V2)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200911181200.39971.rusty@rustcorp.com.au>
Date:	Wed, 18 Nov 2009 12:00:39 +1030
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Adam Litke <agl@...ibm.com>
Cc:	qemu-devel@...gnu.org, Anthony Liguori <aliguori@...ibm.com>,
	Avi Kivity <avi@...hat.com>,
	virtualization@...ts.linux-foundation.org,
	linux-kernel@...r.kernel.org
Subject: Re: virtio: Add memory statistics reporting to the balloon driver (V2)

On Wed, 18 Nov 2009 07:06:29 am Adam Litke wrote:
> virtio: Add memory statistics reporting to the balloon driver (V2)
> 
> Changes since V1:
>  - Use a virtqueue instead of the device config space

Hi Adam,

    If Anthony's happy, I'm happy with this approach.

Couple of minor points:

> +static inline void update_stat(struct virtio_balloon *vb, int idx,
> +				unsigned int tag, unsigned long val)
> +{
> +	BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
> +	vb->stats[idx].tag = tag;
> +	vb->stats[idx].val = cpu_to_le32(val);
> +}

The little-endian conversion of the balloon driver is a historical mistake
(no other driver does this).  Let's not extend it to the stats.

Here you've done one and not the other, which is even worse.  (Sparse would
have found this, I assume).

> +struct virtio_balloon_stat
> +{
> +	__le16 tag;
> +	__le32 val;
> +};

Is 32 bits sufficient?  A big machine might get over 4bn faults, and certainly
4 TB of memory isn't that far away.

Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/