[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [XEN PATCH v2 1/2] Check zone before merging adjacent blocks in heap
- To: Jan Beulich <jbeulich@xxxxxxxx>, Stewart Hildebrand <stewart.hildebrand@xxxxxxxxxxxxxxx>
- From: George Dunlap <george.dunlap@xxxxxxxxxx>
- Date: Tue, 4 Feb 2020 15:37:04 +0000
- Authentication-results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=george.dunlap@xxxxxxxxxx; spf=Pass smtp.mailfrom=George.Dunlap@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx
- Autocrypt: addr=george.dunlap@xxxxxxxxxx; prefer-encrypt=mutual; keydata= mQINBFPqG+MBEACwPYTQpHepyshcufo0dVmqxDo917iWPslB8lauFxVf4WZtGvQSsKStHJSj 92Qkxp4CH2DwudI8qpVbnWCXsZxodDWac9c3PordLwz5/XL41LevEoM3NWRm5TNgJ3ckPA+J K5OfSK04QtmwSHFP3G/SXDJpGs+oDJgASta2AOl9vPV+t3xG6xyfa2NMGn9wmEvvVMD44Z7R W3RhZPn/NEZ5gaJhIUMgTChGwwWDOX0YPY19vcy5fT4bTIxvoZsLOkLSGoZb/jHIzkAAznug Q7PPeZJ1kXpbW9EHHaUHiCD9C87dMyty0N3TmWfp0VvBCaw32yFtM9jUgB7UVneoZUMUKeHA fgIXhJ7I7JFmw3J0PjGLxCLHf2Q5JOD8jeEXpdxugqF7B/fWYYmyIgwKutiGZeoPhl9c/7RE Bf6f9Qv4AtQoJwtLw6+5pDXsTD5q/GwhPjt7ohF7aQZTMMHhZuS52/izKhDzIufl6uiqUBge 0lqG+/ViLKwCkxHDREuSUTtfjRc9/AoAt2V2HOfgKORSCjFC1eI0+8UMxlfdq2z1AAchinU0 eSkRpX2An3CPEjgGFmu2Je4a/R/Kd6nGU8AFaE8ta0oq5BSFDRYdcKchw4TSxetkG6iUtqOO ZFS7VAdF00eqFJNQpi6IUQryhnrOByw+zSobqlOPUO7XC5fjnwARAQABtCRHZW9yZ2UgVy4g RHVubGFwIDxkdW5sYXBnQHVtaWNoLmVkdT6JAlcEEwEKAEECGwMFCwkIBwMFFQoJCAsFFgID AQACHgECF4ACGQEWIQTXqBy2bTNXPzpOYFimNjwxBZC0bQUCXEowWQUJDCJ7dgAKCRCmNjwx BZC0beKvEACJ75YlJXd7TnNHgFyiCJkm/qPeoQ3sFGSDZuZh7SKcdt9+3V2bFEb0Mii1hQaz 3hRqZb8sYPHJrGP0ljK09k3wf8k3OuNxziLQBJyzvn7WNlE4wBEcy/Ejo9TVBdA4ph5D0YaZ nqdsPmxe/xlTFuSkgu4ep1v9dfVP1TQR0e+JIBa/Ss+cKC5intKm+8JxpOploAHuzaPu0L/X FapzsIXqgT9eIQeBEgO2hge6h9Jov3WeED/vh8kA7f8c6zQ/gs5E7VGALwsiLrhr0LZFcKcw kI3oCCrB/C/wyPZv789Ra8EXbeRSJmTjcnBwHRPjnjwQmetRDD1t+VyrkC6uujT5jmgOBzaj KCqZ8PcMAssOzdzQtKmjUQ2b3ICPs2X13xZ5M5/OVs1W3TG5gkvMh4YoHi4ilFnOk+v3/j7q 65FG6N0JLb94Ndi80HkIOQQ1XVGTyu6bUPaBg3rWK91Csp1682kD/dNVF3FKHrRLmSVtmEQR 5rK0+VGc/FmR6vd4haKGWIRuPxzg+pBR77avIZpU7C7+UXGuZ5CbHwIdY8LojJg2TuUdqaVj yxmEZLOA8rVHipCGrslRNthVbJrGN/pqtKjCClFZHIAYJQ9EGLHXLG9Pj76opfjHij3MpR3o pCGAh6KsCrfrsvjnpDwqSbngGyEVH030irSk4SwIqZ7FwA==
- Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Liu <wl@xxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Jeff Kubascik <jeff.kubascik@xxxxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, David Woodhouse <dwmw2@xxxxxxxxxxxxx>
- Delivery-date: Tue, 04 Feb 2020 15:37:12 +0000
- Ironport-sdr: NC4smseQLXRRluKuAYBntfjCEQRAtRZjoYc/+WpOyl7J1ZkGTbeuDZEku8p6QKwpDaj+wLc2P3 wZIIItqLC5XRMfVxVSC7PE2NAuRUvP0uwckBoF094JTsPjyBw7T+aTeoek0uaDMup7FYO1xUN/ s2oIpKZJ8RXAUGkBcgpl41iYY1gG02fajTEIo9TtPs9X3d9SUfYO4i8WttrBysLQQVtozDJ7gj OZZFIG0+l4mx9bY6Lp1D+TxzCvCT1tfpwOlKmMqlVWjVSly0qjblLfK1Iaa6CoU9u3Rl5/g9Jc Cng=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Openpgp: preference=signencrypt
On 2/4/20 3:22 PM, Jan Beulich wrote:
> On 04.02.2020 16:14, Stewart Hildebrand wrote:
>> From: Jeff Kubascik <jeff.kubascik@xxxxxxxxxxxxxxx>
>>
>> The Xen heap is split up into nodes and zones. Each node + zone is
>> managed as a separate pool of memory.
>>
>> When returning pages to the heap, free_heap_pages will check adjacent
>> blocks to see if they can be combined into a larger block. However, the
>> zone of the adjacent block is not checked. This results in blocks that
>> migrate from one zone to another.
>>
>> When a block migrates to the adjacent zone, the avail counters for the
>> old and new node + zone is not updated accordingly. The avail counter
>> is used when allocating pages to determine whether to skip over a zone.
>> With this behavior, it is possible for free pages to collect in a zone
>> with the avail counter smaller than the actual page count, resulting
>> in free pages that are not allocable.
>
> "When a block migrates" - fine. But is this situation possible to
> occur, without "xen/page_alloc: Keep away MFN 0 from the buddy
> allocator" reverted? If not, there's no bug, no need for a change,
> and even less so ...
>
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -1462,6 +1462,7 @@ static void free_heap_pages(
>> if ( !mfn_valid(page_to_mfn(predecessor)) ||
>> !page_state_is(predecessor, free) ||
>> (PFN_ORDER(predecessor) != order) ||
>> + (page_to_zone(predecessor) != zone) ||
>> (phys_to_nid(page_to_maddr(predecessor)) != node) )
>> break;
>>
>> @@ -1485,6 +1486,7 @@ static void free_heap_pages(
>> if ( !mfn_valid(page_to_mfn(successor)) ||
>> !page_state_is(successor, free) ||
>> (PFN_ORDER(successor) != order) ||
>> + (page_to_zone(successor) != zone) ||
>> (phys_to_nid(page_to_maddr(successor)) != node) )
>> break;
>
> ... for one that slows down many free operations, even if just
> slightly. IOW afaict either the change is not needed, or its
> description needs updating.
At very least it's more robust this way; the algorithm is also less
"magic". We just had a long discussion this morning trying to re-create
the logic for why "Remove MFN 0" was sufficient to prevent this issue,
and even then David wasn't sure it was correct at first.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
