Re: [RFC PATCH 03/31] lmb: make the lmb reservations persistent

[Heinrich Schuchardt]

On 1/1/70 01:00, Ilias Apalodimas wrote:

Hi Sughosh

[...]

  #define LMB_ALLOC_ANYWHERE    0

+#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
+struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS];
+struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
+#endif
+
+struct lmb lmb = {
+#if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
+       .memory.max = CONFIG_LMB_MAX_REGIONS,
+       .reserved.max = CONFIG_LMB_MAX_REGIONS,
+#else
+       .memory.max = CONFIG_LMB_MEMORY_REGIONS,
+       .reserved.max = CONFIG_LMB_RESERVED_REGIONS,
+       .memory.region = memory_regions,
+       .reserved.region = reserved_regions,

This is probably a good opportunity to look into why
CONFIG_LMB_MEMORY_REGIONS was introduced.  Since we are moving towards
static allocations, do we still need it? Or allocating the size dynamically
covers all our cases.

Up to now we used static arrays for saving memory allocations in LMB:

include/lmb.h:67:
struct lmb_property region[CONFIG_LMB_MAX_REGIONS];

As the EFI sub-system can produce any number of non-coalescable memory
regions we should use a linked list instead.

The fields memory.max and reserved.max seem to be unused except for
test/lib/lmb.c.

lib/lmb.c:136:  lmb->memory.max = CONFIG_LMB_MAX_REGIONS;
lib/lmb.c:139:  lmb->memory.max = CONFIG_LMB_MEMORY_REGIONS;
test/lib/lmb.c:689:     ut_asserteq(lmb.memory.max, CONFIG_LMB_MAX_REGIONS);

lib/lmb.c:137:  lmb->reserved.max = CONFIG_LMB_MAX_REGIONS;
lib/lmb.c:140:  lmb->reserved.max = CONFIG_LMB_RESERVED_REGIONS;
test/lib/lmb.c:691:     ut_asserteq(lmb.reserved.max,
CONFIG_LMB_MAX_REGIONS);

Best regards

Heinrich

+#endif
+       .memory.cnt = 0,
+       .reserved.cnt = 0,
+};
+
  static void lmb_dump_region(struct lmb_region *rgn, char *name)
  {
        unsigned long long base, size, end;
@@ -42,8 +61,8 @@ static void lmb_dump_region(struct lmb_region *rgn, char 
*name)
  void lmb_dump_all_force(struct lmb *lmb)
  {
        printf("lmb_dump_all:\n");
-       lmb_dump_region(&lmb->memory, "memory");
-       lmb_dump_region(&lmb->reserved, "reserved");
+       lmb_dump_region(&lmb.memory, "memory");
+       lmb_dump_region(&lmb.reserved, "reserved");
  }

[...]


Thanks
/Ilias