PCI Power Management / Interrupt Context

PCI Power Management / Interrupt Context

[David T Eger]

So I'm writing some code for a PCI card that is a framebuffer device, and
happily filling in the functions for the probe() and remove() functions
when I read documentation (Documentation/pci.txt) which mentions that
remove() can be called from interrupt context.

Now in order to properly tear down some things in the system, there are
some standard unregister() interfaces.  For instance,
unregister_framebuffer() and unregister_netdev().  After which, drivers
believe all is safe and go on about releasing ioremap()'d regions and
such.

One of the first things that unregister_netdev() does is to down() the
rtnl semaphore.  *cough* *cough*  This is a blocking operation, I believe,
which, umm - you can't do that from an interrupt, correct?

Reading code in my sister frame buffer devices, I see that
unregister_framebuffer() can fail.  I can easily see a nice happy console
or user app diddling away on the framebuffer writing to the memory on the
device, and then poof! someone yanks the card, processor takes an
interrupt, and then... and then?  what to do? 

In fact, here's an interesting snippet from cyber2000fb.c:

static void __devexit cyberpro_remove(struct pci_dev *dev)
{
	struct cfb_info *cfb = (struct cfb_info *)dev->driver_data;

	if (cfb) {
	/*
	* If unregister_framebuffer fails, then
	* we will be leaving hooks that could cause
	* oopsen laying around.
	*/
	if (unregister_framebuffer(&cfb->fb))
		printk(KERN_WARNING "%s: danger Will Robinson, "
			"danger danger!  Oopsen imminent!\n",
	cfb->fb.fix.id);
	cyberpro_unmap_smem(cfb);
	cyberpro_unmap_mmio(cfb);
	cyberpro_free_fb_info(cfb);

	/*
	* Ensure that the driver data is no longer
	* valid.
	*/
	dev->driver_data = NULL;
	if (cfb == int_cfb_info)
	int_cfb_info = NULL;
	}
}

So, umm, is there a consensus on what to do if someone currently expects
to be writing to memory that doesn't exist any more?  Leave the mappings
there?  Take them out and laugh as our users' machine suddenly oopses to a
halt? Pray? Laugh? Cry?
					
-David Thomas Eger (eger@cc.gatech.edu)

Re: PCI Power Management / Interrupt Context

[Jeff Garzik]

David T Eger wrote:
> when I read documentation (Documentation/pci.txt) which mentions that
> remove() can be called from interrupt context.

ignore that.  You can sleep in remove, and it will not be called from
interrupt context.


> Reading code in my sister frame buffer devices, I see that
> unregister_framebuffer() can fail.  I can easily see a nice happy console
> or user app diddling away on the framebuffer writing to the memory on the
> device, and then poof! someone yanks the card, processor takes an
> interrupt, and then... and then?  what to do?

if someone yanks the card, how is it going to deliver an interrupt to
the CPU?


> In fact, here's an interesting snippet from cyber2000fb.c:
> 
> static void __devexit cyberpro_remove(struct pci_dev *dev)
> {
>         struct cfb_info *cfb = (struct cfb_info *)dev->driver_data;
> 
>         if (cfb) {
>         /*
>         * If unregister_framebuffer fails, then
>         * we will be leaving hooks that could cause
>         * oopsen laying around.
>         */
>         if (unregister_framebuffer(&cfb->fb))
>                 printk(KERN_WARNING "%s: danger Will Robinson, "
>                         "danger danger!  Oopsen imminent!\n",
>         cfb->fb.fix.id);
>         cyberpro_unmap_smem(cfb);
>         cyberpro_unmap_mmio(cfb);
>         cyberpro_free_fb_info(cfb);
> 
>         /*
>         * Ensure that the driver data is no longer
>         * valid.
>         */
>         dev->driver_data = NULL;
>         if (cfb == int_cfb_info)
>         int_cfb_info = NULL;
>         }
> }
> 
> So, umm, is there a consensus on what to do if someone currently expects
> to be writing to memory that doesn't exist any more?

huh?  what are you talking about?  oops or random memory corruption
occurs.  there is no consensus necessary.

If you are worried about unregister_framebuffer failure, then don't
deallocate the memory, or sleep until unregister_framebuffer succeeds,
or any one of a number of workarounds.

	Jeff


-- 
Jeff Garzik      | Andre the Giant has a posse.
Building 1024    |
MandrakeSoft     |

Re: PCI Power Management / Interrupt Context

[Alan Cox]

> if someone yanks the card, how is it going to deliver an interrupt to
> the CPU?

It can happen actually. There is also a window where you can disable an IRQ
on a card and then take an IRQ. The ne2k driver has to jump through a couple
of hoops because of this

Re: PCI Power Management / Interrupt Context

[Alan Cox]

> when I read documentation (Documentation/pci.txt) which mentions that
> remove() can be called from interrupt context.

This I believe is in fact a documentation error

Re: PCI Power Management / Interrupt Context

[Linus Torvalds]

In article <Pine.SOL.4.21.0106262208240.3824-100000@oscar.cc.gatech.edu>,
David T Eger  <eger@cc.gatech.edu> wrote:
>
>So I'm writing some code for a PCI card that is a framebuffer device, and
>happily filling in the functions for the probe() and remove() functions
>when I read documentation (Documentation/pci.txt) which mentions that
>remove() can be called from interrupt context.

This used to be true for a short while for hot-plug CardBus. I don't
think it is true any more - and if it is, that would be a bug.

So I think it's the documentation that is in error, and we should just
fix that.

Jeff?

		Linus

Re: PCI Power Management / Interrupt Context

[Jeff Garzik]

Linus Torvalds wrote:
> 
> In article <Pine.SOL.4.21.0106262208240.3824-100000@oscar.cc.gatech.edu>,
> David T Eger  <eger@cc.gatech.edu> wrote:
> >
> >So I'm writing some code for a PCI card that is a framebuffer device, and
> >happily filling in the functions for the probe() and remove() functions
> >when I read documentation (Documentation/pci.txt) which mentions that
> >remove() can be called from interrupt context.
> 
> This used to be true for a short while for hot-plug CardBus. I don't
> think it is true any more - and if it is, that would be a bug.
> 
> So I think it's the documentation that is in error, and we should just
> fix that.
> 
> Jeff?

Correct, pci_driver::remove does not get called from interrupt context.

I don't know where the heck this thread is coming from ;-)  The docs
appear to be correct:

>         remove          Pointer to a function which gets called whenever a device
>                         being handled by this driver is removed (either during
>                         deregistration of the driver or when it's manually pulled
>                         out of a hot-pluggable slot). This function always gets
>                         called from process context, so it can sleep.

-- 
Jeff Garzik      | Andre the Giant has a posse.
Building 1024    |
MandrakeSoft     |