Re: [PATCH 2/9] kvm tools: Add basic ioport dynamic allocation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* Sasha Levin <levinsasha928@xxxxxxxxx> wrote:

> +u16 ioport__find_free_range(void)
> +{
> +	static u16 cur_loc;

Please don't put statics inside function bodies! (even if they are 
only used within a single function)

I had to look three times to discover that it's really a global 
variable. These should be where other global variables are, or should 
be put before the function, in plain sight.

Also, if you do that i'd suggest a more descriptive name - 
free_io_port_idx perhaps?

> +	return IOPORT_START + (cur_loc++ * IOPORT_SIZE);

So this is SMP unsafe really. While ioport registrations are 
currently only used from initdev() functions and are thus serialized, 
it's not completely unfeasible that we would want to have async 
initcalls like the kernel does, to improve bootup/startup 
performance!

So it would be nice to make this all SMP safe. A single mutex would 
suffice i suspect.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux