linux-kernel - Re: [PATCH v3] vfio/type1: optimize vfio_pin_pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250521033440.72577-1-lizhe.67@bytedance.com>
Date: Wed, 21 May 2025 11:34:40 +0800
From: lizhe.67@...edance.com
To: alex.williamson@...hat.com
Cc: david@...hat.com,
	kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	lizhe.67@...edance.com,
	muchun.song@...ux.dev,
	peterx@...hat.com
Subject: Re: [PATCH v3] vfio/type1: optimize vfio_pin_pages_remote() for huge folio

On Tue, 20 May 2025 08:07:19 -0600, alex.williamson@...hat.com wrote:

>> Before this patch:
>> funcgraph_entry:      # 33813.703 us |  vfio_pin_map_dma();
>> 
>> After this patch:
>> funcgraph_entry:      # 15635.055 us |  vfio_pin_map_dma();
>
>It looks like we're using the same numbers since the initial
>implementation, have these results changed?

Before the release of each version of the patch, I have conducted
performance test, and the results have consistently been in
close proximity to this value. Consequently, I decided there was
no need to update. I will include the latest test results in the
next version.

>> Signed-off-by: Li Zhe <lizhe.67@...edance.com>
>> Signed-off-by: Alex Williamson <alex.williamson@...hat.com>
>
>Appreciate the credit, this should probably be Co-developed-by: though.
>In general a sign-off is something that needs to be explicitly given.

Thank you for the reminder. I will correct this error in the next
version.

>> +			/*
>> +			 * Note: The current nr_pages does not achieve the optimal
>> +			 * performance in scenarios where folio_nr_pages() exceeds
>> +			 * batch->capacity. It is anticipated that future enhancements
>> +			 * will address this limitation.
>> +			 */
>> +			nr_pages = min((long)batch->size, folio_nr_pages(folio) -
>> +						folio_page_idx(folio, batch->pages[batch->offset]));
>
>We should use min_t() here, otherwise it looks good to me.

Thank you once again for your review! I will correct this error in
the next version.

By the way, using min_t() also resolved the build error
reported by the kernel test robot[1].

[1]: https://lore.kernel.org/all/202505210701.WY7sKXwU-lkp@intel.com/

Thanks,
Zhe