[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 2/2] x86/p2m: aid the compiler in folding p2m_is_...()
On Thu, Feb 1, 2024 at 10:15 PM Jan Beulich <jbeulich@xxxxxxxx> wrote: On 01.02.2024 14:32, George Dunlap wrote: That still doesn't work for me. I can't tell what git doesn't like. The error messages (the one you The raw email looks like this: ``` --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -379,7 +379,7 @@ struct page_info *p2m_get_page_from_gfn( return page; =20 /* Error path: not a suitable GFN at all */ - if ( !p2m_is_ram(*t) && !p2m_is_paging(*t) && !p2m_is_pod(*t) && + if ( !(p2m_is_ram(*t) | p2m_is_paging(*t) | p2m_is_pod(*t)) && !mem_sharing_is_fork(p2m->domain) ) return NULL; } ``` Note the "=20" at the beginning of the empty line. Why `patch` handles it but `git am` doesn't, who knows. I'm also not aware of there In the general case, I'm not going to review a patch without being able to see it in context; and it's not reasonable to expect reviewers to have specific contributor-specific scripts for doing so. If we run into this issue in the future, and you want my review, you may have to post a git tree somewhere, or attach the patch as an attachment or something. (Or you can try to figure out why `git am` isn't working and try to upstream a fix.) That said, in this case, context isn't really necessary to understand the change, so it won't be necessary. The logic of the change is obviously correct; but it definitely reduces the readability. I kind of feel like whether this sort of optimization is worth the benefits is more a general x86 maintainer policy decision. Maybe we can talk about it at the next maintainer's meeting I'll be at? -George
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |