Message ID | 511ABA59.9050604@suse.cz (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hello, Jiri. On Tue, Feb 12, 2013 at 10:55:37PM +0100, Jiri Slaby wrote: > this commit in -next causes my KDE to get stuck while starting. I see > only the splash screen. If I disable effects by alt-shift-f12, it continues. > > I bisected it to this commit: > commit 430440fce7da4ad52c2af06a04a5132e5d19faaf > Author: Tejun Heo <tj@kernel.org> > Date: Thu Feb 7 12:31:37 2013 +1100 > > drm: convert to idr_alloc() > > > More concretely to the change in drm_gem_flink_ioctl. If I revert solely > that one, it works again. Maybe I'm blind, but I do not see why... > > Well I see a bug in there, but that is not the root cause -- there is a > missing call to idr_preload_end. Of course, that one only leaves > preemption disabled forever. Fix for this bug is already queued in -mm. http://article.gmane.org/gmane.linux.kernel/1439101/raw But you're saying that adding the missing idr_preload_end() doesn't make the problem go away, right? Thanks.
On 02/12/2013 10:58 PM, Tejun Heo wrote: > Hello, Jiri. > > On Tue, Feb 12, 2013 at 10:55:37PM +0100, Jiri Slaby wrote: >> this commit in -next causes my KDE to get stuck while starting. I see >> only the splash screen. If I disable effects by alt-shift-f12, it continues. >> >> I bisected it to this commit: >> commit 430440fce7da4ad52c2af06a04a5132e5d19faaf >> Author: Tejun Heo <tj@kernel.org> >> Date: Thu Feb 7 12:31:37 2013 +1100 >> >> drm: convert to idr_alloc() >> >> >> More concretely to the change in drm_gem_flink_ioctl. If I revert solely >> that one, it works again. Maybe I'm blind, but I do not see why... >> >> Well I see a bug in there, but that is not the root cause -- there is a >> missing call to idr_preload_end. Of course, that one only leaves >> preemption disabled forever. > > Fix for this bug is already queued in -mm. > > http://article.gmane.org/gmane.linux.kernel/1439101/raw > > But you're saying that adding the missing idr_preload_end() doesn't > make the problem go away, right? Yeah, right. I had to revert the whole hunk...
On 02/12/2013 10:55 PM, Jiri Slaby wrote: > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -453,7 +453,8 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, > spin_lock(&dev->object_name_lock); > if (!obj->name) { > ret = idr_alloc(&dev->object_name_idr, obj, 1, 0, > GFP_NOWAIT); > - obj->name = ret; > + if (ret >= 0) > + obj->name = ret; > args->name = (uint64_t) obj->name; > spin_unlock(&dev->object_name_lock); > > @@ -469,6 +470,7 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, > } > > err: > + idr_preload_end(); > drm_gem_object_unreference_unlocked(obj); > return ret; Oh my, maybe: return ret < 0 ? ret : 0... Let's try.
--- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -453,7 +453,8 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, spin_lock(&dev->object_name_lock); if (!obj->name) { ret = idr_alloc(&dev->object_name_idr, obj, 1, 0, GFP_NOWAIT); - obj->name = ret; + if (ret >= 0) + obj->name = ret; args->name = (uint64_t) obj->name; spin_unlock(&dev->object_name_lock);