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.