Messages in this thread | | | Date | Tue, 14 Jul 2009 15:35:09 +0100 | From | Alan Cox <> | Subject | Re: [PATCH 1/2] vga: implements VGA arbitration on Linux |
| |
> +#ifndef __ARCH_HAS_VGA_ENABLE_RESOURCES > +static inline void vga_enable_resources(struct pci_dev *pdev, > + unsigned int rsrc) > +{ > + struct pci_bus *bus; > + struct pci_dev *bridge; > + u16 cmd; > + > +#ifdef DEBUG > + printk(KERN_DEBUG "%s\n", __func__); > +#endif > + pci_read_config_word(pdev, PCI_COMMAND, &cmd); > + if (rsrc & (VGA_RSRC_LEGACY_IO | VGA_RSRC_NORMAL_IO)) > + cmd |= PCI_COMMAND_IO; > + if (rsrc & (VGA_RSRC_LEGACY_MEM | VGA_RSRC_NORMAL_MEM)) > + cmd |= PCI_COMMAND_MEMORY; > + pci_write_config_word(pdev, PCI_COMMAND, cmd);
Locking question - what locks this lot against hotplug also touching bridge settings ?
> + > + if (!(rsrc & VGA_RSRC_LEGACY_MASK)) > + return; > + > + bus = pdev->bus; > + while (bus) { > + bridge = bus->self; > + if (bridge) { > + pci_read_config_word(bridge, PCI_BRIDGE_CONTROL, > + &cmd); > + if (!(cmd & PCI_BRIDGE_CTL_VGA)) { > + cmd |= PCI_BRIDGE_CTL_VGA; > + pci_write_config_word(bridge, > + PCI_BRIDGE_CONTROL, > + cmd); > + } > + } > + bus = bus->parent; > + } > +} > +#endif
> + /* The one who calls us should check for this, but lets be sure... */ > + if (pdev == NULL) > + pdev = vga_default_device();
What if the BIOS provided device was hot unplugged ?
> + conflict = __vga_tryget(vgadev, rsrc); > + spin_unlock_irqrestore(&vga_lock, flags); > + if (conflict == NULL) > + break; > + > + > + /* We have a conflict, we wait until somebody kicks the > + * work queue. Currently we have one work queue that we
If two drivers own half the resources and both are waiting for the rest what handles the deadlock
> + * kick each time some resources are released, but it would > + * be fairly easy to have a per device one so that we only > + * need to attach to the conflicting device > + */ > + init_waitqueue_entry(&wait, current); > + add_wait_queue(&vga_wait_queue, &wait); > + set_current_state(interruptible ? > + TASK_INTERRUPTIBLE : > + TASK_UNINTERRUPTIBLE); > + if (signal_pending(current)) { > + rc = -EINTR; > + break; > + } > + schedule(); > + remove_wait_queue(&vga_wait_queue, &wait); > + set_current_state(TASK_RUNNING);
Seems a very long winded way to write
wait_event_interruptible(...)
> + /* Allocate structure */ > + vgadev = kmalloc(sizeof(struct vga_device), GFP_KERNEL); > + if (vgadev == NULL) { > + /* What to do on allocation failure ? For now, let's > + * just do nothing, I'm not sure there is anything saner > + * to be done > + */
If this is an "oh dear" moment then at least printk something
> > + /* Set the client' lists of locks */ > + priv->target = vga_default_device(); /* Maybe this is still null! */
PCI device refcounting ?
| |