- 20 Jan, 2013 35 commits
-
-
Daniel Vetter authored
Now that framebuffer are reference-counted for all use-sites, update the documentation accordingly to stress the new rules for initialization and teardown. Also add a short paragraph about the implications for drivers of the new locking rules. Reviewed-by:
Rob Clark <rob@ti.com> Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Daniel Vetter authored
The coup de grace of the entire journey. No more dropped frames every 10s on my testbox! I've tried to audit all ->detect and ->get_modes callbacks, but things became a bit fuzzy after trying to piece together the umpteenth implemenation. Afaict most drivers just have bog-standard output register frobbing with a notch of i2c edid reading, nothing which could potentially race with the newly concurrent pageflip/set_cursor code. The big exception is load-detection code which requires a running pipe, but radeon/nouveau seem to to this without touching any state which can be observed from page_flip (e.g. disabled crtcs temporarily getting enabled and so a pageflip succeeding). The only special case I could find is the i915 load detect code. That uses the normal modeset interface to enable the load-detect crtc, and so userspace could try to squeeze in a pageflip on the load-detect pipe. So we need to grab the relevant crtc mutex in there, to avoid the temporary crtc enabling to sneak out and be visible to userspace. Note that the sysfs files already stopped grabbing the per-crtc locks, since I didn't want to bother with doing a interruptible modeset_lock_all. But since there's very little in-between breakage (essentially just the ability for userspace to pageflip on load-detect crtcs when it shouldn't on the i915 driver) I figured I don't need to bother. Reviewed-by:
-
Daniel Vetter authored
The pagelip ioctl itself is rather simply, so the hard work for this patch is auditing all the drivers: - exynos: Pageflip is protect with dev->struct_mutex and ... synchronous. But nothing fancy going on, besides a check whether the crtc is enabled, which should probably be somewhere in the drm core so that we have unified behaviour across all drivers. - i915: hw-state is protected with dev->struct_mutex, the delayed unpin work together with the other stuff the pageflip complete irq handler needs is protected by the event_lock spinlock. - nouveau: With the pin/unpin functions fixed, everything looks safe: A bit of ttm wrestling and refcounting, and a few channel accesses. The later are either already proteced sufficiently, or are now safe with the channel locking introduced to make cursor updates safe. - radeon: The irq_get/put functions look a bit race, since the atomic_inc/dec isn't protect with locks. Otoh they're all per-crtc, so we should be safe with per-crtc locking from the drm core. Then there's tons of per-crtc register access, which could potentially go through the indirect reg acces. But that's fixed to make cursor updates concurrent. Bookeeping for the drm even is also protected with the even_lock, which also protects against the pageflip irq handler since radeon hw seems to have no way to queue these up asynchronously. Otherwise just a bit of ttm-based buffer handling and fencing, which is now safe with the previous patch to hold bdev->fence_lock while grabbing the ttm fence. - shmob: Only one crtc. That's an easy one ... - vmwgfx: As usual a bit special with tons different things: - Flippable check using is_implicit and num_implicit. Changes to those seem to be nicely covered with the global modeset lock, so we should be fine. - Some dirty cliprect handling stuff, or at least that is my guess. Looks like it's fine since either it's per-crtc, invariant or (like the execbuf stuff launched) protected otherwise. - Adding the actual flip to the fence_event list. On a quick look this seems to have solid locking in place, too. ... but generally this is all way over my head. - imx: Impressive display of races between the page_flip implementation and the irq handler. Also, ipu_drm_set_base which gets eventually called from the irq handler to update the display base isn't really protected against concurrent set_config calls from process context. In any case, going for per-crtc locking won't make this worse, so nothing to do. - omap: The new async callback code merged into 3.8 seems to have solid locking in place, and there doesn't seem to be any shared state at risk. Especially since the callbacks still use modeset_lock_all and are so not converted. v2: Update omapdrm analysis to 3.8 code per the discussion with Rob Clark. Reviewed-by: -
Daniel Vetter authored
Now that all framebuffer usage is properly refcounted, we are no longer required to hold the modeset locks while dropping the last reference. Hence implemented a fastpath which avoids the potential stalls associated with grabbing mode_config.lock for the case where there's no other reference around. Explain in a big comment why it is safe. Also update kerneldocs with the new locking rules around drm_framebuffer_remove. Reviewed-by:
-
Daniel Vetter authored
Afact vmwgfx already has all the right refcounting implemented on the backing storage, and we only need to ensure that the drm fb doesn't disappear untimely. So holding onto the fb reference from _lookup until vmw_kms_present has completed should be enough. Signed-off-by: -
Daniel Vetter authored
Useful for checking whether the new refcounting works as advertised. Signed-off-by: -
Daniel Vetter authored
With the prep patch to encapsulate ->set_crtc calls, this is now rather easy. Hooray for inconsistent semantics between ->set_crtc and ->page_flip, where the driver callback is supposed to update the fb pointer, and ->update_plane, where the drm core does the same. Also, since the drm core functions check crtc->fb before calling into driver callbacks, we can't really reduce the critical sections protected by the mode_config locks. Reviewed-by:
-
Daniel Vetter authored
Now plane->fb holds a reference onto it's framebuffer. Nothing too fancy going on here: - Extract __drm_framebuffer_unreference to be called when we know we're not dropping the last reference, e.g. useful in the fb cleanup code. - Reduce the locked sections in the set_plane ioctl to only protect plane->fb/plane->crtc and the driver callback (i.e. hw state). Everything either doesn't disappear (crtc, plane) or is refcounted (fb), and all the data we check is invariant over the respective object's lifetimes. Reviewed-by:
-
Daniel Vetter authored
We only need to ensure that the fb stays around for long enough. While at it, only grab the modeset locks when we need them (since most drivers don't implement the dirty callback, this should help jitter and stalls when using the generic modeset driver). Reviewed-by:
-
Daniel Vetter authored
We only need to push the fb unreference a bit down. While at it, properly pass the return value from ->create_handle back to userspace. Most drivers either return -ENODEV if they don't have a concept of buffer objects (ast, cirrus, ...) or just install a handle for the underlying gem object (which is ok since we hold a reference on that through the framebuffer). v2: Split out the ->create_handle rework in the individual drivers. Reviewed-by:
-
Daniel Vetter authored
And drop it where it's not needed. Most driver just lookup the gem object, allocate an fb struct, fill in all the useful fields and then register it with drm_framebuffer_init. All of these operations are already separately locked, and since we only put the fb into the fpriv->fbs list _after_ having called ->fb_create, we can't also race with rmfb. We can otoh race with other ioctls that put the framebuffer to use, but all drivers have been reorganized already to call drm_framebuffer_init last in the fb creation sequence. So essentially, we can completely remove any modeset locks from the addfb ioctl paths. Yeah! Also, reference-counting is solid - we get a reference from fb_create which we transfer to the fpriv->fbs list. And after unlocking the fpriv->fbs_lock we don't touch the framebuffer any longer. Furthermore drm_framebuffer_init has added a 2nd reference for the idr lookup, and any access through that table will do it's own refcounting. Reviewed-by:
-
Daniel Vetter authored
Atm we still need to unconditionally take the modeset locks in the rmfb paths. But eventually we only want to take them if there are other users around as a slow-path. This way sane userspace avoids blocking on edid reads and other stuff in rmfb if it ensures that the fb isn't used anywhere by a crtc/plane. We can do a quick check for such other users once framebuffers are properly refcounting by locking at the refcount - if it's more than 1, there are other users left. Again, rmfb racing against other ioctls isn't a real problem, userspace is allowed to shoot its foot. This patch just prepares this by moving the modeset locks to nest within fpriv->fbs_lock. Now the distinction between the fbs_lock and the device-global fb_lock is clear, since we need to hold the fbs_lock outside of any modeset_locks in fb_release. Reviewed-by:
-
Daniel Vetter authored
Since otherwise looking and reference-counting around drm_framebuffer_lookup will be an unmanageable mess. With this change, an object can either be found in the idr and will stay around once we incremented the reference counter. Or it will be gone for good and can't be looked up using its id any more. Atomicity is guaranteed by the dev->mode_config.fb_lock. The newly-introduce fpriv->fbs_lock looks a bit redundant, but the next patch will shuffle the locking order between these two locks and all the modeset locks taken in modeset_lock_all, so we'll need it. Also, since userspace could do really funky stuff and race e.g. a getresources with an rmfb, we need to make sure that the kernel doesn't fall over trying to look-up an inexistent fb, or causing confusion by having two fbs around with the same id. Simply reset the framebuffer id to 0, which marks it as reaped. Any lookups of that id will fail, so the object is really gone for good from userspace's pov. Note that we still need to protect the "remove framebuffer from all use-cases" and the final unreference with the modeset-lock, since most framebuffer use-sites don't implement proper reference counting yet. We can only lift this once _all_ users are converted. With this change, two references are held on alife, but unused framebuffers: - The reference for the idr lookup, created in this patch. - For user-created framebuffers the fpriv->fbs reference, for driver-private fbs the driver is supposed to hold it's own last reference. Note that the dev->mode_config.fb_list itself does _not_ hold a reference onto the framebuffers (this list is essentially only used for debugfs files). Hence if there's anything left there when the driver has cleaned up all it's modeset resources, this is a ref-leak. WARN about it. Now we only need to fix up all other places to properly reference count framebuffers. v2: Fix spelling fail in a comment spotted by Rob Clark. Reviewed-by:
-
Daniel Vetter authored
We have two classes of framebuffer - Created by the driver (atm only for fbdev), and the driver holds onto the last reference count until destruction. - Created by userspace and associated with a given fd. These framebuffers will be reaped when their assoiciated fb is closed. Now these two cases are set up differently, the framebuffers are on different lists and hence destruction needs to clean up different things. Also, for userspace framebuffers we remove them from any current usage, whereas for internal framebuffers it is assumed that the driver has done this already. Long story short, we need two different ways to cleanup such drivers. Three functions are involved in total: - drm_framebuffer_remove: Convenience function which removes the fb from all active usage and then drops the passed-in reference. - drm_framebuffer_unregister_private: Will remove driver-private framebuffers from relevant lists and drop the corresponding references. Should be called for driver-private framebuffers before dropping the last reference (or like for a lot of the drivers where the fbdev is embedded someplace else, before doing the cleanup manually). - drm_framebuffer_cleanup: Final cleanup for both classes of fbs, should be called by the driver's ->destroy callback once the last reference is gone. This patch just rolls out the new interfaces and updates all drivers (by adding calls to drm_framebuffer_unregister_private at all the right places)- no functional changes yet. Follow-on patches will move drm core code around and update the lifetime management for framebuffers, so that we are no longer required to keep framebuffers alive by locking mode_config.mutex. I've also updated the kerneldoc already. vmwgfx seems to again be a bit special, at least I haven't figured out how the fbdev support in that driver works. It smells like it's external though. v2: The i915 driver creates another private framebuffer in the load-detect code. Adjust its cleanup code, too. Reviewed-by:
-
Daniel Vetter authored
And replace all fb lookups with it. Also add a WARN to drm_mode_object_find since that is now no longer the blessed interface to look up an fb. And add kerneldoc to both functions. This only updates all callsites, but immediately drops the acquired refence again. Hence all callers still rely on the fact that a mode fb can't disappear while they're holding the struct mutex. Subsequent patches will instate proper use of refcounts, and then rework the rmfb and unref code to no longer serialize fb destruction with the mode_config lock. We don't want that since otherwise a compositor might end up stalling for a few frames in rmfb. v2: Don't use kref_get_unless_zero - Greg KH doesn't like that kind of interface. Reviewed-by:
-
Daniel Vetter authored
Well, at least step 1. The goal here is that framebuffer objects can survive outside of the mode_config lock, with just a reference held as protection. The first step to get there is to introduce a special fb_lock which protects fb lookup, creation and destruction, to make them appear atomic. This new fb_lock can nest within the mode_config lock. But the idea is (once the reference counting part is completed) that we only quickly take that fb_lock to lookup a framebuffer and grab a reference, without any other locks involved. vmwgfx is the only driver which does framebuffer lookups itself, also wrap those calls to drm_mode_object_find with the new lock. Also protect the fb_list walking in i915 and omapdrm with the new lock. As a slight complication there's also the list of user-created fbs attached to the file private. The problem now is that at fclose() time we need to walk that list, eventually do a modeset call to remove the fb from active usage (and are required to be able to take the mode_config lock), but in the end we need to grab the new fb_lock to remove the fb from the list. The easiest solution is to add another mutex to protect this per-file list. Currently that new fbs_lock nests within the modeset locks and so appears redudant. But later patches will switch around this sequence so that taking the modeset locks in the fb destruction path is optional in the fastpath. Ultimately the goal is that addfb and rmfb do not require the mode_config lock, since otherwise they have the potential to introduce stalls in the pageflip sequence of a compositor (if the compositor e.g. switches to a fullscreen client or if it enables a plane). But that requires a few more steps and hoops to jump through. Note that framebuffer creation/destruction is now double-protected - once by the fb_lock and in parts by the idr_lock. The later would be unnecessariy if framebuffers would have their own idr allocator. But that's material for another patch (series). v2: Properly initialize the fb->filp_head list in _init, otherwise the newly added WARN to check whether the fb isn't on a fpriv list any more will fail for driver-private objects. v3: Fixup two error-case unlock bugs spotted by Richard Wilbur. Reviewed-by:
-
Daniel Vetter authored
->cursor_move uses mostly the same facilities in drivers as ->cursor_set, so pretty much nothing to fix up: - ast/gma500/i915: They all use per-crtc registers to update the cursor position. ast again touches the global cursor cache, but that's ok since there's only one crtc. - nouveau: nv50+ is again special, updates happen through the per-crtc channel (without pushbufs), so it's not protected by the new evo lock introduced earlier. But since this channel is per-crtc, we should be fine anyway. - radeon: A bit a mess: avivo asics need a workaround when both output pipes are enabled, which means it'll access the crtc list. Just reading that flag is ok though as long as radeon _always_ grabs all locks when changing the crtc configuration. Which means with the current scheme it cannot do an optimized modeset which only locks the relevant crtcs. This can be fixed though by introducing a bit of global state with separate locks and ensure in the modeset code that the cursor will be updated appropriately when enabling the 2nd pipe (on affected asics). - vmwgfx: I still don't understand what it's doing exactly, so apply the same trick for now. v2: Fixup unlocking for the error cases, spotted by Richard Wilbur. v3: Another error-case fixup. Reviewed-by:
-
Daniel Vetter authored
First convert ->cursor_set to only take the crtc lock, since that seems to be the function with the least amount of state - the core ioctl function doesn't check anything which can change at runtime, so we don't have any object lifetime issues to contend. The only thing which is important is that the driver's implementation doesn't touch any state outside of that single crtc which is not yet properly protected by other locking: - ast: access the global ast->cache_kmap. Luckily we only have on crtc on this driver, so this is fine. Add a comment. - gma500: calls gma_power_begin|and and psb_gtt_pin|unpin, both which have their own locking to protect their state. Everything else is crtc-local. - i915: touches a bit of global gem state, all protected by the One Lock to Rule Them All (dev->struct_mutex). - nouveau: Pre-nv50 is all nice, nv50+ uses the evo channels to queue up all display changes. And some of these channels are device global. But this is fine now since the previous patch introduced an evo channel mutex. - radeon: Uses some indirect register access for cursor updates, but with the previous patches to protect these indirect 2-register access patterns with a spinlock, this should be fine now, too. - vmwgfx: I have no idea how that works - update_cursor_position doesn't take any per-crtc argument and I haven't figured out any other place where this could be set in some form of a side-channel. But vmwgfx definitely has more than one crtc (or at least can register more than one), so I have no idea how this is supposed to not fail with the current code already. Hence take the easy way out and simply acquire all locks (which requires dropping the crtc lock the core acquired for us). That way it's not worse off for consistency than the old code. Reviewed-by:
-
Daniel Vetter authored
*drumroll* The basic idea is to protect per-crtc state which can change without touching the output configuration with separate mutexes, i.e. all the input side state to a crtc like framebuffers, cursor settings or plane configuration. Holding such a crtc lock gives a read-lock on all the other crtc state which can be changed by e.g. a modeset. All non-crtc state is still protected by the mode_config mutex. Callers that need to change modeset state of a crtc (e.g. dpms or set_mode) need to grab both the mode_config lock and nested within any crtc locks. Note that since there can only ever be one holder of the mode_config lock we can grab the subordinate crtc locks in any order (if we need to grab more than one of them). Lockdep can handle such nesting with the mutex_lock_nest_lock call correctly. With this functions that only touch connectors/encoders but not crtcs only need to take the mode_config lock. The biggest such case is the output probing, which means that we can now pageflip and move cursors while the output probe code is reading an edid. Most cases neatly fall into the three buckets: - Only touches connectors and similar output state and so only needs the mode_config lock. - Touches the global configuration and so needs all locks. - Only touches the crtc input side and so only needs the crtc lock. But a few cases that need special consideration: - Load detection which requires a crtc. The mode_config lock already prevents a modeset change, so we can use any unused crtc as we like to do load detection. The only thing to consider is that such temporary state changes don't leak out to userspace through ioctls that only take the crtc look (like a pageflip). Hence the load detect code needs to grab the crtc of any output pipes it touches (but only if it touches state used by the pageflip or cursor ioctls). - Atomic pageflip when moving planes. The first case is sane hw, where planes have a fixed association with crtcs - nothing needs to be done there. More insane^Wflexible hw needs to have plane->crtc mapping which is separately protect with a lock that nests within the crtc lock. If the plane is unused we can just assign it to the current crtc and continue. But if a plane is already in use by another crtc we can't just reassign it. Two solution present themselves: Either go back to a slow-path which takes all modeset locks, potentially incure quite a hefty delay. Or simply disallowing such changes in one atomic pageflip - in general the vblanks of two crtcs are not synced, so there's no sane way to atomically flip such plane changes accross more than one crtc. I'd heavily favour the later approach, going as far as mandating it as part of the ABI of such a new a nuclear pageflip. And if we _really_ want such semantics, we can always get them by introducing another pageflip mutex between the mode_config.mutex and the individual crtc locks. Pageflips crossing more than one crtc would then need to take that lock first, to lock out concurrent multi-crtc pageflips. - Optimized global modeset operations: We could just take the mode_config lock and then lazily lock all crtc which are affected by a modeset operation. This has the advantage that pageflip could continue unhampered on unaffected crtc. But if e.g. global resources like plls need to be reassigned and so affect unrelated crtcs we can still do that - nested locking works in any order. This patch just adds the locks and takes them in drm_modeset_lock_all, no real locking changes yet. v2: Need to initialize the new lock in crtc_init and lock it righ away, for otherwise the modeset_unlock_all below will try to unlock a not-locked mutex. Reviewed-by:
-
Daniel Vetter authored
I've left the locking in the debugfs code as-is, it's essentially just used to keep the framebuffer object alive (which won't be necessary any more later on). We don't need fb refcounting either, since the new mode_config.fb_lock ensures that the framebuffers can't disappear (once mode_config.mutex doesn't guarantee this any more later on in the series). The fbcon restore needs all modeset locks. The crtc callbacks seem to only need the crtc locks, but I've quickly discussed things with Rob Clark and he's fine with just using modeset_lock_all for those, too. He'll look into converting things over later. Reviewed-by:
-
Daniel Vetter authored
Ok, this one here is a bit more complicated, and I can't really claim to fully understand the locking and lifetime rules of the vmwgfx driver. So just convert ever mutex_lock call, including the interruptible one. Since other places (e.g. in the execbuf ioctl) take the mode_config.mutex without bothering with interruptible handling, I've figured I should be able to get away with this in a few more places ... Signed-off-by: -
Daniel Vetter authored
Only a resume method to account for. v2: Fixup compilation. Signed-off-by: -
Daniel Vetter authored
Just a call to drm_helper_resume_force_mode, obviously wants full locking for that. Signed-off-by: -
Daniel Vetter authored
Only two places: - suspend/resume - Some really strange mode validation tool with too much funny-lucking hand-rolled conversion code. - The recently-added lastclose fbdev restore code. Better safe than sorry, so convert both places to keep the locking semantics as much as possible. Signed-off-by: -
Daniel Vetter authored
Two exceptions: - debugfs files only read information which is not related to crtc, so can stay on the modeset_config lock. - Same holds for the edp vdd work in intel_dp.c. Add a corresponding WARN_ON and a comment next to the intel_dp struct fields for documentation. Signed-off-by: -
Daniel Vetter authored
This is the first step towards introducing the new modeset locking scheme. The plan is to put helper functions into place at all the right places step-by-step, so that the final patch to switch on the new locking scheme doesn't need to touch every single driver. This helper here will serve as the shotgun solutions for all places where a more fine-grained locking isn't (yet) implemented. v2: Fixup kerneldoc for unlock_all. Reviewed-by:
-
Daniel Vetter authored
With refcounting we need to adjust framebuffer refcounts at each callsite - much easier to do if they all call the same little helper function. Reviewed-by:
-
Daniel Vetter authored
Some drivers don't have real ->create_handle callbacks. - cirrus/ast/mga200: Returns either 0 or -EINVAL. - udl: Didn't even bother with a callback, leading to a nice userspace-triggerable OOPS. - vmwgfx: This driver bothered with an implementation to return 0 as the handle (which is the canonical no-obj gem handle). All have in common that ->create_handle doesn't really make too much sense for them - that ioctl is used only for seamless fb takeover in the radeon/nouveau/i915 ddx drivers. So allow drivers to not implement this and return a consistent -ENODEV. Signed-off-by: -
Daniel Vetter authored
... by moving the bo_pin/bo_unpin manipulation of the pin_refcount under the protection of the ttm reservation lock. pin/unpin seems to get called from all over the place, so atm this is completely racy. After this patch there are only a few places in cleanup functions left which access ->pin_refcount without locking. But I'm hoping that those are safe and some other code invariant guarantees that this won't blow up. In any case, I only need to fix up pin/unpin to make ->pageflip work safely, so let's keep it at that. Add a comment to the header to explain the new locking rule. Signed-off-by: -
Daniel Vetter authored
With per-crtc locks modeset operations can run in parallel, and the cursor code uses the device-global evo master channel for hw frobbing. But the pageflip code can also sync with the master under some circumstances. Hence just wrap things up in a mutex to ensure that pushbuf access doesn't intermingle. The approach here is a bit overkill since the per-crtc channels used to schedule the pageflips could probably be used without this pushbuf locking, but I'm not familiar enough with the nouveau codebase to be sure of that. v2: Add missing mutex_init to avoid angering lockdep. Signed-off-by: -
Daniel Vetter authored
Doing this within the fb->destroy callback leads to a locking nightmare. And all other drm drivers that restore the fbcon do it in lastclose, too. With this adjustments all fb->destroy callbacks optionally drop references to any gem objects used as backing storage, call drm_framebuffer_cleanup and then kfree the struct. Which nicely simplifies the locking for framebuffer unreferencing and freeing, since this doesn't require that we hold the mode_config lock. A slight exception is the vmwgfx surface backed framebuffer, it also calls drm_master_put and removes the object from a device-private framebuffer list. Both seem to have solid locking in place already. Conclusion is that now it is no longer required to hold the mode_config lock while freeing a framebuffer. v2: Drop the corresponding mutex_lock WARN check from drm_framebuffer_unreference. v3: Use just the mode_config lock not modeset_lock_all, due to patch reordering. Acked-by:
Alan Cox <alan@lxorguk.ukuu.org.uk> Signed-off-by:
-
Daniel Vetter authored
vmwgfx has an oddity, when failing to reference the surface it'll return 0, since that's what the successfull drm_framebuffer_init will leave behind in ret. Fix this up by returning -EINVAL. Split out from all the other driver updates due to the above tiny semantic change. Shouldn't matter though since the reference grabbing seemingly can't fail. Signed-off-by: -
Daniel Vetter authored
With more fine-grained locking we can no longer rely on the big mode_config lock to prevent concurrent access to mode resources like framebuffers. Instead a framebuffer becomes accessible to other threads as soon as it is added to the relevant lookup structures. Hence it needs to be fully set up by the time drivers call drm_framebuffer_init. This patch here is the drivers part of that reorg. Nothing really fancy going on safe for three special cases. - exynos needs to be careful to properly unref all handles. - nouveau gets a resource leak fixed for free: one of the error cases didn't cleanup the framebuffer, which is now moot since the framebuffer is only registered once it is fully set up. - vmwgfx requires a slight reordering of operations, I'm hoping I didn't break anything (but it's refcount management only, so should be safe). v2: Split out exynos, since it's a bit more hairy than expected. v3: Drop bogus cirrus hunk noticed by Richard Wilbur. v4: Split out vmwgfx since there's a small change in return values. Reviewed-by: Rob Clark <rob@ti.com> (core + omapdrm) Signed-off-by: -
Daniel Vetter authored
And do a quick pass to adjust them to the last few (years?) of changes ... This time actually compile-tested ;-) Reviewed-by:
-
Daniel Vetter authored
- config_cleanup was confused: It claimed that callers need to hold the modeset lock, but the connector|encoder_cleanup helpers grabbed that themselves (note that crtc_cleanup did _not_ grab the modeset lock). Which resulted in all drivers _not_ hodling the lock. Since this is for single-threaded cleanup code, drop the requirement from docs and also drop the lock_grabbing from all _cleanup functions. - Kill the LOCKING section in the doctype, since clearly we're not good enough to keep them up-to-date. And misleading locking documentation is worse than useless (see e.g. the comment in the vmgfx driver about the cleanup mess). And since for most functions the very first line either grabs the lock or has a WARN_ON(!locked) the documentation doesn't really add anything. - Instead put in some effort into explaining the only two special cases a bit better: config_init and config_cleanup are both called from single-threaded setup/teardown code, so don't do any locking. It's the driver's job though to enforce this. - Where lacking, add a WARN_ON(!is_locked). Not many places though, since locking around fbdev setup/teardown is through-roughly screwed up, and so will break almost every single WARN annotation I've tried to add. - Add a drm_modeset_is_locked helper - the Grate Modset Locking Rework will use the compiler to assist in the big reorg by renaming the mode lock, so start encapsulating things. Unfortunately this ended up in the "wrong" header file since it needs the definition of struct drm_device. v2: Drop most WARNS again - we hit them all over the place, mostly in the setup and teardown sequences. And trying to fix it up leads to nice deadlocks, since the locking in the setup code is really inconsistent. Reviewed-by:
-
- 18 Jan, 2013 1 commit
-
-
Linus Torvalds authored
-
- 17 Jan, 2013 2 commits
-
-
git://git.kernel.org/pub/scm/linux/kernel/git/s390/linuxLinus Torvalds authored
Pull more s390 patches from Martin Schwidefsky: "A couple of bug fixes: one of the transparent huge page primitives is broken, the sched_clock function overflows after 417 days, the XFS module has grown too large for -fpic and the new pci code has broken normal channel subsystem notifications." * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux: s390/chsc: fix SEI usage s390/time: fix sched_clock() overflow s390: use -fPIC for module compile s390/mm: fix pmd_pfn() for thp
-
git://oss.sgi.com/xfs/xfsLinus Torvalds authored
Pull xfs bugfixes from Ben Myers: - fix(es) for compound buffers - fix for dquot soft timer asserts due to overflow of d_blk_softlimit - fix for regression in dir v2 code introduced in commit 20f7e9f3 ("xfs: factor dir2 block read operations") * tag 'for-linus-v3.8-rc4' of git://oss.sgi.com/xfs/xfs: xfs: recalculate leaf entry pointer after compacting a dir2 block xfs: remove int casts from debug dquot soft limit timer asserts xfs: fix the multi-segment log buffer format xfs: fix segment in xfs_buf_item_format_segment xfs: rename bli_format to avoid confusion with bli_formats xfs: use b_maps[] for discontiguous buffers
-
- 16 Jan, 2013 2 commits
-
-
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pmLinus Torvalds authored
Pull ACPI and power management fixes from Rafael Wysocki: - cpuidle regression fix related to the initialization of state kobjects from Krzysztof Mazur. - cpuidle fix removing some not very useful code and making some user-visible problems go away at the same time. From Daniel Lezcano. - ACPI build fix from Yinghai Lu. * tag 'pm+acpi-for-3.8-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm: cpuidle: remove the power_specified field in the driver ACPI / glue: Fix build with ACPI_GLUE_DEBUG set cpuidle: fix number of initialized/destroyed states
-
Eric Sandeen authored
Dave Jones hit this assert when doing a compile on recent git, with CONFIG_XFS_DEBUG enabled: XFS: Assertion failed: (char *)dup - (char *)hdr == be16_to_cpu(*xfs_dir2_data_unused_tag_p(dup)), file: fs/xfs/xfs_dir2_data.c, line: 828 Upon further digging, the tag found by xfs_dir2_data_unused_tag_p(dup) contained "2" and not the proper offset, and I found that this value was changed after the memmoves under "Use a stale leaf for our new entry." in xfs_dir2_block_addname(), i.e. memmove(&blp[mid + 1], &blp[mid], (highstale - mid) * sizeof(*blp)); overwrote it. What has happened is that the previous call to xfs_dir2_block_compact() has rearranged things; it changes btp->count as well as the blp array. So after we make that call, we must recalculate the proper pointer to the leaf entries by making another call to xfs_dir2_block_leaf_p(). Dave provided a metadump image which led to a simple reproducer (create a particular filename in the affected directory) and this resolves the testcase as well as the bug on his live system. Thanks also to dchinner for looking at this one with me. Signed-off-by:Eric Sandeen <sandeen@redhat.com> Tested-by:
Dave Jones <davej@redhat.com> Reviewed-by:
Dave Chinner <dchinner@redhat.com> Reviewed-by:
Mark Tinguely <tinguely@sgi.com> Signed-off-by:
Ben Myers <bpm@sgi.com>
-
