Re: [RFC,PATCHSET] Managed device resources

2007-01-07 Thread Tejun Heo
Greg KH wrote:
> On Thu, Jan 04, 2007 at 05:26:43PM -0500, Jeff Garzik wrote:
>> Greg KH wrote:
>>> Hm, but I guess without the follow-up patches for libata, it will not
>>> really get tested much.  Jeff, if I accept this, what's your feelings of
>>> letting libata be the "test bed" for it?
>>
>> It would be easiest for me to merge this through my 
>> libata-dev.git#upstream tree.  That will auto-propagate it to -mm, and 
>> ensure that both base and libata bits are sent in one batch.
>>
>> Just shout if you see NAK-able bits...
>>
>> Work for you?
> 
> That works for me.

Great, I'll push it through Jeff.

> The only question I have is on the EXPORT_SYMBOL() stuff for the new
> driver/base/ functions.  Tejun, traditionally the driver core has all
> exported symbols marked with EXPORT_SYMBOL_GPL().  So, any objection to
> marking the new ones (becides the "mirror" functions) in this manner?

Okay, will do so.

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC,PATCHSET] Managed device resources

2007-01-04 Thread Greg KH
On Wed, Dec 27, 2006 at 12:18:33AM +0900, Tejun Heo wrote:
> Hello, all.
> 
> This patchset implements managed device resources, in short, devres.



I like this.  It feels a bit awkward, and creating a bunch of duplicate
functions is "messy", but I can't think of any other way to achive this.

Your writeup in this message is also quite good, care to add it to the
Documentation/ directory in the kernel?

> ## Patchset
> 
> This patchset contains the following 12 patches and is against the
> current 2.6.20-rc2 (3bf8ba38f38d3647368e4edcf7d019f9f8d9184a).
> 
> 01: implement devres core
> 02-06 : implement managed resource interface for IO region, IRQ, DMA,
> PCI and iomap

Unless anyone objects, I'll add the first 6 patches here to my tree for
testing in -mm for a bit.

Hm, but I guess without the follow-up patches for libata, it will not
really get tested much.  Jeff, if I accept this, what's your feelings of
letting libata be the "test bed" for it?

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC,PATCHSET] Managed device resources

2007-01-04 Thread Greg KH
On Thu, Jan 04, 2007 at 05:26:43PM -0500, Jeff Garzik wrote:
> Greg KH wrote:
> >Hm, but I guess without the follow-up patches for libata, it will not
> >really get tested much.  Jeff, if I accept this, what's your feelings of
> >letting libata be the "test bed" for it?
> 
> 
> It would be easiest for me to merge this through my 
> libata-dev.git#upstream tree.  That will auto-propagate it to -mm, and 
> ensure that both base and libata bits are sent in one batch.
> 
> Just shout if you see NAK-able bits...
> 
> Work for you?

That works for me.

The only question I have is on the EXPORT_SYMBOL() stuff for the new
driver/base/ functions.  Tejun, traditionally the driver core has all
exported symbols marked with EXPORT_SYMBOL_GPL().  So, any objection to
marking the new ones (becides the "mirror" functions) in this manner?

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC,PATCHSET] Managed device resources

2007-01-04 Thread Jeff Garzik

Greg KH wrote:

Hm, but I guess without the follow-up patches for libata, it will not
really get tested much.  Jeff, if I accept this, what's your feelings of
letting libata be the "test bed" for it?



It would be easiest for me to merge this through my 
libata-dev.git#upstream tree.  That will auto-propagate it to -mm, and 
ensure that both base and libata bits are sent in one batch.


Just shout if you see NAK-able bits...

Work for you?

Jeff


-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC,PATCHSET] Managed device resources

2006-12-30 Thread Denis Vlasenko
On Tuesday 26 December 2006 16:18, Tejun Heo wrote:
> Hello, all.
> 
> This patchset implements managed device resources, in short, devres.

I was working on a Linux device driver. Indeed, those error paths
are notoriously prone to bugs.

Patchset looks like good idea to me.
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC,PATCHSET] Managed device resources

2006-12-26 Thread Tejun Heo
Hello, all.

This patchset implements managed device resources, in short, devres.

## Intro

