Skip to content

Commit cdfa835

Browse files
shemmingergregkh
shemminger
authored and
gregkh
committed
uio_hv_generic: defer opening vmbus until first use
This fixes two design flaws in hv_uio_generic. Since hv_uio_probe is called from vmbus_probe with lock held it potentially can cause sleep in an atomic section because vmbus_open will wait for response from host. The hv_uio_generic driver could not handle applications exiting and restarting because the vmbus channel was persistent. Change the semantics so that the buffers are allocated on probe, but not attached to host until device is opened. Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 9da197f commit cdfa835

File tree

1 file changed

+74
-30
lines changed

1 file changed

+74
-30
lines changed

‎drivers/uio/uio_hv_generic.c

Lines changed: 74 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ enum hv_uio_map {
5555
struct hv_uio_private_data {
5656
struct uio_info info;
5757
struct hv_device *device;
58+
atomic_t refcnt;
5859

5960
void *recv_buf;
6061
u32 recv_gpadl;
@@ -128,12 +129,10 @@ static int hv_uio_ring_mmap(struct file *filp, struct kobject *kobj,
128129
{
129130
struct vmbus_channel *channel
130131
= container_of(kobj, struct vmbus_channel, kobj);
131-
struct hv_device *dev = channel->primary_channel->device_obj;
132-
u16 q_idx = channel->offermsg.offer.sub_channel_index;
133132
void *ring_buffer = page_address(channel->ringbuffer_page);
134133

135-
dev_dbg(&dev->device, "mmap channel %u pages %#lx at %#lx\n",
136-
q_idx, vma_pages(vma), vma->vm_pgoff);
134+
if (channel->state != CHANNEL_OPENED_STATE)
135+
return -ENODEV;
137136

138137
return vm_iomap_memory(vma, virt_to_phys(ring_buffer),
139138
channel->ringbuffer_pagecount << PAGE_SHIFT);
@@ -176,57 +175,103 @@ hv_uio_new_channel(struct vmbus_channel *new_sc)
176175
}
177176
}
178177

178+
/* free the reserved buffers for send and receive */
179179
static void
180180
hv_uio_cleanup(struct hv_device *dev, struct hv_uio_private_data *pdata)
181181
{
182-
if (pdata->send_gpadl)
182+
if (pdata->send_gpadl) {
183183
vmbus_teardown_gpadl(dev->channel, pdata->send_gpadl);
184-
vfree(pdata->send_buf);
184+
pdata->send_gpadl = 0;
185+
vfree(pdata->send_buf);
186+
}
185187

186-
if (pdata->recv_gpadl)
188+
if (pdata->recv_gpadl) {
187189
vmbus_teardown_gpadl(dev->channel, pdata->recv_gpadl);
188-
vfree(pdata->recv_buf);
190+
pdata->recv_gpadl = 0;
191+
vfree(pdata->recv_buf);
192+
}
193+
}
194+
195+
/* VMBus primary channel is opened on first use */
196+
static int
197+
hv_uio_open(struct uio_info *info, struct inode *inode)
198+
{
199+
struct hv_uio_private_data *pdata
200+
= container_of(info, struct hv_uio_private_data, info);
201+
struct hv_device *dev = pdata->device;
202+
int ret;
203+
204+
if (atomic_inc_return(&pdata->refcnt) != 1)
205+
return 0;
206+
207+
ret = vmbus_connect_ring(dev->channel,
208+
hv_uio_channel_cb, dev->channel);
209+
210+
if (ret == 0)
211+
dev->channel->inbound.ring_buffer->interrupt_mask = 1;
212+
else
213+
atomic_dec(&pdata->refcnt);
214+
215+
return ret;
216+
}
217+
218+
/* VMBus primary channel is closed on last close */
219+
static int
220+
hv_uio_release(struct uio_info *info, struct inode *inode)
221+
{
222+
struct hv_uio_private_data *pdata
223+
= container_of(info, struct hv_uio_private_data, info);
224+
struct hv_device *dev = pdata->device;
225+
int ret = 0;
226+
227+
if (atomic_dec_and_test(&pdata->refcnt))
228+
ret = vmbus_disconnect_ring(dev->channel);
229+
230+
return ret;
189231
}
190232

191233
static int
192234
hv_uio_probe(struct hv_device *dev,
193235
const struct hv_vmbus_device_id *dev_id)
194236
{
237+
struct vmbus_channel *channel = dev->channel;
195238
struct hv_uio_private_data *pdata;
239+
void *ring_buffer;
196240
int ret;
197241

242+
/* Communicating with host has to be via shared memory not hypercall */
243+
if (!channel->offermsg.monitor_allocated) {
244+
dev_err(&dev->device, "vmbus channel requires hypercall\n");
245+
return -ENOTSUPP;
246+
}
247+
198248
pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
199249
if (!pdata)
200250
return -ENOMEM;
201251

202-
ret = vmbus_open(dev->channel, HV_RING_SIZE * PAGE_SIZE,
203-
HV_RING_SIZE * PAGE_SIZE, NULL, 0,
204-
hv_uio_channel_cb, dev->channel);
252+
ret = vmbus_alloc_ring(channel, HV_RING_SIZE * PAGE_SIZE,
253+
HV_RING_SIZE * PAGE_SIZE);
205254
if (ret)
206255
goto fail;
207256

208-
/* Communicating with host has to be via shared memory not hypercall */
209-
if (!dev->channel->offermsg.monitor_allocated) {
210-
dev_err(&dev->device, "vmbus channel requires hypercall\n");
211-
ret = -ENOTSUPP;
212-
goto fail_close;
213-
}
214-
215-
dev->channel->inbound.ring_buffer->interrupt_mask = 1;
216-
set_channel_read_mode(dev->channel, HV_CALL_ISR);
257+
set_channel_read_mode(channel, HV_CALL_ISR);
217258

218259
/* Fill general uio info */
219260
pdata->info.name = "uio_hv_generic";
220261
pdata->info.version = DRIVER_VERSION;
221262
pdata->info.irqcontrol = hv_uio_irqcontrol;
263+
pdata->info.open = hv_uio_open;
264+
pdata->info.release = hv_uio_release;
222265
pdata->info.irq = UIO_IRQ_CUSTOM;
266+
atomic_set(&pdata->refcnt, 0);
223267

224268
/* mem resources */
225269
pdata->info.mem[TXRX_RING_MAP].name = "txrx_rings";
270+
ring_buffer = page_address(channel->ringbuffer_page);
226271
pdata->info.mem[TXRX_RING_MAP].addr
227-
= (uintptr_t)virt_to_phys(page_address(dev->channel->ringbuffer_page));
272+
= (uintptr_t)virt_to_phys(ring_buffer);
228273
pdata->info.mem[TXRX_RING_MAP].size
229-
= dev->channel->ringbuffer_pagecount << PAGE_SHIFT;
274+
= channel->ringbuffer_pagecount << PAGE_SHIFT;
230275
pdata->info.mem[TXRX_RING_MAP].memtype = UIO_MEM_IOVA;
231276

232277
pdata->info.mem[INT_PAGE_MAP].name = "int_page";
@@ -247,7 +292,7 @@ hv_uio_probe(struct hv_device *dev,
247292
goto fail_close;
248293
}
249294

250-
ret = vmbus_establish_gpadl(dev->channel, pdata->recv_buf,
295+
ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
251296
RECV_BUFFER_SIZE, &pdata->recv_gpadl);
252297
if (ret)
253298
goto fail_close;
@@ -261,14 +306,13 @@ hv_uio_probe(struct hv_device *dev,
261306
pdata->info.mem[RECV_BUF_MAP].size = RECV_BUFFER_SIZE;
262307
pdata->info.mem[RECV_BUF_MAP].memtype = UIO_MEM_VIRTUAL;
263308

264-
265309
pdata->send_buf = vzalloc(SEND_BUFFER_SIZE);
266310
if (pdata->send_buf == NULL) {
267311
ret = -ENOMEM;
268312
goto fail_close;
269313
}
270314

271-
ret = vmbus_establish_gpadl(dev->channel, pdata->send_buf,
315+
ret = vmbus_establish_gpadl(channel, pdata->send_buf,
272316
SEND_BUFFER_SIZE, &pdata->send_gpadl);
273317
if (ret)
274318
goto fail_close;
@@ -290,10 +334,10 @@ hv_uio_probe(struct hv_device *dev,
290334
goto fail_close;
291335
}
292336

293-
vmbus_set_chn_rescind_callback(dev->channel, hv_uio_rescind);
294-
vmbus_set_sc_create_callback(dev->channel, hv_uio_new_channel);
337+
vmbus_set_chn_rescind_callback(channel, hv_uio_rescind);
338+
vmbus_set_sc_create_callback(channel, hv_uio_new_channel);
295339

296-
ret = sysfs_create_bin_file(&dev->channel->kobj, &ring_buffer_bin_attr);
340+
ret = sysfs_create_bin_file(&channel->kobj, &ring_buffer_bin_attr);
297341
if (ret)
298342
dev_notice(&dev->device,
299343
"sysfs create ring bin file failed; %d\n", ret);
@@ -304,7 +348,6 @@ hv_uio_probe(struct hv_device *dev,
304348

305349
fail_close:
306350
hv_uio_cleanup(dev, pdata);
307-
vmbus_close(dev->channel);
308351
fail:
309352
kfree(pdata);
310353

@@ -322,7 +365,8 @@ hv_uio_remove(struct hv_device *dev)
322365
uio_unregister_device(&pdata->info);
323366
hv_uio_cleanup(dev, pdata);
324367
hv_set_drvdata(dev, NULL);
325-
vmbus_close(dev->channel);
368+
369+
vmbus_free_ring(dev->channel);
326370
kfree(pdata);
327371
return 0;
328372
}

0 commit comments

Comments
 (0)