* [PATCH 1/2] docs: fix persistent grants doc typo
@ 2012-11-27 10:03 Roger Pau Monne
2012-11-27 10:03 ` [PATCH 2/2] docs: expand persistent grants protocol Roger Pau Monne
2012-11-27 11:29 ` [PATCH 1/2] docs: fix persistent grants doc typo Ian Campbell
0 siblings, 2 replies; 10+ messages in thread
From: Roger Pau Monne @ 2012-11-27 10:03 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
xen/include/public/io/blkif.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
index accdda4..db9c379 100644
--- a/xen/include/public/io/blkif.h
+++ b/xen/include/public/io/blkif.h
@@ -307,7 +307,7 @@
* the grants.
* (8) The frontend driver has to allow the backend driver to map all grants
* with write access, even when they should be mapped read-only, since
- * further requests may reuse this grants and require write permisions.
+ * further requests may reuse this grants and require write permissions.
*/
/*
--
1.7.7.5 (Apple Git-26)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] docs: expand persistent grants protocol
2012-11-27 10:03 [PATCH 1/2] docs: fix persistent grants doc typo Roger Pau Monne
@ 2012-11-27 10:03 ` Roger Pau Monne
2012-11-29 10:49 ` Ian Campbell
2012-11-27 11:29 ` [PATCH 1/2] docs: fix persistent grants doc typo Ian Campbell
1 sibling, 1 reply; 10+ messages in thread
From: Roger Pau Monne @ 2012-11-27 10:03 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
xen/include/public/io/blkif.h | 24 +++++++++++++++++++++---
1 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
index db9c379..5a4b9ae 100644
--- a/xen/include/public/io/blkif.h
+++ b/xen/include/public/io/blkif.h
@@ -137,7 +137,15 @@
* can map persistently depends on the implementation, but ideally it
* should be RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST. Using this
* feature the backend doesn't need to unmap each grant, preventing
- * costly TLB flushes.
+ * costly TLB flushes. The backend driver should only map grants
+ * persistently if the frontend supports it. If a backend driver chooses
+ * to use the persistent protocol when the frontend doesn't support it,
+ * it will probably hit the maximum number of persistently mapped grants
+ * (due to the fact that the frontend won't be reusing the same grants),
+ * and fall back to non-persistent mode. Backend implementations may
+ * shrink or expand the number of persistently mapped grants without
+ * notifying the frontend depending on memory constraints (this might
+ * cause a performance degradation).
*
*----------------------- Request Transport Parameters ------------------------
*
@@ -258,11 +266,17 @@
* feature-persistent
* Values: 0/1 (boolean)
* Default Value: 0
- * Notes: 7, 8
+ * Notes: 7, 8, 9
*
* A value of "1" indicates that the frontend will reuse the same grants
* for all transactions, allowing the backend to map them with write
- * access (even when it should be read-only).
+ * access (even when it should be read-only). If the frontend hits the
+ * maximum number of allowed persistenlty mapped grants, it can fallback
+ * to non persistent mode. This will cause a performance degradation,
+ * since the the backend driver will still try to map those grants
+ * persistently. Since the persistent grants protocol is compatible with
+ * the previous protocol, a frontend driver can choose to work in
+ * persistent mode even when the backend doesn't support it.
*
*------------------------- Virtual Device Properties -------------------------
*
@@ -308,6 +322,10 @@
* (8) The frontend driver has to allow the backend driver to map all grants
* with write access, even when they should be mapped read-only, since
* further requests may reuse this grants and require write permissions.
+ * (9) Linux implementation doesn't have a limit on the maximum number of
+ * grants that can be persisntly mapped in the frontend driver, but
+ * due to the frontent driver implementation it should never be bigger
+ * than RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST.
*/
/*
--
1.7.7.5 (Apple Git-26)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] docs: fix persistent grants doc typo
2012-11-27 10:03 [PATCH 1/2] docs: fix persistent grants doc typo Roger Pau Monne
2012-11-27 10:03 ` [PATCH 2/2] docs: expand persistent grants protocol Roger Pau Monne
@ 2012-11-27 11:29 ` Ian Campbell
2012-11-28 15:35 ` Roger Pau Monné
1 sibling, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2012-11-27 11:29 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel@lists.xen.org
On Tue, 2012-11-27 at 10:03 +0000, Roger Pau Monne wrote:
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> xen/include/public/io/blkif.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
> index accdda4..db9c379 100644
> --- a/xen/include/public/io/blkif.h
> +++ b/xen/include/public/io/blkif.h
> @@ -307,7 +307,7 @@
> * the grants.
> * (8) The frontend driver has to allow the backend driver to map all grants
> * with write access, even when they should be mapped read-only, since
> - * further requests may reuse this grants and require write permisions.
> + * further requests may reuse this grants and require write permissions.
"these grants" probably? (alternatively "this grant" but I don't think
that is what is meant).
> */
>
> /*
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] docs: fix persistent grants doc typo
2012-11-27 11:29 ` [PATCH 1/2] docs: fix persistent grants doc typo Ian Campbell
@ 2012-11-28 15:35 ` Roger Pau Monné
2012-11-28 15:59 ` Ian Campbell
0 siblings, 1 reply; 10+ messages in thread
From: Roger Pau Monné @ 2012-11-28 15:35 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org
On 27/11/12 12:29, Ian Campbell wrote:
> On Tue, 2012-11-27 at 10:03 +0000, Roger Pau Monne wrote:
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> ---
>> xen/include/public/io/blkif.h | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
>> index accdda4..db9c379 100644
>> --- a/xen/include/public/io/blkif.h
>> +++ b/xen/include/public/io/blkif.h
>> @@ -307,7 +307,7 @@
>> * the grants.
>> * (8) The frontend driver has to allow the backend driver to map all grants
>> * with write access, even when they should be mapped read-only, since
>> - * further requests may reuse this grants and require write permisions.
>> + * further requests may reuse this grants and require write permissions.
>
> "these grants" probably? (alternatively "this grant" but I don't think
> that is what is meant).
Yes, "these grants" is correct. Do I need to resend this, or can the
committer change it?
>
>> */
>>
>> /*
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] docs: fix persistent grants doc typo
2012-11-28 15:35 ` Roger Pau Monné
@ 2012-11-28 15:59 ` Ian Campbell
2012-11-29 11:28 ` Ian Campbell
0 siblings, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2012-11-28 15:59 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel@lists.xen.org
On Wed, 2012-11-28 at 15:35 +0000, Roger Pau Monne wrote:
> On 27/11/12 12:29, Ian Campbell wrote:
> > On Tue, 2012-11-27 at 10:03 +0000, Roger Pau Monne wrote:
> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >> ---
> >> xen/include/public/io/blkif.h | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
> >> index accdda4..db9c379 100644
> >> --- a/xen/include/public/io/blkif.h
> >> +++ b/xen/include/public/io/blkif.h
> >> @@ -307,7 +307,7 @@
> >> * the grants.
> >> * (8) The frontend driver has to allow the backend driver to map all grants
> >> * with write access, even when they should be mapped read-only, since
> >> - * further requests may reuse this grants and require write permisions.
> >> + * further requests may reuse this grants and require write permissions.
> >
> > "these grants" probably? (alternatively "this grant" but I don't think
> > that is what is meant).
>
> Yes, "these grants" is correct. Do I need to resend this, or can the
> committer change it?
I'll do it when I commit.
>
> >
> >> */
> >>
> >> /*
> >
> >
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] docs: expand persistent grants protocol
2012-11-27 10:03 ` [PATCH 2/2] docs: expand persistent grants protocol Roger Pau Monne
@ 2012-11-29 10:49 ` Ian Campbell
2012-11-29 11:30 ` Roger Pau Monné
0 siblings, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2012-11-29 10:49 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel@lists.xen.org
On Tue, 2012-11-27 at 10:03 +0000, Roger Pau Monne wrote:
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> xen/include/public/io/blkif.h | 24 +++++++++++++++++++++---
> 1 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
> index db9c379..5a4b9ae 100644
> --- a/xen/include/public/io/blkif.h
> +++ b/xen/include/public/io/blkif.h
> @@ -137,7 +137,15 @@
> * can map persistently depends on the implementation, but ideally it
> * should be RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST. Using this
> * feature the backend doesn't need to unmap each grant, preventing
> - * costly TLB flushes.
> + * costly TLB flushes. The backend driver should only map grants
> + * persistently if the frontend supports it. If a backend driver chooses
> + * to use the persistent protocol when the frontend doesn't support it,
> + * it will probably hit the maximum number of persistently mapped grants
> + * (due to the fact that the frontend won't be reusing the same grants),
> + * and fall back to non-persistent mode. Backend implementations may
> + * shrink or expand the number of persistently mapped grants without
> + * notifying the frontend depending on memory constraints (this might
> + * cause a performance degradation).
Is there a recommended/required reuse strategy on either end which
minimises this? You don't want to be in a situation where the backend's
"cache" is full of non-persistent single-shot mappings but the frontend
is now reusing a good set of persistent pages, which are getting
repeatedly mapped/unmapped because the cache is full...
> *
> *----------------------- Request Transport Parameters ------------------------
> *
> @@ -258,11 +266,17 @@
> * feature-persistent
> * Values: 0/1 (boolean)
> * Default Value: 0
> - * Notes: 7, 8
> + * Notes: 7, 8, 9
> *
> * A value of "1" indicates that the frontend will reuse the same grants
> * for all transactions, allowing the backend to map them with write
> - * access (even when it should be read-only).
> + * access (even when it should be read-only). If the frontend hits the
> + * maximum number of allowed persistenlty mapped grants, it can fallback
persistently
> + * to non persistent mode. This will cause a performance degradation,
> + * since the the backend driver will still try to map those grants
> + * persistently. Since the persistent grants protocol is compatible with
> + * the previous protocol, a frontend driver can choose to work in
> + * persistent mode even when the backend doesn't support it.
> *
> *------------------------- Virtual Device Properties -------------------------
> *
> @@ -308,6 +322,10 @@
> * (8) The frontend driver has to allow the backend driver to map all grants
> * with write access, even when they should be mapped read-only, since
> * further requests may reuse this grants and require write permissions.
> + * (9) Linux implementation doesn't have a limit on the maximum number of
> + * grants that can be persisntly mapped in the frontend driver, but
persistently
> + * due to the frontent driver implementation it should never be bigger
> + * than RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST.
> */
>
> /*
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] docs: fix persistent grants doc typo
2012-11-28 15:59 ` Ian Campbell
@ 2012-11-29 11:28 ` Ian Campbell
0 siblings, 0 replies; 10+ messages in thread
From: Ian Campbell @ 2012-11-29 11:28 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel@lists.xen.org
On Wed, 2012-11-28 at 15:59 +0000, Ian Campbell wrote:
> On Wed, 2012-11-28 at 15:35 +0000, Roger Pau Monne wrote:
> > On 27/11/12 12:29, Ian Campbell wrote:
> > > On Tue, 2012-11-27 at 10:03 +0000, Roger Pau Monne wrote:
> > >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > >> ---
> > >> xen/include/public/io/blkif.h | 2 +-
> > >> 1 files changed, 1 insertions(+), 1 deletions(-)
> > >>
> > >> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
> > >> index accdda4..db9c379 100644
> > >> --- a/xen/include/public/io/blkif.h
> > >> +++ b/xen/include/public/io/blkif.h
> > >> @@ -307,7 +307,7 @@
> > >> * the grants.
> > >> * (8) The frontend driver has to allow the backend driver to map all grants
> > >> * with write access, even when they should be mapped read-only, since
> > >> - * further requests may reuse this grants and require write permisions.
> > >> + * further requests may reuse this grants and require write permissions.
> > >
> > > "these grants" probably? (alternatively "this grant" but I don't think
> > > that is what is meant).
> >
> > Yes, "these grants" is correct. Do I need to resend this, or can the
> > committer change it?
>
> I'll do it when I commit.
Which I've now done.
>
> >
> > >
> > >> */
> > >>
> > >> /*
> > >
> > >
> >
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] docs: expand persistent grants protocol
2012-11-29 10:49 ` Ian Campbell
@ 2012-11-29 11:30 ` Roger Pau Monné
2012-11-29 11:34 ` Ian Campbell
0 siblings, 1 reply; 10+ messages in thread
From: Roger Pau Monné @ 2012-11-29 11:30 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org
On 29/11/12 11:49, Ian Campbell wrote:
> On Tue, 2012-11-27 at 10:03 +0000, Roger Pau Monne wrote:
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> ---
>> xen/include/public/io/blkif.h | 24 +++++++++++++++++++++---
>> 1 files changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
>> index db9c379..5a4b9ae 100644
>> --- a/xen/include/public/io/blkif.h
>> +++ b/xen/include/public/io/blkif.h
>> @@ -137,7 +137,15 @@
>> * can map persistently depends on the implementation, but ideally it
>> * should be RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST. Using this
>> * feature the backend doesn't need to unmap each grant, preventing
>> - * costly TLB flushes.
>> + * costly TLB flushes. The backend driver should only map grants
>> + * persistently if the frontend supports it. If a backend driver chooses
>> + * to use the persistent protocol when the frontend doesn't support it,
>> + * it will probably hit the maximum number of persistently mapped grants
>> + * (due to the fact that the frontend won't be reusing the same grants),
>> + * and fall back to non-persistent mode. Backend implementations may
>> + * shrink or expand the number of persistently mapped grants without
>> + * notifying the frontend depending on memory constraints (this might
>> + * cause a performance degradation).
>
> Is there a recommended/required reuse strategy on either end which
> minimises this? You don't want to be in a situation where the backend's
> "cache" is full of non-persistent single-shot mappings but the frontend
> is now reusing a good set of persistent pages, which are getting
> repeatedly mapped/unmapped because the cache is full...
Since the only backend implementation is the Linux one, and we set the
maximum number of persistenly mapped grants to RING_SIZE *
BLKIF_MAX_SEGMENTS_PER_REQUEST, we don't have any kind of support for
shrinking/expanding. If a frontend is using more than this number of
grants it is considered that the frontend is broken/hostile.
If in the future some kind of shrinking/expanding is implemented, I
guess the natural way to do it would be to add a counter to keep track
of how many times a grant is used, and try to maintain the grants most
commonly used mapped.
On the blkfront side, we are using a LIFO strategy to store persistently
mapped grants, so we are trying to always reuse the same subset of
persistently mapped grants when blkfront is not under heavy I/O load.
>
>> *
>> *----------------------- Request Transport Parameters ------------------------
>> *
>> @@ -258,11 +266,17 @@
>> * feature-persistent
>> * Values: 0/1 (boolean)
>> * Default Value: 0
>> - * Notes: 7, 8
>> + * Notes: 7, 8, 9
>> *
>> * A value of "1" indicates that the frontend will reuse the same grants
>> * for all transactions, allowing the backend to map them with write
>> - * access (even when it should be read-only).
>> + * access (even when it should be read-only). If the frontend hits the
>> + * maximum number of allowed persistenlty mapped grants, it can fallback
>
> persistently
>
>> + * to non persistent mode. This will cause a performance degradation,
>> + * since the the backend driver will still try to map those grants
>> + * persistently. Since the persistent grants protocol is compatible with
>> + * the previous protocol, a frontend driver can choose to work in
>> + * persistent mode even when the backend doesn't support it.
>> *
>> *------------------------- Virtual Device Properties -------------------------
>> *
>> @@ -308,6 +322,10 @@
>> * (8) The frontend driver has to allow the backend driver to map all grants
>> * with write access, even when they should be mapped read-only, since
>> * further requests may reuse this grants and require write permissions.
>> + * (9) Linux implementation doesn't have a limit on the maximum number of
>> + * grants that can be persisntly mapped in the frontend driver, but
>
> persistently
>
>> + * due to the frontent driver implementation it should never be bigger
>> + * than RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST.
>> */
>>
>> /*
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] docs: expand persistent grants protocol
2012-11-29 11:30 ` Roger Pau Monné
@ 2012-11-29 11:34 ` Ian Campbell
2012-11-29 12:43 ` Roger Pau Monné
0 siblings, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2012-11-29 11:34 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel@lists.xen.org
On Thu, 2012-11-29 at 11:30 +0000, Roger Pau Monne wrote:
> On 29/11/12 11:49, Ian Campbell wrote:
> > On Tue, 2012-11-27 at 10:03 +0000, Roger Pau Monne wrote:
> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >> ---
> >> xen/include/public/io/blkif.h | 24 +++++++++++++++++++++---
> >> 1 files changed, 21 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
> >> index db9c379..5a4b9ae 100644
> >> --- a/xen/include/public/io/blkif.h
> >> +++ b/xen/include/public/io/blkif.h
> >> @@ -137,7 +137,15 @@
> >> * can map persistently depends on the implementation, but ideally it
> >> * should be RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST. Using this
> >> * feature the backend doesn't need to unmap each grant, preventing
> >> - * costly TLB flushes.
> >> + * costly TLB flushes. The backend driver should only map grants
> >> + * persistently if the frontend supports it. If a backend driver chooses
> >> + * to use the persistent protocol when the frontend doesn't support it,
> >> + * it will probably hit the maximum number of persistently mapped grants
> >> + * (due to the fact that the frontend won't be reusing the same grants),
> >> + * and fall back to non-persistent mode. Backend implementations may
> >> + * shrink or expand the number of persistently mapped grants without
> >> + * notifying the frontend depending on memory constraints (this might
> >> + * cause a performance degradation).
> >
> > Is there a recommended/required reuse strategy on either end which
> > minimises this? You don't want to be in a situation where the backend's
> > "cache" is full of non-persistent single-shot mappings but the frontend
> > is now reusing a good set of persistent pages, which are getting
> > repeatedly mapped/unmapped because the cache is full...
>
> Since the only backend implementation is the Linux one, and we set the
> maximum number of persistenly mapped grants to RING_SIZE *
> BLKIF_MAX_SEGMENTS_PER_REQUEST, we don't have any kind of support for
> shrinking/expanding. If a frontend is using more than this number of
> grants it is considered that the frontend is broken/hostile.
>
> If in the future some kind of shrinking/expanding is implemented, I
> guess the natural way to do it would be to add a counter to keep track
> of how many times a grant is used, and try to maintain the grants most
> commonly used mapped.
You mean expire the mappings on an LRU basis? That seems sensible and
would be compatible with a LIFO strategy in the f.e.
NB this doesn't require shrinking/expanding in the backend, but can also
happen with a broken f.e. as you observe. We should still try and do
something sane with a f.e. which follows our recommendations though.
> On the blkfront side, we are using a LIFO strategy to store persistently
> mapped grants, so we are trying to always reuse the same subset of
> persistently mapped grants when blkfront is not under heavy I/O load.
I think this is worth documenting as an actual implementation
recommendation, to improve interoperability. If someone happened to
implement a FIFO instead then they would get some sort of pathological
behaviour.
It may even be worth documenting that the b.e. should use an LRU scheme.
Ian,
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] docs: expand persistent grants protocol
2012-11-29 11:34 ` Ian Campbell
@ 2012-11-29 12:43 ` Roger Pau Monné
0 siblings, 0 replies; 10+ messages in thread
From: Roger Pau Monné @ 2012-11-29 12:43 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org
On 29/11/12 12:34, Ian Campbell wrote:
> On Thu, 2012-11-29 at 11:30 +0000, Roger Pau Monne wrote:
>> On 29/11/12 11:49, Ian Campbell wrote:
>>> On Tue, 2012-11-27 at 10:03 +0000, Roger Pau Monne wrote:
>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>> ---
>>>> xen/include/public/io/blkif.h | 24 +++++++++++++++++++++---
>>>> 1 files changed, 21 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
>>>> index db9c379..5a4b9ae 100644
>>>> --- a/xen/include/public/io/blkif.h
>>>> +++ b/xen/include/public/io/blkif.h
>>>> @@ -137,7 +137,15 @@
>>>> * can map persistently depends on the implementation, but ideally it
>>>> * should be RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST. Using this
>>>> * feature the backend doesn't need to unmap each grant, preventing
>>>> - * costly TLB flushes.
>>>> + * costly TLB flushes. The backend driver should only map grants
>>>> + * persistently if the frontend supports it. If a backend driver chooses
>>>> + * to use the persistent protocol when the frontend doesn't support it,
>>>> + * it will probably hit the maximum number of persistently mapped grants
>>>> + * (due to the fact that the frontend won't be reusing the same grants),
>>>> + * and fall back to non-persistent mode. Backend implementations may
>>>> + * shrink or expand the number of persistently mapped grants without
>>>> + * notifying the frontend depending on memory constraints (this might
>>>> + * cause a performance degradation).
>>>
>>> Is there a recommended/required reuse strategy on either end which
>>> minimises this? You don't want to be in a situation where the backend's
>>> "cache" is full of non-persistent single-shot mappings but the frontend
>>> is now reusing a good set of persistent pages, which are getting
>>> repeatedly mapped/unmapped because the cache is full...
>>
>> Since the only backend implementation is the Linux one, and we set the
>> maximum number of persistenly mapped grants to RING_SIZE *
>> BLKIF_MAX_SEGMENTS_PER_REQUEST, we don't have any kind of support for
>> shrinking/expanding. If a frontend is using more than this number of
>> grants it is considered that the frontend is broken/hostile.
>>
>> If in the future some kind of shrinking/expanding is implemented, I
>> guess the natural way to do it would be to add a counter to keep track
>> of how many times a grant is used, and try to maintain the grants most
>> commonly used mapped.
>
> You mean expire the mappings on an LRU basis? That seems sensible and
> would be compatible with a LIFO strategy in the f.e.
>
> NB this doesn't require shrinking/expanding in the backend, but can also
> happen with a broken f.e. as you observe. We should still try and do
> something sane with a f.e. which follows our recommendations though.
We should recommend that all implementations use a LIFO queue in the
frontend in case a backend decides to limit the number of mapped grants
to something less than RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST, in
which case the recommendation for shrinking should be to use a LRU
strategy in the backend.
>
>> On the blkfront side, we are using a LIFO strategy to store persistently
>> mapped grants, so we are trying to always reuse the same subset of
>> persistently mapped grants when blkfront is not under heavy I/O load.
>
> I think this is worth documenting as an actual implementation
> recommendation, to improve interoperability. If someone happened to
> implement a FIFO instead then they would get some sort of pathological
> behaviour.
>
> It may even be worth documenting that the b.e. should use an LRU scheme.
Thanks for the review, I'm going to add the recommendations we talked
about to the protocol and resubmit.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-11-29 12:43 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-27 10:03 [PATCH 1/2] docs: fix persistent grants doc typo Roger Pau Monne
2012-11-27 10:03 ` [PATCH 2/2] docs: expand persistent grants protocol Roger Pau Monne
2012-11-29 10:49 ` Ian Campbell
2012-11-29 11:30 ` Roger Pau Monné
2012-11-29 11:34 ` Ian Campbell
2012-11-29 12:43 ` Roger Pau Monné
2012-11-27 11:29 ` [PATCH 1/2] docs: fix persistent grants doc typo Ian Campbell
2012-11-28 15:35 ` Roger Pau Monné
2012-11-28 15:59 ` Ian Campbell
2012-11-29 11:28 ` Ian Campbell
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.