Re: [PATCH v3 2/3] hvc_init(): Enforce one-time initialization.
From: Michael Ellerman <hidden>
Date: 2011-11-09 07:24:16
Also in:
linuxppc-dev, virtualization, xen-devel
On Tue, 2011-11-08 at 13:45 -0800, Miche Baker-Harvey wrote:
hvc_init() must only be called once, and no thread should continue with hvc_alloc() until after initialization is complete. The original code does not enforce either of these requirements. A new mutex limits entry to hvc_init() to a single thread, and blocks all later comers until it has completed. This patch fixes multiple crash symptoms.
Hi Miche, A few nit-picky comments below ..
quoted hunk ↗ jump to hunk
@@ -84,6 +85,10 @@ static LIST_HEAD(hvc_structs); * list traversal. */ static DEFINE_SPINLOCK(hvc_structs_lock); +/* + * only one task does allocation at a time. + */ +static DEFINE_MUTEX(hvc_ports_mutex);
The comment is wrong, isn't it? Only one task does _init_ at a time. Once the driver is initialised allocs can run concurrently. So shouldn't it be called hvc_init_mutex ?
quoted hunk ↗ jump to hunk
@@ -825,11 +830,15 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data, int i; /* We wait until a driver actually comes along */ + mutex_lock(&hvc_ports_mutex); if (!hvc_driver) { int err = hvc_init(); - if (err) + if (err) { + mutex_unlock(&hvc_ports_mutex); return ERR_PTR(err); + } } + mutex_unlock(&hvc_ports_mutex); hp = kzalloc(ALIGN(sizeof(*hp), sizeof(long)) + outbuf_size, GFP_KERNEL);
It'd be cleaner I think to do all the locking in hvc_init(). That's safe as long as you recheck !hvc_driver in hvc_init() with the lock held. cheers
Attachments
- signature.asc [application/pgp-signature] 198 bytes