devres came up while trying to convert libata to use iomap.  Each
iomapped address should be kept and unmapped on driver detach.  For
example, a plain SFF ATA controller (that is, good old PCI IDE) in
native mode makes use of 5 PCI BARs and all of them should be
maintained.

As with many other device drivers, libata low level drivers have
sufficient bugs in ->remove and ->probe failure path.  Well, yes,
that's probably because libata low level driver developers are lazy
bunch, but aren't all low level driver developers?  After spending a
day fiddling with braindamaged hardware with no document or
braindamaged document, if it's finally working, well, it's working.

For one reason or another, low level drivers don't receive as much
attention or testing as core code, and bugs on driver detach or
initilaization failure doesn't happen often enough to be noticeable.
Init failure path is worse because it's much less travelled while
needs to handle multiple entry points.

So, many low level drivers end up leaking resources on driver detach
and having half broken failure path implementation in ->probe() which
would leak resources or even cause oops when failure occurs.  iomap
adds more to this mix.  So do msi and msix.

## Devres

devres is basically linked list of arbitrarily sized memory areas
associated with a struct device.  Each devres entry is associated with
a release function.  A devres can be released in several ways.  No
matter what, all devres entries are released on driver detach.  On
release, the associated release function is invoked and then the
devres entry is freed.

Managed interface is created for resources commonly used by device
drivers using devres.  For example, coherent DMA memory is acquired
using dma_alloc_coherent().  The managed version is called
dmam_alloc_coherent().  It is identical to dma_alloc_coherent() except
for the DMA memory allocated using it is managed and will be
automatically released on driver detach.  Implementation looks like
the following.

  struct dma_devres {
size_t  size;
void*vaddr;
dma_addr_t  dma_handle;
  };

  static void dmam_coherent_release(struct device *dev, void *res)
  {
struct dma_devres *this = res;

dma_free_coherent(dev, this->size, this->vaddr, this->dma_handle);
  }

  dmam_alloc_coherent(dev, size, dma_handle, gfp)
  {
struct dma_devres *dr;
void *vaddr;

dr = devres_alloc(dmam_coherent_release, sizeof(*dr), gfp);
...

/* alloc DMA memory as usual */
vaddr = dma_alloc_coherent(...);
...

/* record size, vaddr, dma_handle in dr */
dr->vaddr = vaddr;
...

devres_add(dev, dr);

return vaddr;
  }

If a driver uses dmam_alloc_coherent(), the area is guaranteed to be
freed whether initialization fails half-way or the device gets
detached.  If most resources are acquired using managed interface, a
driver can have much simpler init and exit code.  Init path basically
looks like the following.

  my_init_one()
  {
struct mydev *d;

d = devm_kzalloc(dev, sizeof(*d), GFP_KERNEL);
if (!d)
return -ENOMEM;

d->ring = dmam_alloc_coherent(...);
if (!d->ring)
return -ENOMEM;

if (check something)
return -EINVAL;
...

return register_to_upper_layer(d);
  }

And exit path,

  my_remove_one()
  {
unregister_from_upper_layer(d);
shutdown_my_hardware();
  }

As shown above, low level drivers can be simplified a lot by using
devres.  Complexity is shifted from less maintained low level drivers
to better maintained higher layer.  Also, as init failure path is
shared with exit path, both can get more testing.

## Devres group

Devres entries can be grouped using devres group.  When a group is
released, all contained normal devres entries and properly nested
groups are released.  One usage is to rollback series of acquired
resources on failure.  For example,

  if (!devres_open_group(dev, NULL, GFP_KERNEL))
return -ENOMEM;

  acquire A;
  if (failed)
goto err;

  acquire B;
  if (failed)
goto err;
  ...

  devres_remove_group(dev, NULL);

  return 0;

 err:
  devres_release_group(dev, NULL);
  return err_code;

As resource acquision failure usually means probe failure, constructs
like above are usually useful in midlayer driver (e.g. libata core
layer) where interface function shouldn't have side effect on failure.
For LLDs, just returning error code suffices in most cases.

## Details

Lifetime of a devres entry begins on devres allocation and finishes
when it is released or destroyed (removed and freed) - no reference
counting.

devres core guarantees atomicity to all basic devres operations and
has support for single-instance devres types (atomic
lookup-and-add-if-not-foun