RE: [PATCH] hv_balloon: Update the balloon driver to use the SBRM API

[Michael Kelley (LINUX)]

From: Mitchell Levy <levymitchell0@xxxxxxxxx> Sent: Thursday, August 3, 2023 4:11 PM
> 
> On Wed, Aug 2, 2023 at 10:47 AM Michael Kelley (LINUX)
> <mikelley@xxxxxxxxxxxxx> wrote:
> >
> > From: Mitchell Levy via B4 Relay <devnull+levymitchell0.gmail.com@xxxxxxxxxx> Sent: Tuesday, July 25, 2023 5:24 PM
> > >
> > > This patch is intended as a proof-of-concept for the new SBRM
> > > machinery[1]. For some brief background, the idea behind SBRM is using
> > > the __cleanup__ attribute to automatically unlock locks (or otherwise
> > > release resources) when they go out of scope, similar to C++ style RAII.
> > > This promises some benefits such as making code simpler (particularly
> > > where you have lots of goto fail; type constructs) as well as reducing
> > > the surface area for certain kinds of bugs.
> > >
> > > The changes in this patch should not result in any difference in how the
> > > code actually runs (i.e., it's purely an exercise in this new syntax
> > > sugar). In one instance SBRM was not appropriate, so I left that part
> > > alone, but all other locking/unlocking is handled automatically in this
> > > patch.
> > >
> > > Link: https://lore.kernel.org/all/20230626125726.GU4253@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/  [1]
> >
> > I haven't previously seen the "[1]" footnote-style identifier used with the
> > Link: tag.  Usually the "[1]" goes at the beginning of the line with the
> > additional information, but that conflicts with the Link: tag.  Maybe I'm
> > wrong, but you might either omit the footnote-style identifier, or the Link:
> > tag, instead of trying to use them together.
> 
> Will be sure to fix this (along with the other formatting issues
> raised by you and Boqun) in a v2.
> 
> >
> > Separately, have you built a kernel for ARM64 with these changes in
> > place?  The Hyper-V balloon driver is used on both x86 and ARM64
> > architectures.  There's nothing obviously architecture specific here,
> > but given that SBRM is new, it might be wise to verify that all is good
> > when building and running on ARM64.
> 
> I have built the kernel and confirmed that it's bootable on ARM64. I
> also disassembled the hv_balloon.o output by clang and GCC and
> compared the result to the disassembly of the pre-patch version. As
> far as I can tell, all the changes should be non-functional (some
> register renaming and flipping comparison instructions, etc.), but I
> don't believe I can thoroughly test at the moment as memory hot-add is
> disabled on ARM64.
> 

Excellent.  Thanks for indulging me and doing the basic verification
on ARM64.  You are right about memory hot-add not being used on
ARM64.  If I recall correctly, only the memory pressure reporting is
active on ARM64.

Michael