[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V2 2/7] xen/grants: support allocating consecutive grants


  • To: Juergen Gross <jgross@xxxxxxxx>, Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
  • From: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
  • Date: Mon, 16 May 2022 12:00:05 -0400
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=MTc5lijZIFw35MDgY6cEo9JOLpZH94xs+SwgosuOXDg=; b=KmC2r2fgadE5G8Q47bRznEVtdQocxVGM9CMZY2NI7aMEpuI5mw24N1Vj/yKWWahlq1fhRQggo49hrL/Aw16GE10iU0ZI1gg6yYrAFGaNtefFZvYw6x7TBtpVz4O8FOKm8IZWEwYuC7bqQgwzC8fZoJoF1Ls0Hre3JeCKO8wjZnsUZTGRjdn66nCloyOKx7RyRYKR70QYzSmSKfofp034sj/7UBDKJhGNYqUTUxW3MXLRGfPRyilUn48kPvr2yRAlSqxTzxSNH9p4JhUQXKjUb5yUb0zFe5go798IS9yClEttoppBTXJ9dEd27CeDT4+AI55qsFTMJHL+yhg/ZDCEwA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gA16/x2Um1L13JnoNroZT7RXOWRgZVvEnUqXRtouSTfwv4nGp69HY2vdqW/iRULYI1U99mj1v+mJNqtpeGrEim5oQ4UEjQjCV9uem6zujqQCYWXlHgnqLZ7Nq/OKEwdy7Lxd+nxxEpmaflIUyx3wZo1JQITM1CGn5noWS/weEEohVZoeRqGHOB0xAGebyY9ddTRo30/IfGMFL5yTI6wEFyv9FYvgCf3u/VVF/nZqaN/BnsFgYeHRVXhhU/aXDeJcpKyE+d4ugh3tTNFgRCI5YZcMhy1pp4+qUsHcF1jVy0/sVg4SX9yVNiW/lRPYiOlhrYUHQfZVxai1b1DPrI5eSg==
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>, "Michael S. Tsirkin" <mst@xxxxxxxxxx>, Christoph Hellwig <hch@xxxxxxxxxxxxx>
  • Delivery-date: Mon, 16 May 2022 16:07:54 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>



On 5/16/22 1:59 AM, Juergen Gross wrote:
On 14.05.22 04:34, Boris Ostrovsky wrote:


On 5/13/22 1:33 AM, Juergen Gross wrote:
On 12.05.22 22:01, Boris Ostrovsky wrote:

On 5/7/22 2:19 PM, Oleksandr Tyshchenko wrote:

+/* Rebuilds the free grant list and tries to find count consecutive entries. */
+static int get_free_seq(unsigned int count)
+{
+    int ret = -ENOSPC;
+    unsigned int from, to;
+    grant_ref_t *last;
+
+    gnttab_free_tail_ptr = &gnttab_free_head;
+    last = &gnttab_free_head;
+
+    for (from = find_first_bit(gnttab_free_bitmap, gnttab_size);
+         from < gnttab_size;
+         from = find_next_bit(gnttab_free_bitmap, gnttab_size, to + 1)) {
+        to = find_next_zero_bit(gnttab_free_bitmap, gnttab_size,
+                    from + 1);
+        if (ret < 0 && to - from >= count) {
+            ret = from;
+            bitmap_clear(gnttab_free_bitmap, ret, count);
+            from += count;
+            gnttab_free_count -= count;


IIUIC we can have multiple passes over this, meaning that the gnttab_free_count 
may be decremented more than once. Is that intentional?

After the first pass decrementing gnttab_free_cnt, ret will no
longer be less than zero, so this can be hit only once.

Oh, yes, of course.




+            if (from == to)
+                continue;
+        }
+
+        while (from < to) {
+            *last = from;
+            last = __gnttab_entry(from);
+            gnttab_last_free = from;
+            from++;
+        }


I have been looking at this loop and I can't understand what it is doing ;-( 
Can you enlighten me?

It is recreating the free list in order to have it properly sorted.
This is needed to make sure that the free tail has the maximum
possible size (you can take the tail off the list without having
to worry about breaking the linked list because of references into
the tail).


So let's say we have the (one-dimensional) table of length 13

idx    ..    2    3  ...  10  11  12

grant       12   11        2  -1   3


and gnttab_free_head is 10. I.e. the free list is 2, 12, 3, 11.

You meant 10, 2, 12, 3, 11, I guess?


What will this look like after the 2 iterations of the outer loop?

idx    ..    2    3  ...  10  11  12

grant        3   10       11  12  -1

with gnttab_free_head being 2, i.e the free list is now 2, 3, 10, 11, 12.



OK, thanks, that helped. I couldn't link the free chunks in my head


With the error handling in gnttab_init() fixed

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>



-boris



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.