Re: [PATCH 5/8] staging: vme: add functions for bridge module refcounting

[Martyn Welch]

On 08/08/11 11:11, Emilio G. Cota wrote:
> On Mon, Aug 08, 2011 at 10:26:50 +0100, Martyn Welch wrote:
>> On 08/08/11 10:14, Emilio G. Cota wrote:
>>> Martyn, no one in the kernel is doing what you propose, for
>>> good reason. Look at USB, PCI, RapidIO.  They all provide get
>>> and put functions to be called from probe and release.
>>
>> Really, which functions are these for PCI?
> 
> pci_get_dev/put in drivers/pci/pci-driver.c.
> 

Which isn't explicitly used by the vast majority of PCI drivers. In fact the
instances which I did find where this was used by a PCI device driver, it
appears to be using the old-style PCI probing.

This is taken care of automatically for drivers using the "newer" PCI driver
registration model as part of the probe.

>>> Doing otherwise is a bug. If a driver needs a resource *not
>>> necessarily at .probe time*, it increments the refcount
>>> then (in .probe) to make sure that the parent driver won't
>>> be removed. IOW if you increase the refcount in the bridge
>>> driver only when a resource is requested you're doomed,
>>> because when the non-probe request arrives, the bridge driver
>>> may have been removed already.. And the kernel gently reminds
>>> you of this fact in the form of an oops.
>>
>> The probe is going to need to request the resources to access the hardware it
>> is designed to control. Hence it will be incrementing the refcount exactly
>> when it needs to.
> 
> We cannot presume (impose) what all VME drivers will do; that's
> up to them. Imagine for instance I want to write a driver that
> allows me to operate on the location monitors, all orchestrated
> via sysfs.  Probe would be almost empty, resources would come
> and go at a later time.

You'd need to request the location monitor to use it. If you didn't request it
at probe time, then removed the bridge, when you went to request the resource
at run time it would fail. It would be up to your driver to deal with that
gracefully.

> If the bridge driver is removed when
> I haven't registered any location monitors, I'll get an oops
> when I try to register a block of them. Not pretty.

No, your driver will be told the resource isn't available by vme_lm_request(),
it would return null. It would then be up to you as the driver writer to
handle that gracefully. In fact, just as you'd have to do if the location
monitor was already being used by a different VME device driver which had
positioned them where it needed them.

> Remember that we're writing a bus driver here, we shouldn't
> get on the way of the drivers for the devices on the bus, no
> matter how crazy they are.

Actually we're providing resource management and a bridge independent VME API
for VME device drivers as part of a bus specific core, much as USB and PCI do.
The VME bridge drivers bind under it, the VME device drivers bind on top of it.

Crazy probably equates to not portable across different VME bridges, so no I
don't agree. I'm in favour of providing as much flexibility to VME device
driver authors as possible, without impacting the flexibility of the user to
use the drivers on top of different VME bridges.

> IMHO in order to make sure we're on
> the right track we must look at other bus drivers. And these
> other drivers just keep things simple and stupid, which is
> flexible, safe and "Obviously Correct(tm)".

Though I don't think the approach you are advocating is simple for the VME
device driver writer (new style PCI drivers don't as a rule directly manage
refcounting, in the same way your approach would add complexity for the
individual VME device drivers); it's not safe (I deem it unsafe to expect
every VME device driver writer to manage the refcounting in their drivers
explicitly, just as it appears is the case for PCI devices) and as a result
not "Obviously Correct(tm)".

Martyn

-- 
Martyn Welch (Principal Software Engineer) | Registered in England and
GE Intelligent Platforms                   | Wales (3828642) at 100
T +44(0)127322748                          | Barbirolli Square, Manchester,
E martyn.welch@xxxxxx                      | M2 3AB  VAT:GB 927559189
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/