Linux-Next Archive on lore.kernel.org
 help / color / Atom feed
From: Paul McKenney <paulmckrcu@gmail.com>
To: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics <intel-gfx@lists.freedesktop.org>,
	LKML <linux-kernel@vger.kernel.org>,
	DRI <dri-devel@lists.freedesktop.org>,
	Jani Nikula <jani.nikula@intel.com>,
	linux-next@vger.kernel.org
Subject: Re: linux-next: manual merge of the drm tree with the drm-intel-fixes tree
Date: Mon, 27 Mar 2017 10:14:10 -0700
Message-ID: <CAJzB8QG8bE1onedcBLJPcwQwVH4BrpNcWNzpH00JMfDqgyzObQ@mail.gmail.com> (raw)
In-Reply-To: <20170322110050.1e2ea77d@canb.auug.org.au>

[-- Attachment #1.1: Type: text/plain, Size: 2157 bytes --]

> On Tue, Mar 21, 2017 at 5:00 PM, Stephen Rothwell <sfr@canb.auug.org.au>
wrote:
> Hi Dave,
>
> Today's linux-next merge of the drm tree got a conflict in:
>
>   drivers/gpu/drm/i915/i915_gem_shrinker.c
>
> between commit:
>
>   3d3d18f086cd ("drm/i915: Avoid rcu_barrier() from reclaim paths
(shrinker)")
>
> from the drm-intel-fixes tree and commit:
>
>   519d52498156 ("drm/i915: i915_gem_shrink_all() needs an awake device")
>
> from the drm 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 d5d2b4c6ed38,006a8b908f77..000000000000
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@@ -263,7 -264,9 +264,9 @@@ unsigned long i915_gem_shrink_all(struc
>                                 I915_SHRINK_BOUND |
>                                 I915_SHRINK_UNBOUND |
>                                 I915_SHRINK_ACTIVE);
> +       intel_runtime_pm_put(dev_priv);
> +
>  -      rcu_barrier(); /* wait until our RCU delayed slab frees are
completed */
>  +      synchronize_rcu(); /* wait for our earlier RCU delayed slab frees
*/

Unless I am really confused, the RCU delayed slab frees are via call_rcu().
This means that synchronize_rcu() is -not- guaranteed to wait on them.
For example, CPU 0 might be making its way through a pile of callbacks,
including some RCU delayed slab free callbacks.  If there are enough
callbacks, it could take CPU 0 several grace periods to work through the
backlog.  CPU 1 might have no callbacks queued, and might execute the
above code.  Now synchronize_rcu() will wait for a grace period, but only
one, and not necessarily for CPU 0's backlog to drain.

I do not believe that the above fix is safe.

>         return freed;
>   }
>

[-- Attachment #1.2: Type: text/html, Size: 4202 bytes --]

<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div class="gmail_quote">&gt; On Tue, Mar 21, 2017 at 5:00 PM, Stephen Rothwell &lt;<a href="mailto:sfr@canb.auug.org.au">sfr@canb.auug.org.au</a>&gt; wrote:</div><div class="gmail_quote">&gt; Hi Dave,</div><div class="gmail_quote">&gt; </div><div class="gmail_quote">&gt; Today&#39;s linux-next merge of the drm tree got a conflict in:</div><div class="gmail_quote">&gt; </div><div class="gmail_quote">&gt;   drivers/gpu/drm/i915/i915_gem_shrinker.c</div><div class="gmail_quote">&gt; </div><div class="gmail_quote">&gt; between commit:</div><div class="gmail_quote">&gt; </div><div class="gmail_quote">&gt;   3d3d18f086cd (&quot;drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker)&quot;)</div><div class="gmail_quote">&gt; </div><div class="gmail_quote">&gt; from the drm-intel-fixes tree and commit:</div><div class="gmail_quote">&gt; </div><div class="gmail_quote">&gt;   519d52498156 (&quot;drm/i915: i915_gem_shrink_all() needs an awake device&quot;)</div><div class="gmail_quote">&gt; </div><div class="gmail_quote">&gt; from the drm tree.</div><div class="gmail_quote">&gt; </div><div class="gmail_quote">&gt; I fixed it up (see below) and can carry the fix as necessary. This</div><div class="gmail_quote">&gt; is now fixed as far as linux-next is concerned, but any non trivial</div><div class="gmail_quote">&gt; conflicts should be mentioned to your upstream maintainer when your tree</div><div class="gmail_quote">&gt; is submitted for merging.  You may also want to consider cooperating</div><div class="gmail_quote">&gt; with the maintainer of the conflicting tree to minimise any particularly</div><div class="gmail_quote">&gt; complex conflicts.</div><div class="gmail_quote">&gt; </div><div class="gmail_quote">&gt; --</div><div class="gmail_quote">&gt; Cheers,</div><div class="gmail_quote">&gt; Stephen Rothwell</div><div class="gmail_quote">&gt; </div><div class="gmail_quote">&gt; diff --cc drivers/gpu/drm/i915/i915_gem_shrinker.c</div><div class="gmail_quote">&gt; index d5d2b4c6ed38,006a8b908f77..000000000000</div><div class="gmail_quote">&gt; --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c</div><div class="gmail_quote">&gt; +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c</div><div class="gmail_quote">&gt; @@@ -263,7 -264,9 +264,9 @@@ unsigned long i915_gem_shrink_all(struc</div><div class="gmail_quote">&gt;                                 I915_SHRINK_BOUND |</div><div class="gmail_quote">&gt;                                 I915_SHRINK_UNBOUND |</div><div class="gmail_quote">&gt;                                 I915_SHRINK_ACTIVE);</div><div class="gmail_quote">&gt; +       intel_runtime_pm_put(dev_priv);</div><div class="gmail_quote">&gt; +</div><div class="gmail_quote">&gt;  -      rcu_barrier(); /* wait until our RCU delayed slab frees are completed */</div><div class="gmail_quote">&gt;  +      synchronize_rcu(); /* wait for our earlier RCU delayed slab frees */</div><div class="gmail_quote"><br></div><div class="gmail_quote">Unless I am really confused, the RCU delayed slab frees are via call_rcu().</div><div class="gmail_quote">This means that synchronize_rcu() is -not- guaranteed to wait on them.</div><div class="gmail_quote">For example, CPU 0 might be making its way through a pile of callbacks,</div><div class="gmail_quote">including some RCU delayed slab free callbacks.  If there are enough</div><div class="gmail_quote">callbacks, it could take CPU 0 several grace periods to work through the</div><div class="gmail_quote">backlog.  CPU 1 might have no callbacks queued, and might execute the</div><div class="gmail_quote">above code.  Now synchronize_rcu() will wait for a grace period, but only</div><div class="gmail_quote">one, and not necessarily for CPU 0&#39;s backlog to drain.</div><div class="gmail_quote"><br></div><div class="gmail_quote">I do not believe that the above fix is safe.</div><div class="gmail_quote"><br></div><div class="gmail_quote">&gt;         return freed;</div><div class="gmail_quote">&gt;   }</div><div class="gmail_quote">&gt; </div></div></div></div>

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply index

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-22  0:00 Stephen Rothwell
2017-03-27 17:14 ` Paul McKenney [this message]
  -- strict thread matches above, loose matches on Subject: below --
2019-10-31  0:33 Stephen Rothwell
2019-11-08  0:42 ` Stephen Rothwell
2018-01-05  1:00 Stephen Rothwell
2017-07-21  1:26 Stephen Rothwell
2017-06-14  0:56 Stephen Rothwell
2017-06-14  0:50 Stephen Rothwell
2017-06-09  2:26 Stephen Rothwell
2017-06-08  2:53 Stephen Rothwell
2017-03-30  1:14 Stephen Rothwell
2017-03-30  1:08 Stephen Rothwell
2017-03-21 23:57 Stephen Rothwell
2017-03-21  0:28 Stephen Rothwell
2016-06-14  2:10 Stephen Rothwell
2015-12-22 23:06 Stephen Rothwell
2015-12-09  2:35 Stephen Rothwell
2015-12-03 14:51 Mark Brown
2015-12-03 15:49 ` Jani Nikula
2015-12-03 14:47 Mark Brown
2015-12-03 14:52 ` Imre Deak
2015-08-17  3:23 Stephen Rothwell
2015-06-05  5:46 michael
2015-06-05  8:03 ` Jani Nikula
2015-06-09  1:58   ` Stephen Rothwell
2015-03-16  2:30 Stephen Rothwell
2015-03-16 13:43 ` Xi Ruoyao
2015-03-16 15:04   ` Jani Nikula
2014-12-03  2:27 Stephen Rothwell
2014-12-03  8:24 ` Jani Nikula
2014-12-03  8:28   ` Stephen Rothwell
2014-11-17  3:04 Stephen Rothwell
2014-07-23  2:38 Stephen Rothwell
2014-07-09  4:06 Stephen Rothwell
2014-05-22  5:44 Stephen Rothwell
2014-05-22  5:40 Stephen Rothwell
2014-05-07  3:24 Stephen Rothwell
2013-10-28  5:46 Stephen Rothwell
2013-10-28  6:12 ` Stephen Rothwell

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJzB8QG8bE1onedcBLJPcwQwVH4BrpNcWNzpH00JMfDqgyzObQ@mail.gmail.com \
    --to=paulmckrcu@gmail.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=sfr@canb.auug.org.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Next Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-next/0 linux-next/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-next linux-next/ https://lore.kernel.org/linux-next \
		linux-next@vger.kernel.org
	public-inbox-index linux-next

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-next


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git