All of lore.kernel.org
 help / color / mirror / Atom feed
From: Juergen Gross <jgross@suse.com>
To: Jan Beulich <JBeulich@suse.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Julien Grall <julien.grall@arm.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH 1/3] xen: drop in_atomic()
Date: Fri, 24 May 2019 07:41:54 +0200	[thread overview]
Message-ID: <316af0ff-904b-4cf7-1cb4-08c4ae69c4bd@suse.com> (raw)
In-Reply-To: <5CE52014020000780023147E@prv1-mh.provo.novell.com>

On 22/05/2019 12:10, Jan Beulich wrote:
>>>> On 22.05.19 at 11:45, <jgross@suse.com> wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -3185,22 +3185,6 @@ static enum hvm_translation_result __hvm_copy(
>>  
>>      ASSERT(is_hvm_vcpu(v));
>>  
>> -    /*
>> -     * XXX Disable for 4.1.0: PV-on-HVM drivers will do grant-table ops
>> -     * such as query_size. Grant-table code currently does copy_to/from_guest
>> -     * accesses under the big per-domain lock, which this test would disallow.
>> -     * The test is not needed until we implement sleeping-on-waitqueue when
>> -     * we access a paged-out frame, and that's post 4.1.0 now.
>> -     */
>> -#if 0
>> -    /*
>> -     * If the required guest memory is paged out, this function may sleep.
>> -     * Hence we bail immediately if called from atomic context.
>> -     */
>> -    if ( in_atomic() )
>> -        return HVMTRANS_unhandleable;
>> -#endif
> 
> Dealing with this TODO item is of course much appreciated, but
> should it really be deleted altogether? The big-domain-lock issue
> is gone afair, in which case dropping the #if 0 would seem
> possible to me, even if it's not strictly needed without the sleep-
> on-waitqueue behavior mentioned.

I just had a look and found the following path:

do_domctl() (takes domctl_lock and hypercall_deadlock_mutex)
  arch_do_domctl()
    raw_copy_from_guest()
      copy_from_user_hvm()
        hvm_copy_from_guest_linear()
          __hvm_copy()

So no, we can't do the in_atomic() test IMO.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

WARNING: multiple messages have this Message-ID (diff)
From: Juergen Gross <jgross@suse.com>
To: Jan Beulich <JBeulich@suse.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Julien Grall <julien.grall@arm.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH 1/3] xen: drop in_atomic()
Date: Fri, 24 May 2019 07:41:54 +0200	[thread overview]
Message-ID: <316af0ff-904b-4cf7-1cb4-08c4ae69c4bd@suse.com> (raw)
Message-ID: <20190524054154.fAgLS9JOuMRH4jfqr88DUz0km81rqbcbqvAXrySfOlY@z> (raw)
In-Reply-To: <5CE52014020000780023147E@prv1-mh.provo.novell.com>

