Re: [RFC,PATCHSET] Managed device resources
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
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
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
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
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
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