[Xen-devel] [PATCH v5 4/8] xen: make grant resource limits per domain
Instead of using the same global resource limits of grant tables (max.
number of grant frames, max. number of maptrack frames) for all domains
make these limits per domain. This will allow setting individual limits
in the future. For now initialize the per domain limits with the global
values.
Signed-off-by: Juergen Gross
Reviewed-by: Paul Durrant
Reviewed-by: Wei Liu
---
V3:
- correct error message (Paul Durrant)
---
xen/common/grant_table.c | 82 ++--
1 file changed, 45 insertions(+), 37 deletions(-)
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 29e7fa539b..ff735a4b47 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -71,6 +71,9 @@ struct grant_table {
* what version to use yet.
*/
unsigned gt_version;
+/* Resource limits of the domain. */
+unsigned int max_grant_frames;
+unsigned int max_maptrack_frames;
};
#ifndef DEFAULT_MAX_NR_GRANT_FRAMES /* to allow arch to override */
@@ -287,8 +290,8 @@ num_act_frames_from_sha_frames(const unsigned int num)
return DIV_ROUND_UP(num * sha_per_page, ACGNT_PER_PAGE);
}
-#define max_nr_active_grant_frames \
-num_act_frames_from_sha_frames(max_grant_frames)
+#define max_nr_active_grant_frames(gt) \
+num_act_frames_from_sha_frames(gt->max_grant_frames)
static inline unsigned int
nr_active_grant_frames(struct grant_table *gt)
@@ -526,7 +529,7 @@ get_maptrack_handle(
* out of memory, try stealing an entry from another VCPU (in case the
* guest isn't mapping across its VCPUs evenly).
*/
-if ( nr_maptrack_frames(lgt) < max_maptrack_frames )
+if ( nr_maptrack_frames(lgt) < lgt->max_maptrack_frames )
new_mt = alloc_xenheap_page();
if ( !new_mt )
@@ -1664,14 +1667,15 @@ grant_table_init(struct domain *d)
if ( gt->nr_grant_frames )
return 0;
-gt->nr_grant_frames = INITIAL_NR_GRANT_FRAMES;
+gt->nr_grant_frames = min_t(unsigned int, INITIAL_NR_GRANT_FRAMES,
+ gt->max_grant_frames);
/* Active grant table. */
if ( (gt->active = xzalloc_array(struct active_grant_entry *,
- max_nr_active_grant_frames)) == NULL )
+ max_nr_active_grant_frames(gt))) == NULL )
goto no_mem_1;
for ( i = 0;
- i < num_act_frames_from_sha_frames(INITIAL_NR_GRANT_FRAMES); i++ )
+ i < num_act_frames_from_sha_frames(gt->nr_grant_frames); i++ )
{
if ( (gt->active[i] = alloc_xenheap_page()) == NULL )
goto no_mem_2;
@@ -1681,14 +1685,14 @@ grant_table_init(struct domain *d)
}
/* Tracking of mapped foreign frames table */
-gt->maptrack = vzalloc(max_maptrack_frames * sizeof(*gt->maptrack));
+gt->maptrack = vzalloc(gt->max_maptrack_frames * sizeof(*gt->maptrack));
if ( gt->maptrack == NULL )
goto no_mem_2;
/* Shared grant table. */
-if ( (gt->shared_raw = xzalloc_array(void *, max_grant_frames)) == NULL )
+if ( (gt->shared_raw = xzalloc_array(void *, gt->max_grant_frames)) ==
NULL )
goto no_mem_3;
-for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
+for ( i = 0; i < gt->nr_grant_frames; i++ )
{
if ( (gt->shared_raw[i] = alloc_xenheap_page()) == NULL )
goto no_mem_4;
@@ -1697,11 +1701,11 @@ grant_table_init(struct domain *d)
/* Status pages for grant table - for version 2 */
gt->status = xzalloc_array(grant_status_t *,
- grant_to_status_frames(max_grant_frames));
+ grant_to_status_frames(gt->max_grant_frames));
if ( gt->status == NULL )
goto no_mem_4;
-for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
+for ( i = 0; i < gt->nr_grant_frames; i++ )
gnttab_create_shared_page(d, gt, i);
gt->nr_status_frames = 0;
@@ -1709,7 +1713,7 @@ grant_table_init(struct domain *d)
return 0;
no_mem_4:
-for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
+for ( i = 0; i < gt->nr_grant_frames; i++ )
free_xenheap_page(gt->shared_raw[i]);
xfree(gt->shared_raw);
gt->shared_raw = NULL;
@@ -1718,7 +1722,7 @@ grant_table_init(struct domain *d)
gt->maptrack = NULL;
no_mem_2:
for ( i = 0;
- i < num_act_frames_from_sha_frames(INITIAL_NR_GRANT_FRAMES); i++ )
+ i < num_act_frames_from_sha_frames(gt->nr_grant_frames); i++ )
free_xenheap_page(gt->active[i]);
xfree(gt->active);
gt->active = NULL;
@@ -1743,7 +1747,7 @@ gnttab_grow_table(struct domain *d, unsigned int
req_nr_frames)
return 0;
}
-ASSERT(req_nr_frames <= max_grant_frames);
+ASSERT(req_nr_frames <= gt->max_grant_frames);
gdprintk(XENLOG_INFO,
"Expanding dom (%d) grant table from (%d) to (%d) frames.\n",
@@ -1815,15 +1819,6 @@ gnttab_setup_tabl
Re: [Xen-devel] [PATCH v5 4/8] xen: make grant resource limits per domain
>>> On 08.09.17 at 08:56, wrote:
> @@ -1843,6 +1838,14 @@ gnttab_setup_table(
> gt = d->grant_table;
> grant_write_lock(gt);
>
> +if ( unlikely(op.nr_frames > gt->max_grant_frames) )
> +{
> +gdprintk(XENLOG_INFO, "Domain is limited to %d grant-table
> frames.\n",
> +gt->max_grant_frames);
%u please
> @@ -3465,6 +3471,8 @@ grant_table_create(
> /* Simple stuff. */
> percpu_rwlock_resource_init(&t->lock, grant_rwlock);
> spin_lock_init(&t->maptrack_lock);
> +t->max_grant_frames = max_grant_frames;
> +t->max_maptrack_frames = max_maptrack_frames;
Am I mistaken or are these the only uses of the two static variables
now? If so (also to prove that's the case) their definitions would
probably better be moved into this function, together with their
integer_param() invocations. The adjustments done by
gnttab_usage_init() could also go here afaict.
> @@ -3755,6 +3763,12 @@ static void gnttab_usage_print(struct domain *rd)
>
> grant_read_lock(gt);
>
> +printk("grant-table for remote domain:%5d (v%d)\n"
> + " %d frames (%d max), %d maptrack frames (%d max)\n",
> + rd->domain_id, gt->gt_version,
> + nr_grant_frames(gt), gt->max_grant_frames,
> + nr_maptrack_frames(gt), gt->max_maptrack_frames);
Various %u instances again, and Dom%d please. Also you put this
after the table header, corrupting intended output.
> @@ -3782,12 +3796,7 @@ static void gnttab_usage_print(struct domain *rd)
> status = status_entry(gt, ref);
> }
>
> -if ( first )
> -{
> -printk("grant-table for remote domain:%5d (v%d)\n",
> - rd->domain_id, gt->gt_version);
> -first = 0;
> -}
> +first = 0;
Is it useful to print the per-table information when there are no
entries at all for a domain? I think it would be better to move
what you add as well as the table header into the if() that you
delete.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 4/8] xen: make grant resource limits per domain
On 08/09/17 17:44, Jan Beulich wrote:
On 08.09.17 at 08:56, wrote:
>> @@ -1843,6 +1838,14 @@ gnttab_setup_table(
>> gt = d->grant_table;
>> grant_write_lock(gt);
>>
>> +if ( unlikely(op.nr_frames > gt->max_grant_frames) )
>> +{
>> +gdprintk(XENLOG_INFO, "Domain is limited to %d grant-table
>> frames.\n",
>> +gt->max_grant_frames);
>
> %u please
Okay.
>
>> @@ -3465,6 +3471,8 @@ grant_table_create(
>> /* Simple stuff. */
>> percpu_rwlock_resource_init(&t->lock, grant_rwlock);
>> spin_lock_init(&t->maptrack_lock);
>> +t->max_grant_frames = max_grant_frames;
>> +t->max_maptrack_frames = max_maptrack_frames;
>
> Am I mistaken or are these the only uses of the two static variables
> now? If so (also to prove that's the case) their definitions would
> probably better be moved into this function, together with their
> integer_param() invocations. The adjustments done by
> gnttab_usage_init() could also go here afaict.
Okay.
>
>> @@ -3755,6 +3763,12 @@ static void gnttab_usage_print(struct domain *rd)
>>
>> grant_read_lock(gt);
>>
>> +printk("grant-table for remote domain:%5d (v%d)\n"
>> + " %d frames (%d max), %d maptrack frames (%d max)\n",
>> + rd->domain_id, gt->gt_version,
>> + nr_grant_frames(gt), gt->max_grant_frames,
>> + nr_maptrack_frames(gt), gt->max_maptrack_frames);
>
> Various %u instances again, and Dom%d please. Also you put this
> after the table header, corrupting intended output.
The position where the domain header is printed didn't change. It is
just unconditional now and contains some more information.
>
>> @@ -3782,12 +3796,7 @@ static void gnttab_usage_print(struct domain *rd)
>> status = status_entry(gt, ref);
>> }
>>
>> -if ( first )
>> -{
>> -printk("grant-table for remote domain:%5d (v%d)\n",
>> - rd->domain_id, gt->gt_version);
>> -first = 0;
>> -}
>> +first = 0;
>
> Is it useful to print the per-table information when there are no
> entries at all for a domain? I think it would be better to move
> what you add as well as the table header into the if() that you
> delete.
Hmm, I think the per-domain limits are valuable even without any
grant entries being present. In the end I don't expect lots of domains
without any grant entry other than dom0, as the default entries for
xenstore and console are being created by the tools already. And having
dom0 information especially for the maptrack entries will be
interesting.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 4/8] xen: make grant resource limits per domain
>>> On 11.09.17 at 12:40, wrote:
> On 08/09/17 17:44, Jan Beulich wrote:
> On 08.09.17 at 08:56, wrote:
>>> @@ -3755,6 +3763,12 @@ static void gnttab_usage_print(struct domain *rd)
>>>
>>> grant_read_lock(gt);
>>>
>>> +printk("grant-table for remote domain:%5d (v%d)\n"
>>> + " %d frames (%d max), %d maptrack frames (%d max)\n",
>>> + rd->domain_id, gt->gt_version,
>>> + nr_grant_frames(gt), gt->max_grant_frames,
>>> + nr_maptrack_frames(gt), gt->max_maptrack_frames);
>>
>> Various %u instances again, and Dom%d please. Also you put this
>> after the table header, corrupting intended output.
>
> The position where the domain header is printed didn't change. It is
> just unconditional now and contains some more information.
Oh, indeed, it's sort of malformed even before your change.
>>> @@ -3782,12 +3796,7 @@ static void gnttab_usage_print(struct domain *rd)
>>> status = status_entry(gt, ref);
>>> }
>>>
>>> -if ( first )
>>> -{
>>> -printk("grant-table for remote domain:%5d (v%d)\n",
>>> - rd->domain_id, gt->gt_version);
>>> -first = 0;
>>> -}
>>> +first = 0;
>>
>> Is it useful to print the per-table information when there are no
>> entries at all for a domain? I think it would be better to move
>> what you add as well as the table header into the if() that you
>> delete.
>
> Hmm, I think the per-domain limits are valuable even without any
> grant entries being present. In the end I don't expect lots of domains
> without any grant entry other than dom0, as the default entries for
> xenstore and console are being created by the tools already. And having
> dom0 information especially for the maptrack entries will be
> interesting.
Okay, valid point.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
