All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Igor Druzhinin" <igor.druzhinin@citrix.com>,
	xen-devel@lists.xen.org, boris.ostrovsky@oracle.com,
	stephen.s.brennan@oracle.com,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: BUG in 1f3d87c75129 ("x86/vpt: do not take pt_migrate rwlock in some cases")
Date: Tue, 15 Jun 2021 15:17:22 +0200	[thread overview]
Message-ID: <e93fefae-a178-5fb9-1747-45d45818d66d@suse.com> (raw)
In-Reply-To: <3aaed845-b273-0688-4cac-3d440e3d58d3@citrix.com>

On 15.06.2021 11:17, Andrew Cooper wrote:
> On 15/06/2021 09:12, Roger Pau Monné wrote:
>> On Mon, Jun 14, 2021 at 06:01:17PM +0200, Jan Beulich wrote:
>>> On 14.06.2021 15:27, Roger Pau Monné wrote:
>>>> On Mon, Jun 14, 2021 at 01:53:09PM +0200, Jan Beulich wrote:
>>>>> x86/vpt: fully init timers before putting onto list
>>>>>
>>>>> With pt_vcpu_lock() no longer acquiring the pt_migrate lock, parties
>>>>> iterating the list and acting on the timers of the list entries will no
>>>>> longer be kept from entering their loops by create_periodic_time()'s
>>>>> holding of that lock. Therefore at least init_timer() needs calling
>>>>> ahead of list insertion, but keep this and set_timer() together.
>>>>>
>>>>> Fixes: 8113b02f0bf8 ("x86/vpt: do not take pt_migrate rwlock in some cases")
>>>>> Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> Thanks for looking into this so quickly, and sorry for not realizing
>>>> myself when relaxing the locking. Adding the timer to the list without
>>>> it being fully initialized was a latent issue even if protected by the
>>>> lock initially.
>>>>
>>>> Provided testing shows the issue is fixed:
>>> I guess the change here is needed anyway, even if testing finds there's
>>> still something amiss?
>> Indeed, just wondered whether there might be other instances using a
>> similar pattern, but I'm not able to spot any.
>>
>> It might even be better to fix other issues (if any) on a different
>> commit.
> 
> To be honest, this change is clearly good, and necessary.  I'd be
> tempted to commit it now, as is, irrespective of whether there are
> further bugs in this area.

Done.

Jan



      reply	other threads:[~2021-06-15 13:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-14 11:15 BUG in 1f3d87c75129 ("x86/vpt: do not take pt_migrate rwlock in some cases") Igor Druzhinin
2021-06-14 11:53 ` Jan Beulich
2021-06-14 13:27   ` Roger Pau Monné
2021-06-14 16:01     ` Jan Beulich
2021-06-14 16:10       ` Andrew Cooper
2021-06-14 17:06         ` Boris Ostrovsky
2021-06-15  8:12       ` Roger Pau Monné
2021-06-15  9:17         ` Andrew Cooper
2021-06-15 13:17           ` Jan Beulich [this message]

Reply instructions:

You may reply publicly 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=e93fefae-a178-5fb9-1747-45d45818d66d@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=igor.druzhinin@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=stephen.s.brennan@oracle.com \
    --cc=xen-devel@lists.xen.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.