Re: [PATCH net-next v2 1/9] bnxt_en: add support for storing crash dump into host memory

On Mon, Aug 19, 2024 at 3:00 AM Przemek Kitszel
[off-list ref] wrote:

On 8/16/24 23:28, Michael Chan wrote:

quoted

From: Vikas Gupta <vikas.gupta@broadcom.com>
+static int bnxt_alloc_crash_dump_mem(struct bnxt *bp)
+{
+     u32 mem_size = 0;
+     int rc;
+
+     if (!(bp->fw_dbg_cap & DBG_QCAPS_RESP_FLAGS_CRASHDUMP_HOST_DDR))
+             return 0;
+
+     rc = bnxt_hwrm_get_dump_len(bp, BNXT_DUMP_CRASH, &mem_size);
+     if (rc)
+             return rc;
+
+     mem_size = round_up(mem_size, 4);
+
+     if (bp->fw_crash_mem && mem_size == bp->fw_crash_len)
+             return 0;
+
+     bnxt_free_crash_dump_mem(bp);

I would say it would be better to have the old buffer still allocated
in case of an allocation failure for the new one (like krealloc()).
Especially in case of shrink request.

Thanks for the review.  The original author Vikas is no longer working
for our group.  The FW crash dump memory is not just one buffer, but a
series of pages.  To resize these pages, we'll have to add a lot of
new code especially to cover the case that the page level can change.
A change in the memory size is rare.  It only happens when FW has
reset and the new running version requires a different crash dump
length.  So freeing it all and allocating again is a lot easier.  I'll
take a closer look to see if some small optimizations can be done
here.