On Thu, 15 May 2008, Miklos Szeredi wrote:Yeah, don't do this: This kind of serialization can often hide bugs, and in some cases even make them go away (if the caller of the function means that the device is pinned and the tear-down cannot happen, for example), but it's really really bad form. In order to use locking in a repeatable manner that is easy to think about, you really need to *keep* the lock until you've stopped using the data (or have dereferenced it into a stable form - eg maybe accessing the pointer itself needs locking, but some individual data read _off_ the pointer does not). So the above kind of "get and release the lock" does obviously serialize wrt others that hold the lock, but it's still wrong. You should just get the lock in the routines that acually use this thing. Or, if the "struct backing_dev_info *" pointer itself is stable, and it really is just the access from "struct device" that needs protection, then at the very least it should have been static struct backing_dev_info *dev_get_bdi(struct device *dev) { struct backing_dev_info *bdi; mutex_lock(&bdi_dev_mutex); bdi = dev_get_drvdata(dev); mutex_unlock(&bdi_dev_mutex); return bdi; } which makes it clear that it's the "dev_get_drvdata()" that needs the locking, not the bdi pointer itself. Linus --
| Linus Torvalds | Re: LSM conversion to static interface |
| Ingo Molnar | [patch 03/13] syslets: generic kernel bits |
| Ingo Molnar | Re: [PATCH 6/6] sched: disabled rt-bandwidth by default |
| Greg Kroah-Hartman | [PATCH 001/196] Chinese: Add the known_regression URI to the HOWTO |
git: | |
| David Miller | [GIT]: Networking |
| Gregory Haskins | [RFC PATCH 00/17] virtual-bus |
| Gerrit Renker | [PATCH 27/37] dccp: Integration of dynamic feature activation - part 2 (server side) |
| Jarek Poplawski | [PATCH] pkt_sched: Destroy gen estimators under rtnl_lock(). |
