On Fri, Mar 17, 2023 at 12:00:30PM -0700, Anish Moorthy wrote: > On Fri, Mar 17, 2023 at 11:27 AM Oliver Upton <oliver.upton@xxxxxxxxx> wrote: > > > > + pfn = __gfn_to_pfn_memslot( > > > + memslot, gfn, exit_on_memory_fault, false, NULL, > > > + write_fault, &writable, NULL); > > > > As stated before [*], this google3-esque style does not match the kernel style > > guide. You may want to check if your work machine is setting up a G3-specific > > editor configuration behind your back. > > > > [*] https://lore.kernel.org/kvm/Y+0QRsZ4yWyUdpnc@xxxxxxxxxx/ > > If you're referring to the indentation, then that was definitely me. > I'll give the style guide another readthrough before I submit the next > version then, since checkpatch.pl doesn't seem to complain here. > > > > + if (exit_on_memory_fault && pfn == KVM_PFN_ERR_FAULT) { > > > > nit: I don't think the local is explicitly necessary. I still find this > > readable: > > The local was for keeping a consistent value between the two blocks of code here > > pfn = __gfn_to_pfn_memslot( > memslot, gfn, exit_on_memory_fault, false, NULL, > write_fault, &writable, NULL); > > if (exit_on_memory_fault && pfn == KVM_PFN_ERR_FAULT) { > // Set up vCPU exit and return 0 > } > > I wanted to avoid the possibility of causing an early > __gfn_to_pfn_memslot exit but then not populating the vCPU exit. Ignore me, I didn't see the other use of the local. -- Thanks, Oliver
