[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z+IPEeg7jkjrIB+8@nvidia.com>
Date: Mon, 24 Mar 2025 23:04:01 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Changyuan Lyu <changyuanl@...gle.com>
Cc: akpm@...ux-foundation.org, anthony.yznaga@...cle.com, arnd@...db.de,
ashish.kalra@....com, benh@...nel.crashing.org, bp@...en8.de,
catalin.marinas@....com, corbet@....net,
dave.hansen@...ux.intel.com, devicetree@...r.kernel.org,
dwmw2@...radead.org, ebiederm@...ssion.com, graf@...zon.com,
hpa@...or.com, jgowans@...zon.com, kexec@...ts.infradead.org,
krzk@...nel.org, linux-arm-kernel@...ts.infradead.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, luto@...nel.org, mark.rutland@....com,
mingo@...hat.com, pasha.tatashin@...een.com, pbonzini@...hat.com,
peterz@...radead.org, ptyadav@...zon.de, robh+dt@...nel.org,
robh@...nel.org, rostedt@...dmis.org, rppt@...nel.org,
saravanak@...gle.com, skinsburskii@...ux.microsoft.com,
tglx@...utronix.de, thomas.lendacky@....com, will@...nel.org,
x86@...nel.org
Subject: Re: [PATCH v5 09/16] kexec: enable KHO support for memory
preservation
On Sun, Mar 23, 2025 at 12:07:58PM -0700, Changyuan Lyu wrote:
> > > + down_read(&kho_out.tree_lock);
> > > + if (kho_out.fdt) {
> >
> > What is the lock and fdt test for?
>
> It is to avoid the competition between the following 2 operations,
> - converting the hashtables and mem traker to FDT,
> - adding new data to hashtable/mem tracker.
I think you should strive to prevent this by code construction at a
higher level.
Do not lock each preserve but lock entire object serializations, operations.
For instance if we do recursive FDT then you'd lock the call that
builds a single FDT page for a single object.
> In most cases we only need read lock. Different KHO users can adding
> data into their own subnodes in parallel.
read locks like this are still quite slow in parallel systems, there
is alot of slow cacheline bouncing as taking a read lock still has to
write to the lock memory.
> > What do you imagine this is used for? I'm not sure what value there is
> > in returning a void *? How does the caller "free" this?
>
> This function is also from Mike :)
>
> I suppose some KHO users may still
> preserve memory using memory ranges (instead of folio).
I don't know what that would be, but the folio scheme is all about
preserving memory from the page buddy allocator, I don't know what
this is for or how it would be used.
IMHO split this to its own patch and include it in the series that
would use it.
> I guess the caller can free the ranges by free_pages()?
The folios were not setup right, so no.. And if this is the case then
you'd just get the struct page and convert it to a void * with some
helper function, not implement a whole new function...
> > There should be yaml files just like in the normal DT case defining
> > all of this. This level of documentation and stability was one of the
> > selling reasons why FDT is being used here!
>
> YAML files were dropped because we think it may take a while for our
> schema to be near stable. So we start from some simple plain text. We
> can add some prop and node docs (that are considered stable at this point)
> back to YAML in the next version.
You need to do something to document what is going on here and show
the full schema with some explanation. It is hard to grasp the full
intention just from the C code.
Jason
Powered by blists - more mailing lists