On 22/05/2019 12:10, Jan Beulich wrote:
>>>> On 22.05.19 at 11:45, <jgross@suse.com> wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -3185,22 +3185,6 @@ static enum hvm_translation_result __hvm_copy(
>>  
>>      ASSERT(is_hvm_vcpu(v));
>>  
>> -    /*
>> -     * XXX Disable for 4.1.0: PV-on-HVM drivers will do grant-table ops
>> -     * such as query_size. Grant-table code currently does copy_to/from_guest
>> -     * accesses under the big per-domain lock, which this test would disallow.
>> -     * The test is not needed until we implement sleeping-on-waitqueue when
>> -     * we access a paged-out frame, and that's post 4.1.0 now.
>> -     */
>> -#if 0
>> -    /*
>> -     * If the required guest memory is paged out, this function may sleep.
>> -     * Hence we bail immediately if called from atomic context.
>> -     */
>> -    if ( in_atomic() )
>> -        return HVMTRANS_unhandleable;
>> -#endif
> 
> Dealing with this TODO item is of course much appreciated, but
> should it really be deleted altogether? The big-domain-lock issue
> is gone afair, in which case dropping the #if 0 would seem
> possible to me, even if it's not strictly needed without the sleep-
> on-waitqueue behavior mentioned.

I just had a look and found the following path:

do_domctl() (takes domctl_lock and hypercall_deadlock_mutex)
  arch_do_domctl()
    raw_copy_from_guest()
      copy_from_user_hvm()
        hvm_copy_from_guest_linear()
          __hvm_copy()

So no, we can't do the in_atomic() test IMO.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply	other threads:[~2019-05-24  5:41 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-22  9:45 [PATCH 0/3] tune preempt_[dis|en]able() Juergen Gross
2019-05-22  9:45 ` [Xen-devel] " Juergen Gross
2019-05-22  9:45 ` [PATCH 1/3] xen: drop in_atomic() Juergen Gross
2019-05-22  9:45   ` [Xen-devel] " Juergen Gross
2019-05-22  9:58   ` Andrew Cooper
2019-05-22  9:58     ` [Xen-devel] " Andrew Cooper
2019-05-22 10:10   ` Jan Beulich
2019-05-22 10:10     ` [Xen-devel] " Jan Beulich
2019-05-24  5:41     ` Juergen Gross [this message]
2019-05-24  5:41       ` Juergen Gross
2019-05-24  6:38       ` Jan Beulich
2019-05-24  6:38         ` [Xen-devel] " Jan Beulich
2019-05-24  8:34         ` Juergen Gross
2019-05-24  8:34           ` [Xen-devel] " Juergen Gross
2019-05-24  8:39           ` Jan Beulich
2019-05-24  8:39             ` [Xen-devel] " Jan Beulich
2019-05-24 12:30             ` Andrew Cooper
2019-05-24 12:30               ` [Xen-devel] " Andrew Cooper
2019-05-24 12:35               ` Jan Beulich
2019-05-24 12:35                 ` [Xen-devel] " Jan Beulich
     [not found]   ` <5CE52014020000780023147E@suse.com>
2019-05-22 10:19     ` Juergen Gross
2019-05-22 10:19       ` [Xen-devel] " Juergen Gross
2019-05-22 12:34       ` Jan Beulich
2019-05-22 12:34         ` [Xen-devel] " Jan Beulich
2019-05-22  9:45 ` [PATCH 2/3] xen: drop preempt_count() for non-debug builds Juergen Gross
2019-05-22  9:45   ` [Xen-devel] " Juergen Gross
2019-05-22 10:00   ` Andrew Cooper
2019-05-22 10:00     ` [Xen-devel] " Andrew Cooper
2019-05-22 10:17     ` Juergen Gross
2019-05-22 10:17       ` [Xen-devel] " Juergen Gross
2019-05-22 10:18     ` Jan Beulich
2019-05-22 10:18       ` [Xen-devel] " Jan Beulich
2019-05-22 10:39       ` Andrew Cooper
2019-05-22 10:39         ` [Xen-devel] " Andrew Cooper
2019-05-22 10:12   ` Jan Beulich
2019-05-22 10:12     ` [Xen-devel] " Jan Beulich
     [not found]   ` <5CE5207A0200007800231481@suse.com>
2019-05-22 10:17     ` Juergen Gross
2019-05-22 10:17       ` [Xen-devel] " Juergen Gross
2019-05-22  9:45 ` [PATCH 3/3] xen: build common/preempt.c only with CONFIG_DEBUG Juergen Gross
2019-05-22  9:45   ` [Xen-devel] " Juergen Gross
2019-05-22 13:01 [PATCH 1/3] xen: drop in_atomic() Juergen Gross

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=316af0ff-904b-4cf7-1cb4-08c4ae69c4bd@suse.com \
    --to=jgross@suse.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=julien.grall@arm.com \
    --cc=konrad.wilk@oracle.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.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.