* Re: [PATCH] Fix shared entry status for grant copy operation on paged out gfn
2012-08-21 20:13 [PATCH] Fix shared entry status for grant copy operation on paged out gfn Andres Lagar-Cavilla
@ 2012-08-21 20:11 ` Andres Lagar-Cavilla
2012-08-22 14:21 ` Jan Beulich
1 sibling, 0 replies; 5+ messages in thread
From: Andres Lagar-Cavilla @ 2012-08-21 20:11 UTC (permalink / raw)
To: Andres Lagar-Cavilla
Cc: Keir Fraser, Tim Deegan, Ian Campbell, Jan Beulich, xen-devel
NB: this intended for 4.2 window as it solves a paging-related BUG
Andres
On Aug 21, 2012, at 4:13 PM, Andres Lagar-Cavilla wrote:
> xen/common/grant_table.c | 33 ++++++++++++++++++++++-----------
> 1 files changed, 22 insertions(+), 11 deletions(-)
>
>
> The unwind path was not clearing the shared entry status bits. This was
> BSOD-ing guests on network activity under certain configurations.
>
> Also:
> * sed the fixup method name to signal it's related to grant copy.
> * use atomic clear flag ops during fixup.
>
> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
>
> diff -r 5267f78c8a1e -r 84a23ae853a5 xen/common/grant_table.c
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1751,14 +1751,14 @@ __release_grant_for_copy(
> under the domain's grant table lock. */
> /* Only safe on transitive grants. Even then, note that we don't
> attempt to drop any pin on the referent grant. */
> -static void __fixup_status_for_pin(const struct active_grant_entry *act,
> +static void __fixup_status_for_copy_pin(const struct active_grant_entry *act,
> uint16_t *status)
> {
> if ( !(act->pin & GNTPIN_hstw_mask) )
> - *status &= ~GTF_writing;
> + gnttab_clear_flag(_GTF_writing, status);
>
> if ( !(act->pin & GNTPIN_hstr_mask) )
> - *status &= ~GTF_reading;
> + gnttab_clear_flag(_GTF_reading, status);
> }
>
> /* Grab a frame number from a grant entry and update the flags and pin
> @@ -1834,7 +1834,7 @@ __acquire_grant_for_copy(
> if ( sha2 && (shah->flags & GTF_type_mask) == GTF_transitive )
> {
> if ( !allow_transitive )
> - PIN_FAIL(unlock_out, GNTST_general_error,
> + PIN_FAIL(unlock_out_clear, GNTST_general_error,
> "transitive grant when transitivity not allowed\n");
>
> trans_domid = sha2->transitive.trans_domid;
> @@ -1842,7 +1842,7 @@ __acquire_grant_for_copy(
> barrier(); /* Stop the compiler from re-loading
> trans_domid from shared memory */
> if ( trans_domid == rd->domain_id )
> - PIN_FAIL(unlock_out, GNTST_general_error,
> + PIN_FAIL(unlock_out_clear, GNTST_general_error,
> "transitive grants cannot be self-referential\n");
>
> /* We allow the trans_domid == ldom case, which
> @@ -1855,7 +1855,7 @@ __acquire_grant_for_copy(
> /* We need to leave the rrd locked during the grant copy */
> td = rcu_lock_domain_by_id(trans_domid);
> if ( td == NULL )
> - PIN_FAIL(unlock_out, GNTST_general_error,
> + PIN_FAIL(unlock_out_clear, GNTST_general_error,
> "transitive grant referenced bad domain %d\n",
> trans_domid);
> spin_unlock(&rgt->lock);
> @@ -1866,7 +1866,7 @@ __acquire_grant_for_copy(
>
> spin_lock(&rgt->lock);
> if ( rc != GNTST_okay ) {
> - __fixup_status_for_pin(act, status);
> + __fixup_status_for_copy_pin(act, status);
> rcu_unlock_domain(td);
> spin_unlock(&rgt->lock);
> return rc;
> @@ -1878,7 +1878,7 @@ __acquire_grant_for_copy(
> and try again. */
> if ( act->pin != old_pin )
> {
> - __fixup_status_for_pin(act, status);
> + __fixup_status_for_copy_pin(act, status);
> rcu_unlock_domain(td);
> spin_unlock(&rgt->lock);
> put_page(*page);
> @@ -1897,7 +1897,7 @@ __acquire_grant_for_copy(
> {
> rc = __get_paged_frame(sha1->frame, &grant_frame, page, readonly, rd);
> if ( rc != GNTST_okay )
> - goto unlock_out;
> + goto unlock_out_clear;
> act->gfn = sha1->frame;
> is_sub_page = 0;
> trans_page_off = 0;
> @@ -1907,7 +1907,7 @@ __acquire_grant_for_copy(
> {
> rc = __get_paged_frame(sha2->full_page.frame, &grant_frame, page, readonly, rd);
> if ( rc != GNTST_okay )
> - goto unlock_out;
> + goto unlock_out_clear;
> act->gfn = sha2->full_page.frame;
> is_sub_page = 0;
> trans_page_off = 0;
> @@ -1917,7 +1917,7 @@ __acquire_grant_for_copy(
> {
> rc = __get_paged_frame(sha2->sub_page.frame, &grant_frame, page, readonly, rd);
> if ( rc != GNTST_okay )
> - goto unlock_out;
> + goto unlock_out_clear;
> act->gfn = sha2->sub_page.frame;
> is_sub_page = 1;
> trans_page_off = sha2->sub_page.page_off;
> @@ -1948,6 +1948,17 @@ __acquire_grant_for_copy(
> *length = act->length;
> *frame = act->frame;
>
> + spin_unlock(&rgt->lock);
> + return rc;
> +
> + unlock_out_clear:
> + if ( !(readonly) &&
> + !(act->pin & GNTPIN_hstw_mask) )
> + gnttab_clear_flag(_GTF_writing, status);
> +
> + if ( !act->pin )
> + gnttab_clear_flag(_GTF_reading, status);
> +
> unlock_out:
> spin_unlock(&rgt->lock);
> return rc;
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Fix shared entry status for grant copy operation on paged out gfn
@ 2012-08-21 20:13 Andres Lagar-Cavilla
2012-08-21 20:11 ` Andres Lagar-Cavilla
2012-08-22 14:21 ` Jan Beulich
0 siblings, 2 replies; 5+ messages in thread
From: Andres Lagar-Cavilla @ 2012-08-21 20:13 UTC (permalink / raw)
To: xen-devel; +Cc: keir, andres, tim, JBeulich
xen/common/grant_table.c | 33 ++++++++++++++++++++++-----------
1 files changed, 22 insertions(+), 11 deletions(-)
The unwind path was not clearing the shared entry status bits. This was
BSOD-ing guests on network activity under certain configurations.
Also:
* sed the fixup method name to signal it's related to grant copy.
* use atomic clear flag ops during fixup.
Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
diff -r 5267f78c8a1e -r 84a23ae853a5 xen/common/grant_table.c
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1751,14 +1751,14 @@ __release_grant_for_copy(
under the domain's grant table lock. */
/* Only safe on transitive grants. Even then, note that we don't
attempt to drop any pin on the referent grant. */
-static void __fixup_status_for_pin(const struct active_grant_entry *act,
+static void __fixup_status_for_copy_pin(const struct active_grant_entry *act,
uint16_t *status)
{
if ( !(act->pin & GNTPIN_hstw_mask) )
- *status &= ~GTF_writing;
+ gnttab_clear_flag(_GTF_writing, status);
if ( !(act->pin & GNTPIN_hstr_mask) )
- *status &= ~GTF_reading;
+ gnttab_clear_flag(_GTF_reading, status);
}
/* Grab a frame number from a grant entry and update the flags and pin
@@ -1834,7 +1834,7 @@ __acquire_grant_for_copy(
if ( sha2 && (shah->flags & GTF_type_mask) == GTF_transitive )
{
if ( !allow_transitive )
- PIN_FAIL(unlock_out, GNTST_general_error,
+ PIN_FAIL(unlock_out_clear, GNTST_general_error,
"transitive grant when transitivity not allowed\n");
trans_domid = sha2->transitive.trans_domid;
@@ -1842,7 +1842,7 @@ __acquire_grant_for_copy(
barrier(); /* Stop the compiler from re-loading
trans_domid from shared memory */
if ( trans_domid == rd->domain_id )
- PIN_FAIL(unlock_out, GNTST_general_error,
+ PIN_FAIL(unlock_out_clear, GNTST_general_error,
"transitive grants cannot be self-referential\n");
/* We allow the trans_domid == ldom case, which
@@ -1855,7 +1855,7 @@ __acquire_grant_for_copy(
/* We need to leave the rrd locked during the grant copy */
td = rcu_lock_domain_by_id(trans_domid);
if ( td == NULL )
- PIN_FAIL(unlock_out, GNTST_general_error,
+ PIN_FAIL(unlock_out_clear, GNTST_general_error,
"transitive grant referenced bad domain %d\n",
trans_domid);
spin_unlock(&rgt->lock);
@@ -1866,7 +1866,7 @@ __acquire_grant_for_copy(
spin_lock(&rgt->lock);
if ( rc != GNTST_okay ) {
- __fixup_status_for_pin(act, status);
+ __fixup_status_for_copy_pin(act, status);
rcu_unlock_domain(td);
spin_unlock(&rgt->lock);
return rc;
@@ -1878,7 +1878,7 @@ __acquire_grant_for_copy(
and try again. */
if ( act->pin != old_pin )
{
- __fixup_status_for_pin(act, status);
+ __fixup_status_for_copy_pin(act, status);
rcu_unlock_domain(td);
spin_unlock(&rgt->lock);
put_page(*page);
@@ -1897,7 +1897,7 @@ __acquire_grant_for_copy(
{
rc = __get_paged_frame(sha1->frame, &grant_frame, page, readonly, rd);
if ( rc != GNTST_okay )
- goto unlock_out;
+ goto unlock_out_clear;
act->gfn = sha1->frame;
is_sub_page = 0;
trans_page_off = 0;
@@ -1907,7 +1907,7 @@ __acquire_grant_for_copy(
{
rc = __get_paged_frame(sha2->full_page.frame, &grant_frame, page, readonly, rd);
if ( rc != GNTST_okay )
- goto unlock_out;
+ goto unlock_out_clear;
act->gfn = sha2->full_page.frame;
is_sub_page = 0;
trans_page_off = 0;
@@ -1917,7 +1917,7 @@ __acquire_grant_for_copy(
{
rc = __get_paged_frame(sha2->sub_page.frame, &grant_frame, page, readonly, rd);
if ( rc != GNTST_okay )
- goto unlock_out;
+ goto unlock_out_clear;
act->gfn = sha2->sub_page.frame;
is_sub_page = 1;
trans_page_off = sha2->sub_page.page_off;
@@ -1948,6 +1948,17 @@ __acquire_grant_for_copy(
*length = act->length;
*frame = act->frame;
+ spin_unlock(&rgt->lock);
+ return rc;
+
+ unlock_out_clear:
+ if ( !(readonly) &&
+ !(act->pin & GNTPIN_hstw_mask) )
+ gnttab_clear_flag(_GTF_writing, status);
+
+ if ( !act->pin )
+ gnttab_clear_flag(_GTF_reading, status);
+
unlock_out:
spin_unlock(&rgt->lock);
return rc;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix shared entry status for grant copy operation on paged out gfn
2012-08-21 20:13 [PATCH] Fix shared entry status for grant copy operation on paged out gfn Andres Lagar-Cavilla
2012-08-21 20:11 ` Andres Lagar-Cavilla
@ 2012-08-22 14:21 ` Jan Beulich
2012-08-22 14:32 ` Andres Lagar-Cavilla
1 sibling, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2012-08-22 14:21 UTC (permalink / raw)
To: Andres Lagar-Cavilla; +Cc: tim, andres, keir, xen-devel
>>> On 21.08.12 at 22:13, Andres Lagar-Cavilla <andres@lagarcavilla.org> wrote:
> xen/common/grant_table.c | 33 ++++++++++++++++++++++-----------
> 1 files changed, 22 insertions(+), 11 deletions(-)
>
>
> The unwind path was not clearing the shared entry status bits. This was
> BSOD-ing guests on network activity under certain configurations.
>
> Also:
> * sed the fixup method name to signal it's related to grant copy.
> * use atomic clear flag ops during fixup.
Is that last thing really needed? I remember having looked at
these non-atomic operations too a little while back, and came to
the conclusion that probably the authors intentionally coded it
that way.
jan
> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
>
> diff -r 5267f78c8a1e -r 84a23ae853a5 xen/common/grant_table.c
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1751,14 +1751,14 @@ __release_grant_for_copy(
> under the domain's grant table lock. */
> /* Only safe on transitive grants. Even then, note that we don't
> attempt to drop any pin on the referent grant. */
> -static void __fixup_status_for_pin(const struct active_grant_entry *act,
> +static void __fixup_status_for_copy_pin(const struct active_grant_entry
> *act,
> uint16_t *status)
> {
> if ( !(act->pin & GNTPIN_hstw_mask) )
> - *status &= ~GTF_writing;
> + gnttab_clear_flag(_GTF_writing, status);
>
> if ( !(act->pin & GNTPIN_hstr_mask) )
> - *status &= ~GTF_reading;
> + gnttab_clear_flag(_GTF_reading, status);
> }
>
> /* Grab a frame number from a grant entry and update the flags and pin
> @@ -1834,7 +1834,7 @@ __acquire_grant_for_copy(
> if ( sha2 && (shah->flags & GTF_type_mask) == GTF_transitive )
> {
> if ( !allow_transitive )
> - PIN_FAIL(unlock_out, GNTST_general_error,
> + PIN_FAIL(unlock_out_clear, GNTST_general_error,
> "transitive grant when transitivity not
> allowed\n");
>
> trans_domid = sha2->transitive.trans_domid;
> @@ -1842,7 +1842,7 @@ __acquire_grant_for_copy(
> barrier(); /* Stop the compiler from re-loading
> trans_domid from shared memory */
> if ( trans_domid == rd->domain_id )
> - PIN_FAIL(unlock_out, GNTST_general_error,
> + PIN_FAIL(unlock_out_clear, GNTST_general_error,
> "transitive grants cannot be self-referential\n");
>
> /* We allow the trans_domid == ldom case, which
> @@ -1855,7 +1855,7 @@ __acquire_grant_for_copy(
> /* We need to leave the rrd locked during the grant copy */
> td = rcu_lock_domain_by_id(trans_domid);
> if ( td == NULL )
> - PIN_FAIL(unlock_out, GNTST_general_error,
> + PIN_FAIL(unlock_out_clear, GNTST_general_error,
> "transitive grant referenced bad domain %d\n",
> trans_domid);
> spin_unlock(&rgt->lock);
> @@ -1866,7 +1866,7 @@ __acquire_grant_for_copy(
>
> spin_lock(&rgt->lock);
> if ( rc != GNTST_okay ) {
> - __fixup_status_for_pin(act, status);
> + __fixup_status_for_copy_pin(act, status);
> rcu_unlock_domain(td);
> spin_unlock(&rgt->lock);
> return rc;
> @@ -1878,7 +1878,7 @@ __acquire_grant_for_copy(
> and try again. */
> if ( act->pin != old_pin )
> {
> - __fixup_status_for_pin(act, status);
> + __fixup_status_for_copy_pin(act, status);
> rcu_unlock_domain(td);
> spin_unlock(&rgt->lock);
> put_page(*page);
> @@ -1897,7 +1897,7 @@ __acquire_grant_for_copy(
> {
> rc = __get_paged_frame(sha1->frame, &grant_frame, page, readonly,
> rd);
> if ( rc != GNTST_okay )
> - goto unlock_out;
> + goto unlock_out_clear;
> act->gfn = sha1->frame;
> is_sub_page = 0;
> trans_page_off = 0;
> @@ -1907,7 +1907,7 @@ __acquire_grant_for_copy(
> {
> rc = __get_paged_frame(sha2->full_page.frame, &grant_frame, page,
> readonly, rd);
> if ( rc != GNTST_okay )
> - goto unlock_out;
> + goto unlock_out_clear;
> act->gfn = sha2->full_page.frame;
> is_sub_page = 0;
> trans_page_off = 0;
> @@ -1917,7 +1917,7 @@ __acquire_grant_for_copy(
> {
> rc = __get_paged_frame(sha2->sub_page.frame, &grant_frame, page,
> readonly, rd);
> if ( rc != GNTST_okay )
> - goto unlock_out;
> + goto unlock_out_clear;
> act->gfn = sha2->sub_page.frame;
> is_sub_page = 1;
> trans_page_off = sha2->sub_page.page_off;
> @@ -1948,6 +1948,17 @@ __acquire_grant_for_copy(
> *length = act->length;
> *frame = act->frame;
>
> + spin_unlock(&rgt->lock);
> + return rc;
> +
> + unlock_out_clear:
> + if ( !(readonly) &&
> + !(act->pin & GNTPIN_hstw_mask) )
> + gnttab_clear_flag(_GTF_writing, status);
> +
> + if ( !act->pin )
> + gnttab_clear_flag(_GTF_reading, status);
> +
> unlock_out:
> spin_unlock(&rgt->lock);
> return rc;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix shared entry status for grant copy operation on paged out gfn
2012-08-22 14:21 ` Jan Beulich
@ 2012-08-22 14:32 ` Andres Lagar-Cavilla
2012-08-22 14:48 ` Jan Beulich
0 siblings, 1 reply; 5+ messages in thread
From: Andres Lagar-Cavilla @ 2012-08-22 14:32 UTC (permalink / raw)
To: Jan Beulich; +Cc: tim, keir, Andres Lagar-Cavilla, xen-devel
On Aug 22, 2012, at 10:21 AM, Jan Beulich wrote:
>>>> On 21.08.12 at 22:13, Andres Lagar-Cavilla <andres@lagarcavilla.org> wrote:
>> xen/common/grant_table.c | 33 ++++++++++++++++++++++-----------
>> 1 files changed, 22 insertions(+), 11 deletions(-)
>>
>>
>> The unwind path was not clearing the shared entry status bits. This was
>> BSOD-ing guests on network activity under certain configurations.
>>
>> Also:
>> * sed the fixup method name to signal it's related to grant copy.
>> * use atomic clear flag ops during fixup.
>
> Is that last thing really needed? I remember having looked at
> these non-atomic operations too a little while back, and came to
> the conclusion that probably the authors intentionally coded it
> that way.
Due to some obscure property of transitive grants? All other flag clearing/setting in shared grant entries by the hypervisor is done atomically (GTF_transfer_completed being an exception).
>From my p.o.v. there is no downside. But I am not 100% certain and I can back it off.
Thanks
Andres
>
> jan
>
>> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
>>
>> diff -r 5267f78c8a1e -r 84a23ae853a5 xen/common/grant_table.c
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -1751,14 +1751,14 @@ __release_grant_for_copy(
>> under the domain's grant table lock. */
>> /* Only safe on transitive grants. Even then, note that we don't
>> attempt to drop any pin on the referent grant. */
>> -static void __fixup_status_for_pin(const struct active_grant_entry *act,
>> +static void __fixup_status_for_copy_pin(const struct active_grant_entry
>> *act,
>> uint16_t *status)
>> {
>> if ( !(act->pin & GNTPIN_hstw_mask) )
>> - *status &= ~GTF_writing;
>> + gnttab_clear_flag(_GTF_writing, status);
>>
>> if ( !(act->pin & GNTPIN_hstr_mask) )
>> - *status &= ~GTF_reading;
>> + gnttab_clear_flag(_GTF_reading, status);
>> }
>>
>> /* Grab a frame number from a grant entry and update the flags and pin
>> @@ -1834,7 +1834,7 @@ __acquire_grant_for_copy(
>> if ( sha2 && (shah->flags & GTF_type_mask) == GTF_transitive )
>> {
>> if ( !allow_transitive )
>> - PIN_FAIL(unlock_out, GNTST_general_error,
>> + PIN_FAIL(unlock_out_clear, GNTST_general_error,
>> "transitive grant when transitivity not
>> allowed\n");
>>
>> trans_domid = sha2->transitive.trans_domid;
>> @@ -1842,7 +1842,7 @@ __acquire_grant_for_copy(
>> barrier(); /* Stop the compiler from re-loading
>> trans_domid from shared memory */
>> if ( trans_domid == rd->domain_id )
>> - PIN_FAIL(unlock_out, GNTST_general_error,
>> + PIN_FAIL(unlock_out_clear, GNTST_general_error,
>> "transitive grants cannot be self-referential\n");
>>
>> /* We allow the trans_domid == ldom case, which
>> @@ -1855,7 +1855,7 @@ __acquire_grant_for_copy(
>> /* We need to leave the rrd locked during the grant copy */
>> td = rcu_lock_domain_by_id(trans_domid);
>> if ( td == NULL )
>> - PIN_FAIL(unlock_out, GNTST_general_error,
>> + PIN_FAIL(unlock_out_clear, GNTST_general_error,
>> "transitive grant referenced bad domain %d\n",
>> trans_domid);
>> spin_unlock(&rgt->lock);
>> @@ -1866,7 +1866,7 @@ __acquire_grant_for_copy(
>>
>> spin_lock(&rgt->lock);
>> if ( rc != GNTST_okay ) {
>> - __fixup_status_for_pin(act, status);
>> + __fixup_status_for_copy_pin(act, status);
>> rcu_unlock_domain(td);
>> spin_unlock(&rgt->lock);
>> return rc;
>> @@ -1878,7 +1878,7 @@ __acquire_grant_for_copy(
>> and try again. */
>> if ( act->pin != old_pin )
>> {
>> - __fixup_status_for_pin(act, status);
>> + __fixup_status_for_copy_pin(act, status);
>> rcu_unlock_domain(td);
>> spin_unlock(&rgt->lock);
>> put_page(*page);
>> @@ -1897,7 +1897,7 @@ __acquire_grant_for_copy(
>> {
>> rc = __get_paged_frame(sha1->frame, &grant_frame, page, readonly,
>> rd);
>> if ( rc != GNTST_okay )
>> - goto unlock_out;
>> + goto unlock_out_clear;
>> act->gfn = sha1->frame;
>> is_sub_page = 0;
>> trans_page_off = 0;
>> @@ -1907,7 +1907,7 @@ __acquire_grant_for_copy(
>> {
>> rc = __get_paged_frame(sha2->full_page.frame, &grant_frame, page,
>> readonly, rd);
>> if ( rc != GNTST_okay )
>> - goto unlock_out;
>> + goto unlock_out_clear;
>> act->gfn = sha2->full_page.frame;
>> is_sub_page = 0;
>> trans_page_off = 0;
>> @@ -1917,7 +1917,7 @@ __acquire_grant_for_copy(
>> {
>> rc = __get_paged_frame(sha2->sub_page.frame, &grant_frame, page,
>> readonly, rd);
>> if ( rc != GNTST_okay )
>> - goto unlock_out;
>> + goto unlock_out_clear;
>> act->gfn = sha2->sub_page.frame;
>> is_sub_page = 1;
>> trans_page_off = sha2->sub_page.page_off;
>> @@ -1948,6 +1948,17 @@ __acquire_grant_for_copy(
>> *length = act->length;
>> *frame = act->frame;
>>
>> + spin_unlock(&rgt->lock);
>> + return rc;
>> +
>> + unlock_out_clear:
>> + if ( !(readonly) &&
>> + !(act->pin & GNTPIN_hstw_mask) )
>> + gnttab_clear_flag(_GTF_writing, status);
>> +
>> + if ( !act->pin )
>> + gnttab_clear_flag(_GTF_reading, status);
>> +
>> unlock_out:
>> spin_unlock(&rgt->lock);
>> return rc;
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix shared entry status for grant copy operation on paged out gfn
2012-08-22 14:32 ` Andres Lagar-Cavilla
@ 2012-08-22 14:48 ` Jan Beulich
0 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2012-08-22 14:48 UTC (permalink / raw)
To: Andres Lagar-Cavilla; +Cc: tim, keir, Andres Lagar-Cavilla, xen-devel
>>> On 22.08.12 at 16:32, Andres Lagar-Cavilla <andreslc@gridcentric.ca> wrote:
> On Aug 22, 2012, at 10:21 AM, Jan Beulich wrote:
>
>>>>> On 21.08.12 at 22:13, Andres Lagar-Cavilla <andres@lagarcavilla.org> wrote:
>>> xen/common/grant_table.c | 33 ++++++++++++++++++++++-----------
>>> 1 files changed, 22 insertions(+), 11 deletions(-)
>>>
>>>
>>> The unwind path was not clearing the shared entry status bits. This was
>>> BSOD-ing guests on network activity under certain configurations.
>>>
>>> Also:
>>> * sed the fixup method name to signal it's related to grant copy.
>>> * use atomic clear flag ops during fixup.
>>
>> Is that last thing really needed? I remember having looked at
>> these non-atomic operations too a little while back, and came to
>> the conclusion that probably the authors intentionally coded it
>> that way.
>
> Due to some obscure property of transitive grants? All other flag
> clearing/setting in shared grant entries by the hypervisor is done atomically
> (GTF_transfer_completed being an exception).
>
> From my p.o.v. there is no downside. But I am not 100% certain and I can
> back it off.
Only if they read the thread and respond with an explanation.
After all, the change is certainly not wrong, just slightly slowing
things down.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-08-22 14:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-21 20:13 [PATCH] Fix shared entry status for grant copy operation on paged out gfn Andres Lagar-Cavilla
2012-08-21 20:11 ` Andres Lagar-Cavilla
2012-08-22 14:21 ` Jan Beulich
2012-08-22 14:32 ` Andres Lagar-Cavilla
2012-08-22 14:48 ` Jan Beulich
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.