[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] gnttab: simplify (replace) gnttab_set_frame_gfn()
- To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- From: Jan Beulich <jbeulich@xxxxxxxx>
- Date: Fri, 8 May 2026 08:17:05 +0200
- Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=google header.d=suse.com header.i="@suse.com" header.h="Content-Transfer-Encoding:In-Reply-To:Autocrypt:From:Content-Language:References:Cc:To:Subject:User-Agent:MIME-Version:Date:Message-ID"
- Autocrypt: addr=jbeulich@xxxxxxxx; keydata= xsDiBFk3nEQRBADAEaSw6zC/EJkiwGPXbWtPxl2xCdSoeepS07jW8UgcHNurfHvUzogEq5xk hu507c3BarVjyWCJOylMNR98Yd8VqD9UfmX0Hb8/BrA+Hl6/DB/eqGptrf4BSRwcZQM32aZK 7Pj2XbGWIUrZrd70x1eAP9QE3P79Y2oLrsCgbZJfEwCgvz9JjGmQqQkRiTVzlZVCJYcyGGsD /0tbFCzD2h20ahe8rC1gbb3K3qk+LpBtvjBu1RY9drYk0NymiGbJWZgab6t1jM7sk2vuf0Py O9Hf9XBmK0uE9IgMaiCpc32XV9oASz6UJebwkX+zF2jG5I1BfnO9g7KlotcA/v5ClMjgo6Gl MDY4HxoSRu3i1cqqSDtVlt+AOVBJBACrZcnHAUSuCXBPy0jOlBhxPqRWv6ND4c9PH1xjQ3NP nxJuMBS8rnNg22uyfAgmBKNLpLgAGVRMZGaGoJObGf72s6TeIqKJo/LtggAS9qAUiuKVnygo 3wjfkS9A3DRO+SpU7JqWdsveeIQyeyEJ/8PTowmSQLakF+3fote9ybzd880fSmFuIEJldWxp Y2ggPGpiZXVsaWNoQHN1c2UuY29tPsJgBBMRAgAgBQJZN5xEAhsDBgsJCAcDAgQVAggDBBYC AwECHgECF4AACgkQoDSui/t3IH4J+wCfQ5jHdEjCRHj23O/5ttg9r9OIruwAn3103WUITZee e7Sbg12UgcQ5lv7SzsFNBFk3nEQQCACCuTjCjFOUdi5Nm244F+78kLghRcin/awv+IrTcIWF hUpSs1Y91iQQ7KItirz5uwCPlwejSJDQJLIS+QtJHaXDXeV6NI0Uef1hP20+y8qydDiVkv6l IreXjTb7DvksRgJNvCkWtYnlS3mYvQ9NzS9PhyALWbXnH6sIJd2O9lKS1Mrfq+y0IXCP10eS FFGg+Av3IQeFatkJAyju0PPthyTqxSI4lZYuJVPknzgaeuJv/2NccrPvmeDg6Coe7ZIeQ8Yj t0ARxu2xytAkkLCel1Lz1WLmwLstV30g80nkgZf/wr+/BXJW/oIvRlonUkxv+IbBM3dX2OV8 AmRv1ySWPTP7AAMFB/9PQK/VtlNUJvg8GXj9ootzrteGfVZVVT4XBJkfwBcpC/XcPzldjv+3 HYudvpdNK3lLujXeA5fLOH+Z/G9WBc5pFVSMocI71I8bT8lIAzreg0WvkWg5V2WZsUMlnDL9 mpwIGFhlbM3gfDMs7MPMu8YQRFVdUvtSpaAs8OFfGQ0ia3LGZcjA6Ik2+xcqscEJzNH+qh8V m5jjp28yZgaqTaRbg3M/+MTbMpicpZuqF4rnB0AQD12/3BNWDR6bmh+EkYSMcEIpQmBM51qM EKYTQGybRCjpnKHGOxG0rfFY1085mBDZCH5Kx0cl0HVJuQKC+dV2ZY5AqjcKwAxpE75MLFkr wkkEGBECAAkFAlk3nEQCGwwACgkQoDSui/t3IH7nnwCfcJWUDUFKdCsBH/E5d+0ZnMQi+G0A nAuWpQkjM1ASeQwSHEeAWPgskBQL
- Cc: Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Teddy Astie <teddy.astie@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Rafal Wojtczuk <rafal.wojtczuk@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Fri, 08 May 2026 06:17:16 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 07.05.2026 19:49, Andrew Cooper wrote:
> On 07/05/2026 12:41 pm, Jan Beulich wrote:
>> It's not really doing anything for valid GFNs, which renders its one use
>> site pretty pointless. The other isn't so much about setting anything, but
>> rather about clearing. Rename the macro accordingly.
>>
>> The main point here, however, is about Rafal spotting the double
>> fetching of the GFN (first in gnttab_unpopulate_status_frames(), then
>> again in gnttab_set_frame_gfn()). Re-purpose the macro parameter to pass
>> in the already fetched GFN, while dropping the no longer used parameters.
>>
>> Suggested-by: Rafal Wojtczuk <rafal.wojtczuk@xxxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> Do we even need the hook anymore? It has been expanding the same for Arm
>> and x86.
>
> At this point, I'd say no.
>
> The macro has changed contents several times since it's introduction.
> The grant table macros especially demonstrate how poor the common/arch
> interfaces are.
Okay, unless I hear otherwise (pretty soon) I'll drop it altogether in v2.
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -1844,8 +1844,7 @@ gnttab_unpopulate_status_frames(struct d
>> {
>> int rc = gfn_eq(gfn, INVALID_GFN)
>> ? 0
>> - : gnttab_set_frame_gfn(gt, true, i, INVALID_GFN,
>> - page_to_mfn(pg));
>> + : gnttab_clear_frame_gfn(gt, gfn, page_to_mfn(pg));
>>
>
> This is just a more complex way of writing
>
> if ( !gfn_eq(gfn, INVALID_GFN) )
> rc = gnttab_clear_frame_gfn(gt, gfn, page_to_mfn(pg));
With up-front
int rc = 0;
yes. As in many other cases I'm trying to keep churn low when there's no
pretty much objectively better way of writing things. Personally I prefer
the "just an initializer" form (iirc it was also me who introduced the
above code in that shape).
Jan
|
