On Wed, Feb 1, 2023 at 1:25 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Wed, Dec 21, 2022, Ben Gardon wrote: > > @@ -978,9 +978,13 @@ static void slot_rmap_walk_next(struct slot_rmap_walk_iterator *iterator) > > slot_rmap_walk_okay(_iter_); \ > > slot_rmap_walk_next(_iter_)) > > > > -__always_inline bool kvm_handle_gfn_range(struct kvm *kvm, > > - struct kvm_gfn_range *range, > > - rmap_handler_t handler) > > +typedef bool (*rmap_handler_t)(struct kvm *kvm, struct kvm_rmap_head *rmap_head, > > + struct kvm_memory_slot *slot, gfn_t gfn, > > + int level, pte_t pte); > > + > > +static __always_inline bool > > +kvm_handle_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range, > > Don't split function returns/attributes from the function declaration. I don't > think the rule ended up getting officially documented and enforced, but Linus was > unequivocal when it came up[*], and I happen to agree with him :-) > > Actually, since I'm guessing you got the idea from existing code, can you fold > in the attached patches to purge the existing cases in mmu.c before those uglies > get moved around? Assuming you don't dislike the proposed rename, that is. > > [*] https://lore.kernel.org/mm-commits/CAHk-=wjS-Jg7sGMwUPpDsjv392nDOOs0CtUtVkp=S6Q7JzFJRw@xxxxxxxxxxxxxx Sounds good to me. Added the attached patches to the start of the series. I didn't love those weird splits in the function def. Happy to see them cleaned up too.