From: lizhe.67@bytedance.com
To: dan.carpenter@linaro.org
Cc: alex.williamson@redhat.com, david@redhat.com, jgg@ziepe.ca,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
lizhe.67@bytedance.com, lkp@intel.com,
oe-kbuild-all@lists.linux.dev, oe-kbuild@lists.linux.dev,
peterx@redhat.com
Subject: Re: [PATCH 3/4] vfio/type1: introduce a new member has_rsvd for struct vfio_dma
Date: Wed, 2 Jul 2025 11:47:20 +0800 [thread overview]
Message-ID: <20250702034720.53574-1-lizhe.67@bytedance.com> (raw)
In-Reply-To: <c209cfd6-05b1-4491-91ce-c672414d718c@suswa.mountain>
On Tue, 1 Jul 2025 18:13:48 +0300, dan.carpenter@linaro.org wrote:
> New smatch warnings:
> drivers/vfio/vfio_iommu_type1.c:788 vfio_pin_pages_remote() error: uninitialized symbol 'rsvd'.
>
> Old smatch warnings:
> drivers/vfio/vfio_iommu_type1.c:2376 vfio_iommu_type1_attach_group() warn: '&group->next' not removed from list
>
> vim +/rsvd +788 drivers/vfio/vfio_iommu_type1.c
>
> 8f0d5bb95f763c Kirti Wankhede 2016-11-17 684 static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
> 0635559233434a Alex Williamson 2025-02-18 685 unsigned long npage, unsigned long *pfn_base,
> 4b6c33b3229678 Daniel Jordan 2021-02-19 686 unsigned long limit, struct vfio_batch *batch)
> 73fa0d10d077d9 Alex Williamson 2012-07-31 687 {
> 4d83de6da265cd Daniel Jordan 2021-02-19 688 unsigned long pfn;
> 4d83de6da265cd Daniel Jordan 2021-02-19 689 struct mm_struct *mm = current->mm;
> 6c38c055cc4c0a Alex Williamson 2016-12-30 690 long ret, pinned = 0, lock_acct = 0;
> 89c29def6b0101 Alex Williamson 2018-06-02 691 bool rsvd;
> a54eb55045ae9b Kirti Wankhede 2016-11-17 692 dma_addr_t iova = vaddr - dma->vaddr + dma->iova;
> 166fd7d94afdac Alex Williamson 2013-06-21 693
> 6c38c055cc4c0a Alex Williamson 2016-12-30 694 /* This code path is only user initiated */
> 4d83de6da265cd Daniel Jordan 2021-02-19 695 if (!mm)
> 166fd7d94afdac Alex Williamson 2013-06-21 696 return -ENODEV;
> 73fa0d10d077d9 Alex Williamson 2012-07-31 697
> 4d83de6da265cd Daniel Jordan 2021-02-19 698 if (batch->size) {
> 4d83de6da265cd Daniel Jordan 2021-02-19 699 /* Leftover pages in batch from an earlier call. */
> 4d83de6da265cd Daniel Jordan 2021-02-19 700 *pfn_base = page_to_pfn(batch->pages[batch->offset]);
> 4d83de6da265cd Daniel Jordan 2021-02-19 701 pfn = *pfn_base;
> 89c29def6b0101 Alex Williamson 2018-06-02 702 rsvd = is_invalid_reserved_pfn(*pfn_base);
When batch->size is not zero, we initialize rsvd here.
> 4d83de6da265cd Daniel Jordan 2021-02-19 703 } else {
> 4d83de6da265cd Daniel Jordan 2021-02-19 704 *pfn_base = 0;
When the value of batch->size is zero, we set the value of *pfn_base
to zero and do not initialize rsvd for the time being.
> 5c6c2b21ecc9ad Alex Williamson 2013-06-21 705 }
> 5c6c2b21ecc9ad Alex Williamson 2013-06-21 706
> eb996eec783c1e Alex Williamson 2025-02-18 707 if (unlikely(disable_hugepages))
> eb996eec783c1e Alex Williamson 2025-02-18 708 npage = 1;
> eb996eec783c1e Alex Williamson 2025-02-18 709
> 4d83de6da265cd Daniel Jordan 2021-02-19 710 while (npage) {
> 4d83de6da265cd Daniel Jordan 2021-02-19 711 if (!batch->size) {
> 4d83de6da265cd Daniel Jordan 2021-02-19 712 /* Empty batch, so refill it. */
> eb996eec783c1e Alex Williamson 2025-02-18 713 ret = vaddr_get_pfns(mm, vaddr, npage, dma->prot,
> eb996eec783c1e Alex Williamson 2025-02-18 714 &pfn, batch);
> be16c1fd99f41a Daniel Jordan 2021-02-19 715 if (ret < 0)
> 4d83de6da265cd Daniel Jordan 2021-02-19 716 goto unpin_out;
> 166fd7d94afdac Alex Williamson 2013-06-21 717
> 4d83de6da265cd Daniel Jordan 2021-02-19 718 if (!*pfn_base) {
> 4d83de6da265cd Daniel Jordan 2021-02-19 719 *pfn_base = pfn;
> 4d83de6da265cd Daniel Jordan 2021-02-19 720 rsvd = is_invalid_reserved_pfn(*pfn_base);
Therefore, for the first loop, when batch->size is zero, *pfn_base must
be zero, which will then lead to the initialization of rsvd.
> 4d83de6da265cd Daniel Jordan 2021-02-19 721 }
>
> If "*pfn_base" is true then "rsvd" is uninitialized.
>
> eb996eec783c1e Alex Williamson 2025-02-18 722
> eb996eec783c1e Alex Williamson 2025-02-18 723 /* Handle pfnmap */
> eb996eec783c1e Alex Williamson 2025-02-18 724 if (!batch->size) {
> eb996eec783c1e Alex Williamson 2025-02-18 725 if (pfn != *pfn_base + pinned || !rsvd)
> eb996eec783c1e Alex Williamson 2025-02-18 726 goto out;
>
> goto out;
>
> eb996eec783c1e Alex Williamson 2025-02-18 727
> eb996eec783c1e Alex Williamson 2025-02-18 728 pinned += ret;
> eb996eec783c1e Alex Williamson 2025-02-18 729 npage -= ret;
> eb996eec783c1e Alex Williamson 2025-02-18 730 vaddr += (PAGE_SIZE * ret);
> eb996eec783c1e Alex Williamson 2025-02-18 731 iova += (PAGE_SIZE * ret);
> eb996eec783c1e Alex Williamson 2025-02-18 732 continue;
> eb996eec783c1e Alex Williamson 2025-02-18 733 }
> 166fd7d94afdac Alex Williamson 2013-06-21 734 }
> 166fd7d94afdac Alex Williamson 2013-06-21 735
> 4d83de6da265cd Daniel Jordan 2021-02-19 736 /*
> eb996eec783c1e Alex Williamson 2025-02-18 737 * pfn is preset for the first iteration of this inner loop
> eb996eec783c1e Alex Williamson 2025-02-18 738 * due to the fact that vaddr_get_pfns() needs to provide the
> eb996eec783c1e Alex Williamson 2025-02-18 739 * initial pfn for pfnmaps. Therefore to reduce redundancy,
> eb996eec783c1e Alex Williamson 2025-02-18 740 * the next pfn is fetched at the end of the loop.
> eb996eec783c1e Alex Williamson 2025-02-18 741 * A PageReserved() page could still qualify as page backed
> eb996eec783c1e Alex Williamson 2025-02-18 742 * and rsvd here, and therefore continues to use the batch.
> 4d83de6da265cd Daniel Jordan 2021-02-19 743 */
> 4d83de6da265cd Daniel Jordan 2021-02-19 744 while (true) {
> 6a2d9b72168041 Li Zhe 2025-06-30 745 long nr_pages, acct_pages = 0;
> 6a2d9b72168041 Li Zhe 2025-06-30 746
> 4d83de6da265cd Daniel Jordan 2021-02-19 747 if (pfn != *pfn_base + pinned ||
> 4d83de6da265cd Daniel Jordan 2021-02-19 748 rsvd != is_invalid_reserved_pfn(pfn))
> 4d83de6da265cd Daniel Jordan 2021-02-19 749 goto out;
> 4d83de6da265cd Daniel Jordan 2021-02-19 750
> 6a2d9b72168041 Li Zhe 2025-06-30 751 nr_pages = contig_pages(dma, batch, iova);
> 6a2d9b72168041 Li Zhe 2025-06-30 752 if (!rsvd) {
> 6a2d9b72168041 Li Zhe 2025-06-30 753 acct_pages = nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 754 acct_pages -= vpfn_pages(dma, iova, nr_pages);
> 6a2d9b72168041 Li Zhe 2025-06-30 755 }
> 6a2d9b72168041 Li Zhe 2025-06-30 756
> 4d83de6da265cd Daniel Jordan 2021-02-19 757 /*
> 4d83de6da265cd Daniel Jordan 2021-02-19 758 * Reserved pages aren't counted against the user,
> 4d83de6da265cd Daniel Jordan 2021-02-19 759 * externally pinned pages are already counted against
> 4d83de6da265cd Daniel Jordan 2021-02-19 760 * the user.
> 4d83de6da265cd Daniel Jordan 2021-02-19 761 */
> 6a2d9b72168041 Li Zhe 2025-06-30 762 if (acct_pages) {
> 48d8476b41eed6 Alex Williamson 2018-05-11 763 if (!dma->lock_cap &&
> 6a2d9b72168041 Li Zhe 2025-06-30 764 mm->locked_vm + lock_acct + acct_pages > limit) {
> 6c38c055cc4c0a Alex Williamson 2016-12-30 765 pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n",
> 6c38c055cc4c0a Alex Williamson 2016-12-30 766 __func__, limit << PAGE_SHIFT);
> 0cfef2b7410b64 Alex Williamson 2017-04-13 767 ret = -ENOMEM;
> 0cfef2b7410b64 Alex Williamson 2017-04-13 768 goto unpin_out;
> 166fd7d94afdac Alex Williamson 2013-06-21 769 }
> 6a2d9b72168041 Li Zhe 2025-06-30 770 lock_acct += acct_pages;
> a54eb55045ae9b Kirti Wankhede 2016-11-17 771 }
> 4d83de6da265cd Daniel Jordan 2021-02-19 772
> 6a2d9b72168041 Li Zhe 2025-06-30 773 pinned += nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 774 npage -= nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 775 vaddr += PAGE_SIZE * nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 776 iova += PAGE_SIZE * nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 777 batch->offset += nr_pages;
> 6a2d9b72168041 Li Zhe 2025-06-30 778 batch->size -= nr_pages;
> 4d83de6da265cd Daniel Jordan 2021-02-19 779
> 4d83de6da265cd Daniel Jordan 2021-02-19 780 if (!batch->size)
> 4d83de6da265cd Daniel Jordan 2021-02-19 781 break;
> 4d83de6da265cd Daniel Jordan 2021-02-19 782
> 4d83de6da265cd Daniel Jordan 2021-02-19 783 pfn = page_to_pfn(batch->pages[batch->offset]);
> 4d83de6da265cd Daniel Jordan 2021-02-19 784 }
> a54eb55045ae9b Kirti Wankhede 2016-11-17 785 }
> 166fd7d94afdac Alex Williamson 2013-06-21 786
> 6c38c055cc4c0a Alex Williamson 2016-12-30 787 out:
> 20448310d6b71d Li Zhe 2025-06-30 @788 dma->has_rsvd |= rsvd;
> ^^^^
In summary, it is likely to be a false alarm.
Please correct me if I am wrong.
Thanks,
Zhe
next prev parent reply other threads:[~2025-07-02 3:47 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-30 7:25 [PATCH 0/4] vfio/type1: optimize vfio_pin_pages_remote() and vfio_unpin_pages_remote() for large folio lizhe.67
2025-06-30 7:25 ` [PATCH 1/4] vfio/type1: optimize vfio_pin_pages_remote() for large folios lizhe.67
2025-06-30 7:25 ` [PATCH 2/4] vfio/type1: batch vfio_find_vpfn() in function vfio_unpin_pages_remote() lizhe.67
2025-07-02 18:27 ` Jason Gunthorpe
2025-07-03 4:18 ` lizhe.67
2025-07-03 12:27 ` Jason Gunthorpe
2025-07-04 2:20 ` lizhe.67
2025-06-30 7:25 ` [PATCH 3/4] vfio/type1: introduce a new member has_rsvd for struct vfio_dma lizhe.67
2025-07-01 15:13 ` Dan Carpenter
2025-07-02 3:47 ` lizhe.67 [this message]
2025-07-02 16:11 ` Dan Carpenter
2025-06-30 7:25 ` [PATCH 4/4] vfio/type1: optimize vfio_unpin_pages_remote() for large folio lizhe.67
2025-07-02 18:28 ` Jason Gunthorpe
2025-07-03 6:12 ` lizhe.67
2025-07-02 8:15 ` [PATCH 0/4] vfio/type1: optimize vfio_pin_pages_remote() and " David Hildenbrand
2025-07-02 9:38 ` lizhe.67
2025-07-02 9:57 ` David Hildenbrand
2025-07-02 12:47 ` Jason Gunthorpe
2025-07-03 4:04 ` lizhe.67
2025-07-03 3:54 ` lizhe.67
2025-07-03 11:06 ` David Hildenbrand
2025-07-03 11:12 ` Jason Gunthorpe
2025-07-03 11:35 ` lizhe.67
2025-07-03 11:34 ` lizhe.67
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250702034720.53574-1-lizhe.67@bytedance.com \
--to=lizhe.67@bytedance.com \
--cc=alex.williamson@redhat.com \
--cc=dan.carpenter@linaro.org \
--cc=david@redhat.com \
--cc=jgg@ziepe.ca \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@intel.com \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=oe-kbuild@lists.linux.dev \
--cc=peterx@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox