Re: [PATCH] regmap: don't use spinlocks with REGCACHE_{RBTREE,COMPRESSED}
On Thu, Feb 27, 2014 at 06:32:53PM -0800, Andrew Bresticker wrote: > On Thu, Feb 27, 2014 at 3:37 AM, Mark Brown wrote: > > Have all the users been audited to verify that they're actually safe > > with this? I just took a quick look at the Tegra drivers and they're > > doing regmap operations in their trigger operations which is done from > > atomic context so they should run into problems trying to take mutexes > > anyway. > Oops, you're right, I didn't look through all the regmap operations > carefully enough. It is in fact the Tegra drivers I've ran into this > issue with. Yeah, I suspected it might be :) > > I think we need to either ensure that all users allocate their caches at > > probe time (which is fine and is hopefully what the current users are > > doing), provide a mechanism for them to do cache allocations outside of > > the spinlock (which sounds hairy) or convert them to flat cache. > Allocations are made in the write path for rbtree and in both the read > and write paths for lzo. I suppose we can pre-allocate everything at > init time, but I haven't looked too much at that. For at least the > Tegra drivers, the best option for now appears to be to convert to a > flat cache. OK, that's certainly simple to implement. Currently to allocate at startup the standard thing is to supply the hardware defaults as data, though going through and reading everything should also do the trick. signature.asc Description: Digital signature
Re: [PATCH] regmap: don't use spinlocks with REGCACHE_{RBTREE,COMPRESSED}
On Thu, Feb 27, 2014 at 3:37 AM, Mark Brown wrote: > On Wed, Feb 26, 2014 at 07:50:57PM -0800, Andrew Bresticker wrote: >> Both REGCACHE_RBTREE and REGCACHE_COMPRESSED make GFP_KERNEL allocations >> with the regmap lock held. If we're initializing a regmap which would >> normally use a spinlock (e.g. MMIO), fall back to using a mutex if one >> of these caching types is to be used. > > Have all the users been audited to verify that they're actually safe > with this? I just took a quick look at the Tegra drivers and they're > doing regmap operations in their trigger operations which is done from > atomic context so they should run into problems trying to take mutexes > anyway. Oops, you're right, I didn't look through all the regmap operations carefully enough. It is in fact the Tegra drivers I've ran into this issue with. > I think we need to either ensure that all users allocate their caches at > probe time (which is fine and is hopefully what the current users are > doing), provide a mechanism for them to do cache allocations outside of > the spinlock (which sounds hairy) or convert them to flat cache. Allocations are made in the write path for rbtree and in both the read and write paths for lzo. I suppose we can pre-allocate everything at init time, but I haven't looked too much at that. For at least the Tegra drivers, the best option for now appears to be to convert to a flat cache. Thanks, Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regmap: don't use spinlocks with REGCACHE_{RBTREE,COMPRESSED}
On Wed, Feb 26, 2014 at 07:50:57PM -0800, Andrew Bresticker wrote: > Both REGCACHE_RBTREE and REGCACHE_COMPRESSED make GFP_KERNEL allocations > with the regmap lock held. If we're initializing a regmap which would > normally use a spinlock (e.g. MMIO), fall back to using a mutex if one > of these caching types is to be used. Have all the users been audited to verify that they're actually safe with this? I just took a quick look at the Tegra drivers and they're doing regmap operations in their trigger operations which is done from atomic context so they should run into problems trying to take mutexes anyway. I think we need to either ensure that all users allocate their caches at probe time (which is fine and is hopefully what the current users are doing), provide a mechanism for them to do cache allocations outside of the spinlock (which sounds hairy) or convert them to flat cache. signature.asc Description: Digital signature
Re: [PATCH] regmap: don't use spinlocks with REGCACHE_{RBTREE,COMPRESSED}
On Wed, Feb 26, 2014 at 07:50:57PM -0800, Andrew Bresticker wrote: Both REGCACHE_RBTREE and REGCACHE_COMPRESSED make GFP_KERNEL allocations with the regmap lock held. If we're initializing a regmap which would normally use a spinlock (e.g. MMIO), fall back to using a mutex if one of these caching types is to be used. Have all the users been audited to verify that they're actually safe with this? I just took a quick look at the Tegra drivers and they're doing regmap operations in their trigger operations which is done from atomic context so they should run into problems trying to take mutexes anyway. I think we need to either ensure that all users allocate their caches at probe time (which is fine and is hopefully what the current users are doing), provide a mechanism for them to do cache allocations outside of the spinlock (which sounds hairy) or convert them to flat cache. signature.asc Description: Digital signature
Re: [PATCH] regmap: don't use spinlocks with REGCACHE_{RBTREE,COMPRESSED}
On Thu, Feb 27, 2014 at 3:37 AM, Mark Brown broo...@kernel.org wrote: On Wed, Feb 26, 2014 at 07:50:57PM -0800, Andrew Bresticker wrote: Both REGCACHE_RBTREE and REGCACHE_COMPRESSED make GFP_KERNEL allocations with the regmap lock held. If we're initializing a regmap which would normally use a spinlock (e.g. MMIO), fall back to using a mutex if one of these caching types is to be used. Have all the users been audited to verify that they're actually safe with this? I just took a quick look at the Tegra drivers and they're doing regmap operations in their trigger operations which is done from atomic context so they should run into problems trying to take mutexes anyway. Oops, you're right, I didn't look through all the regmap operations carefully enough. It is in fact the Tegra drivers I've ran into this issue with. I think we need to either ensure that all users allocate their caches at probe time (which is fine and is hopefully what the current users are doing), provide a mechanism for them to do cache allocations outside of the spinlock (which sounds hairy) or convert them to flat cache. Allocations are made in the write path for rbtree and in both the read and write paths for lzo. I suppose we can pre-allocate everything at init time, but I haven't looked too much at that. For at least the Tegra drivers, the best option for now appears to be to convert to a flat cache. Thanks, Andrew -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regmap: don't use spinlocks with REGCACHE_{RBTREE,COMPRESSED}
On Thu, Feb 27, 2014 at 06:32:53PM -0800, Andrew Bresticker wrote: On Thu, Feb 27, 2014 at 3:37 AM, Mark Brown broo...@kernel.org wrote: Have all the users been audited to verify that they're actually safe with this? I just took a quick look at the Tegra drivers and they're doing regmap operations in their trigger operations which is done from atomic context so they should run into problems trying to take mutexes anyway. Oops, you're right, I didn't look through all the regmap operations carefully enough. It is in fact the Tegra drivers I've ran into this issue with. Yeah, I suspected it might be :) I think we need to either ensure that all users allocate their caches at probe time (which is fine and is hopefully what the current users are doing), provide a mechanism for them to do cache allocations outside of the spinlock (which sounds hairy) or convert them to flat cache. Allocations are made in the write path for rbtree and in both the read and write paths for lzo. I suppose we can pre-allocate everything at init time, but I haven't looked too much at that. For at least the Tegra drivers, the best option for now appears to be to convert to a flat cache. OK, that's certainly simple to implement. Currently to allocate at startup the standard thing is to supply the hardware defaults as data, though going through and reading everything should also do the trick. signature.asc Description: Digital signature
[PATCH] regmap: don't use spinlocks with REGCACHE_{RBTREE,COMPRESSED}
Both REGCACHE_RBTREE and REGCACHE_COMPRESSED make GFP_KERNEL allocations
with the regmap lock held. If we're initializing a regmap which would
normally use a spinlock (e.g. MMIO), fall back to using a mutex if one
of these caching types is to be used.
Signed-off-by: Andrew Bresticker
---
drivers/base/regmap/regmap.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 2b4e72f..fb618d2 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -438,8 +438,9 @@ struct regmap *regmap_init(struct device *dev,
map->unlock = config->unlock;
map->lock_arg = config->lock_arg;
} else {
- if ((bus && bus->fast_io) ||
- config->fast_io) {
+ if (((bus && bus->fast_io) || config->fast_io) &&
+ config->cache_type != REGCACHE_RBTREE &&
+ config->cache_type != REGCACHE_COMPRESSED) {
spin_lock_init(>spinlock);
map->lock = regmap_lock_spinlock;
map->unlock = regmap_unlock_spinlock;
--
1.9.0.rc1.175.g0b1dcb5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regmap: don't use spinlocks with REGCACHE_{RBTREE,COMPRESSED}
Both REGCACHE_RBTREE and REGCACHE_COMPRESSED make GFP_KERNEL allocations
with the regmap lock held. If we're initializing a regmap which would
normally use a spinlock (e.g. MMIO), fall back to using a mutex if one
of these caching types is to be used.
Signed-off-by: Andrew Bresticker abres...@chromium.org
---
drivers/base/regmap/regmap.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 2b4e72f..fb618d2 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -438,8 +438,9 @@ struct regmap *regmap_init(struct device *dev,
map-unlock = config-unlock;
map-lock_arg = config-lock_arg;
} else {
- if ((bus bus-fast_io) ||
- config-fast_io) {
+ if (((bus bus-fast_io) || config-fast_io)
+ config-cache_type != REGCACHE_RBTREE
+ config-cache_type != REGCACHE_COMPRESSED) {
spin_lock_init(map-spinlock);
map-lock = regmap_lock_spinlock;
map-unlock = regmap_unlock_spinlock;
--
1.9.0.rc1.175.g0b1dcb5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
