linux-next: manual merge of the tip tree with the drm-intel tree

linux-next: manual merge of the tip tree with the drm-intel tree

[Stephen Rothwell]

Hi all,

FIXME: Add owner of second tree to To:
       Add author(s)/SOB of conflicting commits.

Today's linux-next merge of the tip tree got a conflict in:

  drivers/gpu/drm/i915/i915_gem_shrinker.c

between commits:

  1233e2db199d ("drm/i915: Move object backing storage manipulation to its own locking")

from the drm-intel tree and commit:

  3ab7c086d5ec ("locking/drm: Kill mutex trickery")
  c7faee2109f9 ("locking/drm: Fix i915_gem_shrinker_lock() locking")

from the tip tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/i915/i915_gem_shrinker.c
index a6fc1bdc48af,e9bd2a81d03a..000000000000
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@@ -35,33 -35,6 +35,15 @@@
  #include "i915_drv.h"
  #include "i915_trace.h"
  
- static bool mutex_is_locked_by(struct mutex *mutex, struct task_struct *task)
- {
- 	if (!mutex_is_locked(mutex))
- 		return false;
- 
- #if defined(CONFIG_DEBUG_MUTEXES) || defined(CONFIG_MUTEX_SPIN_ON_OWNER)
- 	return mutex->owner == task;
- #else
- 	/* Since UP may be pre-empted, we cannot assume that we own the lock */
- 	return false;
- #endif
- }
- 
 +static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
 +{
- 	if (!mutex_trylock(&dev->struct_mutex)) {
- 		if (!mutex_is_locked_by(&dev->struct_mutex, current))
- 			return false;
- 
- 		*unlock = false;
- 	} else {
- 		*unlock = true;
- 	}
++	if (!mutex_trylock(&dev->struct_mutex))
++		return false;
 +
++	*unlock = true;
 +	return true;
 +}
 +
  static bool any_vma_pinned(struct drm_i915_gem_object *obj)
  {
  	struct i915_vma *vma;

Re: linux-next: manual merge of the tip tree with the drm-intel tree

[Daniel Vetter]

On Tue, Nov 08, 2016 at 03:25:41PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> FIXME: Add owner of second tree to To:
>        Add author(s)/SOB of conflicting commits.
> 
> Today's linux-next merge of the tip tree got a conflict in:
> 
>   drivers/gpu/drm/i915/i915_gem_shrinker.c
> 
> between commits:
> 
>   1233e2db199d ("drm/i915: Move object backing storage manipulation to its own locking")
> 
> from the drm-intel tree and commit:
> 
>   3ab7c086d5ec ("locking/drm: Kill mutex trickery")
>   c7faee2109f9 ("locking/drm: Fix i915_gem_shrinker_lock() locking")

Hm, this seems to be the older versions that nuke the recursive locking
trickery entirely, I thought we had version in-flight that kept that? I
know that the i915 (and msm locking fwiw) is horrible since essentially
it's a recursive BKL, and we're working (slowly, after all getting rid of
the BKL wasn't simple either) to fix this. But meanwhile I'm assuming that
we'll still need this to be able to get out of low memory situations in
i915. Has that part simply not yet landed?

Thanks, Daniel

> 
> from the tip tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc drivers/gpu/drm/i915/i915_gem_shrinker.c
> index a6fc1bdc48af,e9bd2a81d03a..000000000000
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@@ -35,33 -35,6 +35,15 @@@
>   #include "i915_drv.h"
>   #include "i915_trace.h"
>   
> - static bool mutex_is_locked_by(struct mutex *mutex, struct task_struct *task)
> - {
> - 	if (!mutex_is_locked(mutex))
> - 		return false;
> - 
> - #if defined(CONFIG_DEBUG_MUTEXES) || defined(CONFIG_MUTEX_SPIN_ON_OWNER)
> - 	return mutex->owner == task;
> - #else
> - 	/* Since UP may be pre-empted, we cannot assume that we own the lock */
> - 	return false;
> - #endif
> - }
> - 
>  +static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
>  +{
> - 	if (!mutex_trylock(&dev->struct_mutex)) {
> - 		if (!mutex_is_locked_by(&dev->struct_mutex, current))
> - 			return false;
> - 
> - 		*unlock = false;
> - 	} else {
> - 		*unlock = true;
> - 	}
> ++	if (!mutex_trylock(&dev->struct_mutex))
> ++		return false;
>  +
> ++	*unlock = true;
>  +	return true;
>  +}
>  +
>   static bool any_vma_pinned(struct drm_i915_gem_object *obj)
>   {
>   	struct i915_vma *vma;

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: linux-next: manual merge of the tip tree with the drm-intel tree

[Peter Zijlstra]

On Tue, Nov 08, 2016 at 11:44:03AM +0100, Daniel Vetter wrote:
> On Tue, Nov 08, 2016 at 03:25:41PM +1100, Stephen Rothwell wrote:
> > Hi all,
> > 
> > FIXME: Add owner of second tree to To:
> >        Add author(s)/SOB of conflicting commits.
> > 
> > Today's linux-next merge of the tip tree got a conflict in:
> > 
> >   drivers/gpu/drm/i915/i915_gem_shrinker.c
> > 
> > between commits:
> > 
> >   1233e2db199d ("drm/i915: Move object backing storage manipulation to its own locking")
> > 
> > from the drm-intel tree and commit:
> > 
> >   3ab7c086d5ec ("locking/drm: Kill mutex trickery")
> >   c7faee2109f9 ("locking/drm: Fix i915_gem_shrinker_lock() locking")
> 
> Hm, this seems to be the older versions that nuke the recursive locking
> trickery entirely, I thought we had version in-flight that kept that? I
> know that the i915 (and msm locking fwiw) is horrible since essentially
> it's a recursive BKL, and we're working (slowly, after all getting rid of
> the BKL wasn't simple either) to fix this. But meanwhile I'm assuming that
> we'll still need this to be able to get out of low memory situations in
> i915. Has that part simply not yet landed?

You're talking about:

  lkml.kernel.org/r/20161007154351.GL3117@twins.programming.kicks-ass.net

? I got no feedback from you DRM guys on that so I kinda forgot about
that in the hope we'd not have to do this at all.

I can try and resurrect, that I suppose.

Now, I know you're working on getting rid of this entirely for i915, but
what about that MSM driver? Will we continue to need it there, is
anybody actually maintaining that thing?

Re: [Intel-gfx] linux-next: manual merge of the tip tree with the drm-intel tree

[Daniel Vetter]

On Tue, Nov 08, 2016 at 02:24:48PM +0100, Peter Zijlstra wrote:
> On Tue, Nov 08, 2016 at 11:44:03AM +0100, Daniel Vetter wrote:
> > On Tue, Nov 08, 2016 at 03:25:41PM +1100, Stephen Rothwell wrote:
> > > Hi all,
> > > 
> > > FIXME: Add owner of second tree to To:
> > >        Add author(s)/SOB of conflicting commits.
> > > 
> > > Today's linux-next merge of the tip tree got a conflict in:
> > > 
> > >   drivers/gpu/drm/i915/i915_gem_shrinker.c
> > > 
> > > between commits:
> > > 
> > >   1233e2db199d ("drm/i915: Move object backing storage manipulation to its own locking")
> > > 
> > > from the drm-intel tree and commit:
> > > 
> > >   3ab7c086d5ec ("locking/drm: Kill mutex trickery")
> > >   c7faee2109f9 ("locking/drm: Fix i915_gem_shrinker_lock() locking")
> > 
> > Hm, this seems to be the older versions that nuke the recursive locking
> > trickery entirely, I thought we had version in-flight that kept that? I
> > know that the i915 (and msm locking fwiw) is horrible since essentially
> > it's a recursive BKL, and we're working (slowly, after all getting rid of
> > the BKL wasn't simple either) to fix this. But meanwhile I'm assuming that
> > we'll still need this to be able to get out of low memory situations in
> > i915. Has that part simply not yet landed?
> 
> You're talking about:
> 
>   lkml.kernel.org/r/20161007154351.GL3117@twins.programming.kicks-ass.net
> 
> ? I got no feedback from you DRM guys on that so I kinda forgot about
> that in the hope we'd not have to do this at all.

Yes. Chris/Joonas, pls give this is a spin and review.
> 
> I can try and resurrect, that I suppose.
> 
> Now, I know you're working on getting rid of this entirely for i915, but
> what about that MSM driver? Will we continue to need it there, is
> anybody actually maintaining that thing?

Rob Clark is, and since he's a one-man gpu driver team with other
responsibilities it might take even longer than for i915 :(
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Intel-gfx] linux-next: manual merge of the tip tree with the drm-intel tree

[Peter Zijlstra]

On Tue, Nov 08, 2016 at 05:09:16PM +0100, Daniel Vetter wrote:
> > You're talking about:
> > 
> >   lkml.kernel.org/r/20161007154351.GL3117@twins.programming.kicks-ass.net
> > 
> > ? I got no feedback from you DRM guys on that so I kinda forgot about
> > that in the hope we'd not have to do this at all.
> 
> Yes. Chris/Joonas, pls give this is a spin and review.

OK, I'll respin that thing with Linus' feedback etc. Might not be until
tomorrow though, so any additional feedback would be good.

Thanks!

Re: [Intel-gfx] linux-next: manual merge of the tip tree with the drm-intel tree

[Peter Zijlstra]

On Tue, Nov 08, 2016 at 05:09:16PM +0100, Daniel Vetter wrote:
> > Now, I know you're working on getting rid of this entirely for i915, but
> > what about that MSM driver? Will we continue to need it there, is
> > anybody actually maintaining that thing?
> 
> Rob Clark is, and since he's a one-man gpu driver team with other
> responsibilities it might take even longer than for i915 :(

Fair enough. For my information, how much a of copy/paste job from i915
was that? Could he, in principle, copy/paste your changes to get rid of
this back into MSM without too much effort, or have things diverged
greatly since the initial copy/paste?

Re: [Intel-gfx] linux-next: manual merge of the tip tree with the drm-intel tree

[Daniel Vetter]

On Tue, Nov 08, 2016 at 06:04:15PM +0100, Peter Zijlstra wrote:
> On Tue, Nov 08, 2016 at 05:09:16PM +0100, Daniel Vetter wrote:
> > > Now, I know you're working on getting rid of this entirely for i915, but
> > > what about that MSM driver? Will we continue to need it there, is
> > > anybody actually maintaining that thing?
> > 
> > Rob Clark is, and since he's a one-man gpu driver team with other
> > responsibilities it might take even longer than for i915 :(
> 
> Fair enough. For my information, how much a of copy/paste job from i915
> was that? Could he, in principle, copy/paste your changes to get rid of
> this back into MSM without too much effort, or have things diverged
> greatly since the initial copy/paste?

Probably diverged too much already, and on top the big part is the command
submission, and that's entirely driver/hw specific. But etnaviv is a plain
gem driver which uses per-bo locking, and there's all the ttm drivers with
similar designs, so there's plenty of templates. But it's not just
copypasta for sure.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch