linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
@ 2025-03-12  6:15 Huan Yang
  2025-03-12  6:18 ` Christoph Hellwig
  0 siblings, 1 reply; 23+ messages in thread
From: Huan Yang @ 2025-03-12  6:15 UTC (permalink / raw)
  To: Andrew Morton, Uladzislau Rezki, Ryan Roberts, Zi Yan,
	Lorenzo Stoakes, Mike Rapoport (IBM),
	linux-mm, linux-kernel
  Cc: opensource.kernel, Christoph Hellwig, Huan Yang, Bingbu Cao

When invoke vmap_pfns, it call vmap_pfn_apply to set pfn into pte.
It check pfn is valid, if true then warn and return.

This is a mischeck, actually we need set a valid pfn into pte, not an
invalid pfn.

This patch fix it.

Signed-off-by: Huan Yang <link@vivo.com>
Reported-by: Bingbu Cao <bingbu.cao@linux.intel.com>
Closes: https://lore.kernel.org/dri-devel/eb7e0137-3508-4287-98c4-816c5fd98e10@vivo.com/T/#mbda4f64a3532b32e061f4e8763bc8e307bea3ca8
Fixes: b3f78e749865 ("mm: vmalloc must set pte via arch code")

---
 mm/vmalloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 044af7088359..ebadb8ceeba7 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3488,7 +3488,7 @@ static int vmap_pfn_apply(pte_t *pte, unsigned long addr, void *private)
 	unsigned long pfn = data->pfns[data->idx];
 	pte_t ptent;
 
-	if (WARN_ON_ONCE(pfn_valid(pfn)))
+	if (WARN_ON_ONCE(!pfn_valid(pfn)))
 		return -EINVAL;
 
 	ptent = pte_mkspecial(pfn_pte(pfn, data->prot));
-- 
2.39.0



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-12  6:15 [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns Huan Yang
@ 2025-03-12  6:18 ` Christoph Hellwig
  2025-03-17  2:12   ` Huan Yang
  0 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2025-03-12  6:18 UTC (permalink / raw)
  To: Huan Yang
  Cc: Andrew Morton, Uladzislau Rezki, Ryan Roberts, Zi Yan,
	Lorenzo Stoakes, Mike Rapoport (IBM),
	linux-mm, linux-kernel, opensource.kernel, Christoph Hellwig,
	Bingbu Cao

On Wed, Mar 12, 2025 at 02:15:12PM +0800, Huan Yang wrote:
> When invoke vmap_pfns, it call vmap_pfn_apply to set pfn into pte.
> It check pfn is valid, if true then warn and return.
> 
> This is a mischeck, actually we need set a valid pfn into pte, not an
> invalid pfn.

As just discussed this is wrong.  vmap_pfn is for mapping non-page
PFNs and the check is what enforces that.  What is the point of having
that detailed discussion if you just send the broken patch anyway with
a commit log not even acknowledging the facts?



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-12  6:18 ` Christoph Hellwig
@ 2025-03-17  2:12   ` Huan Yang
  2025-03-17  5:29     ` Bingbu Cao
  0 siblings, 1 reply; 23+ messages in thread
From: Huan Yang @ 2025-03-17  2:12 UTC (permalink / raw)
  To: hch
  Cc: akpm, bingbu.cao, link, linux-kernel, linux-mm, lorenzo.stoakes,
	opensource.kernel, rppt, ryan.roberts, urezki, ziy

HI Christoph,

Thanks for your reply, and I'm sorry for my late reply. Your response
didn't appear in my email client; I only saw it on the website.:(

>> On Wed, Mar 12, 2025 at 02:15:12PM +0800, Huan Yang wrote:
>> When invoke vmap_pfns, it call vmap_pfn_apply to set pfn into pte. >> It check pfn is valid, if true then warn and return. >> >> This is 
a mischeck, actually we need set a valid pfn into pte, not an >> invalid 
pfn. >
> As just discussed this is wrong.  vmap_pfn is for mapping non-page
Thank you for your explanation. I now understand that the design of vmap_pfn
is indeed intentional. It's design to do this.
> PFNs and the check is what enforces that.  What is the point of having
> that detailed discussion if you just send the broken patch anyway with
> a commit log not even acknowledging the facts?
Sorry for that.

We now have a new use case where, in udmabuf, memory is passed via memfd and can
be either shmem or hugetlb.
When the memory is hugetlb and HVO is enabled, the tail page's struct is no longer
reliable because it has been freed. Can't use vmap.
Therefore, when making modifications, I recorded the pfn of the folio base pfn + offset and called vmap_pfns.
And, these pfns are valid. So rejected by vmap_pfns.

Can we just remove pfn_valid check in vmap_pfns, so make it suit for both of they?
If you agree, I wanna send a new patch.

Thank you,
Huan Yang



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-17  2:12   ` Huan Yang
@ 2025-03-17  5:29     ` Bingbu Cao
  2025-03-17  5:53       ` Christoph Hellwig
  2025-03-17  6:26       ` Huan Yang
  0 siblings, 2 replies; 23+ messages in thread
From: Bingbu Cao @ 2025-03-17  5:29 UTC (permalink / raw)
  To: Huan Yang, hch
  Cc: akpm, linux-kernel, linux-mm, lorenzo.stoakes, opensource.kernel,
	rppt, ryan.roberts, urezki, ziy


On 3/17/25 10:12 AM, Huan Yang wrote:
> HI Christoph,
> 
> Thanks for your reply, and I'm sorry for my late reply. Your response
> didn't appear in my email client; I only saw it on the website.:(
> 
>>> On Wed, Mar 12, 2025 at 02:15:12PM +0800, Huan Yang wrote:
>>> When invoke vmap_pfns, it call vmap_pfn_apply to set pfn into pte. >> It check pfn is valid, if true then warn and return. >> >> This is 
> a mischeck, actually we need set a valid pfn into pte, not an >> invalid pfn. >
>> As just discussed this is wrong.  vmap_pfn is for mapping non-page
> Thank you for your explanation. I now understand that the design of vmap_pfn
> is indeed intentional. It's design to do this.
>> PFNs and the check is what enforces that.  What is the point of having
>> that detailed discussion if you just send the broken patch anyway with
>> a commit log not even acknowledging the facts?
> Sorry for that.
> 
> We now have a new use case where, in udmabuf, memory is passed via memfd and can
> be either shmem or hugetlb.
> When the memory is hugetlb and HVO is enabled, the tail page's struct is no longer
> reliable because it has been freed. Can't use vmap.
> Therefore, when making modifications, I recorded the pfn of the folio base pfn + offset and called vmap_pfns.
> And, these pfns are valid. So rejected by vmap_pfns.
> 
> Can we just remove pfn_valid check in vmap_pfns, so make it suit for both of they?
> If you agree, I wanna send a new patch.

Huan,

Why not update udmabuf to make it work with both vmap_pfns() and
vmap()? As only the udmabuf knows it is actually working on?

I don't think it's a good idea to hack the common API, the WARN_ON()
is really a mandatory check, and current case is a good example.

> 
> Thank you,
> Huan Yang
> 

-- 
Best regards,
Bingbu Cao


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-17  5:29     ` Bingbu Cao
@ 2025-03-17  5:53       ` Christoph Hellwig
  2025-03-17  7:42         ` Huan Yang
  2025-03-17  6:26       ` Huan Yang
  1 sibling, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2025-03-17  5:53 UTC (permalink / raw)
  To: Bingbu Cao
  Cc: Huan Yang, hch, akpm, linux-kernel, linux-mm, lorenzo.stoakes,
	opensource.kernel, rppt, ryan.roberts, urezki, ziy

On Mon, Mar 17, 2025 at 01:29:05PM +0800, Bingbu Cao wrote:
> Why not update udmabuf to make it work with both vmap_pfns() and
> vmap()? As only the udmabuf knows it is actually working on?
> 
> I don't think it's a good idea to hack the common API, the WARN_ON()
> is really a mandatory check, and current case is a good example.

What non-page backed memory does udmabuf try to work on, and more
importantly how does it actually work on them given that the normal
DMA APIs require page backed memory.  Or is this just made it up
and it doesn't work at all given that it also tries to dma map
to the fake miscdevice struct device which can't work for most
cases?

Mapping non-page memory is difficult and without having coherent theory
of what non-page memory you are mapping and being very careful you
are extremely unlikely to get it right.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-17  5:29     ` Bingbu Cao
  2025-03-17  5:53       ` Christoph Hellwig
@ 2025-03-17  6:26       ` Huan Yang
  1 sibling, 0 replies; 23+ messages in thread
From: Huan Yang @ 2025-03-17  6:26 UTC (permalink / raw)
  To: Bingbu Cao, hch
  Cc: akpm, linux-kernel, linux-mm, lorenzo.stoakes, opensource.kernel,
	rppt, ryan.roberts, urezki, ziy

Hi Bingbu

在 2025/3/17 13:29, Bingbu Cao 写道:
> On 3/17/25 10:12 AM, Huan Yang wrote:
>> HI Christoph,
>>
>> Thanks for your reply, and I'm sorry for my late reply. Your response
>> didn't appear in my email client; I only saw it on the website.:(
>>
>>>> On Wed, Mar 12, 2025 at 02:15:12PM +0800, Huan Yang wrote:
>>>> When invoke vmap_pfns, it call vmap_pfn_apply to set pfn into pte. >> It check pfn is valid, if true then warn and return. >> >> This is
>> a mischeck, actually we need set a valid pfn into pte, not an >> invalid pfn. >
>>> As just discussed this is wrong.  vmap_pfn is for mapping non-page
>> Thank you for your explanation. I now understand that the design of vmap_pfn
>> is indeed intentional. It's design to do this.
>>> PFNs and the check is what enforces that.  What is the point of having
>>> that detailed discussion if you just send the broken patch anyway with
>>> a commit log not even acknowledging the facts?
>> Sorry for that.
>>
>> We now have a new use case where, in udmabuf, memory is passed via memfd and can
>> be either shmem or hugetlb.
>> When the memory is hugetlb and HVO is enabled, the tail page's struct is no longer
>> reliable because it has been freed. Can't use vmap.
>> Therefore, when making modifications, I recorded the pfn of the folio base pfn + offset and called vmap_pfns.
>> And, these pfns are valid. So rejected by vmap_pfns.
>>
>> Can we just remove pfn_valid check in vmap_pfns, so make it suit for both of they?
>> If you agree, I wanna send a new patch.
> Huan,
>
> Why not update udmabuf to make it work with both vmap_pfns() and
> vmap()? As only the udmabuf knows it is actually working on?

You mean, If udmabuf invoke vmap if it's normal page-based folio range, 
else invoke vmap_pfns

if it's in HVO based?

udmabuf can contained a rane in folio and offset, what if it contains 
folio's head(with page struct) and

remain tail(without page struct, freed by HVO).

I think there are no suitable way to map it into vmalloc area.:)

Or else, just block hugetlb's folio mapped into vmalloc area? Which I 
don't think it's a good way.

>
> I don't think it's a good idea to hack the common API, the WARN_ON()

You mean common, but I think vmalloc can provide a more common API that 
not care if page it's

exist, if provide pfn, just map? :)

Or else, document it that vmap_pfn just do not welcome page based pfn 
map?(Just IMO)

Thanks,

Huan Yang

> is really a mandatory check, and current case is a good example.
>
>> Thank you,
>> Huan Yang
>>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-17  5:53       ` Christoph Hellwig
@ 2025-03-17  7:42         ` Huan Yang
  2025-03-18  6:48           ` Christoph Hellwig
  0 siblings, 1 reply; 23+ messages in thread
From: Huan Yang @ 2025-03-17  7:42 UTC (permalink / raw)
  To: hch
  Cc: akpm, bingbu.cao, link, linux-kernel, linux-mm, lorenzo.stoakes,
	opensource.kernel, rppt, ryan.roberts, urezki, ziy

HI Christoph

> On Mon, Mar 17, 2025 at 01:29:05PM +0800, Bingbu Cao wrote:
>> Why not update udmabuf to make it work with both vmap_pfns() and >> vmap()? As only the udmabuf knows it is actually working on? >> >> 
I don't think it's a good idea to hack the common API, the WARN_ON() >> 
is really a mandatory check, and current case is a good example.
> What non-page backed memory does udmabuf try to work on, and more
It's HUGETLB which enabled VMEMMAP_OPTIMIZE, all tail page's struct will
ref to head page struct, and then release tailed page struct.
> importantly how does it actually work on them given that the normal
> DMA APIs require page backed memory.  Or is this just made it up
udmabuf's sg_table ref only folio+offset, no any page struct ref.
So, any DMA APIs just use folio based.
It pin each folio given by memfd&offset.(memfd can be shmem or hugetlb).
So, shmem memfd can get page struct, hugetlb's may can't.
> and it doesn't work at all given that it also tries to dma map
> to the fake miscdevice struct device which can't work for most
> cases?
This implement map_dma_buf&mmap&vmap&begin/end_cpu_access.
It's simple implement.
>
> Mapping non-page memory is difficult and without having coherent theory
> of what non-page memory you are mapping and being very careful you
> are extremely unlikely to get it right.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-17  7:42         ` Huan Yang
@ 2025-03-18  6:48           ` Christoph Hellwig
  2025-03-18  8:20             ` Huan Yang
  0 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2025-03-18  6:48 UTC (permalink / raw)
  To: Huan Yang
  Cc: hch, akpm, bingbu.cao, linux-kernel, linux-mm, lorenzo.stoakes,
	opensource.kernel, rppt, ryan.roberts, urezki, ziy

So you want a folio based version of vmap.  Please work on that instead
of doing crazy hacks using vmap_pfns which is just for memory not
historically backed by pages and now folios.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-18  6:48           ` Christoph Hellwig
@ 2025-03-18  8:20             ` Huan Yang
  2025-03-18  8:33               ` Christoph Hellwig
  0 siblings, 1 reply; 23+ messages in thread
From: Huan Yang @ 2025-03-18  8:20 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: akpm, bingbu.cao, linux-kernel, linux-mm, lorenzo.stoakes,
	opensource.kernel, rppt, ryan.roberts, urezki, ziy,
	vivek.kasireddy

HI Christoph

在 2025/3/18 14:48, Christoph Hellwig 写道:
> So you want a folio based version of vmap.  Please work on that instead

The folio-based vmap implementation you mentioned may indeed be necessary, but it has limited

direct relevance to the specific issue I'm currently addressing.

> of doing crazy hacks using vmap_pfns which is just for memory not

I mentioned that if we use HVO-based hugetlb, we cannot access the page structures corresponding to

the physical memory that needs to be mapped.

This prevents us from properly invoking vmap, which is why we have turned to using vmap_pfn instead.

Even if a folio-based vmap is implemented, it still cannot handle mapping multiple folio ranges of physical

memory to vmalloc regions. A range of folio is important, it maybe an offset in memfd, no need entire folio.

So, I still consider vmap_pfn to be the optimal solution for this specific scenario. :)

> historically backed by pages and now folios.

So by HVO, it also not backed by pages, only contains folio head, each tail pfn's page struct go away.

Do I misunderstood this part? Please correct me.

Thank you

Huan Yang




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-18  8:20             ` Huan Yang
@ 2025-03-18  8:33               ` Christoph Hellwig
  2025-03-18  8:39                 ` Huan Yang
  2025-03-24  2:57                 ` Matthew Wilcox
  0 siblings, 2 replies; 23+ messages in thread
From: Christoph Hellwig @ 2025-03-18  8:33 UTC (permalink / raw)
  To: Huan Yang
  Cc: Christoph Hellwig, akpm, bingbu.cao, linux-kernel, linux-mm,
	lorenzo.stoakes, opensource.kernel, rppt, ryan.roberts, urezki,
	ziy, vivek.kasireddy

On Tue, Mar 18, 2025 at 04:20:17PM +0800, Huan Yang wrote:
> This prevents us from properly invoking vmap, which is why we have turned to using vmap_pfn instead.
>
> Even if a folio-based vmap is implemented, it still cannot handle mapping multiple folio ranges of physical
>
> memory to vmalloc regions. A range of folio is important, it maybe an offset in memfd, no need entire folio.
>
> So, I still consider vmap_pfn to be the optimal solution for this specific scenario. :)

No, vmap_pfn is entirely for memory not backed by pages or folios,
i.e. PCIe BARs and similar memory.  This must not be mixed with proper
folio backed memory.

So you'll need a vmap for folios to support this use case.

>
>> historically backed by pages and now folios.
>
> So by HVO, it also not backed by pages, only contains folio head, each tail pfn's page struct go away.

And a fully folios based vmap solves that problem.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-18  8:33               ` Christoph Hellwig
@ 2025-03-18  8:39                 ` Huan Yang
  2025-03-18  8:44                   ` Christoph Hellwig
  2025-03-24  2:57                 ` Matthew Wilcox
  1 sibling, 1 reply; 23+ messages in thread
From: Huan Yang @ 2025-03-18  8:39 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: akpm, bingbu.cao, linux-kernel, linux-mm, lorenzo.stoakes,
	opensource.kernel, rppt, ryan.roberts, urezki, ziy,
	vivek.kasireddy

HI Christoph

在 2025/3/18 16:33, Christoph Hellwig 写道:
> [You don't often get email from hch@lst.de. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Tue, Mar 18, 2025 at 04:20:17PM +0800, Huan Yang wrote:
>> This prevents us from properly invoking vmap, which is why we have turned to using vmap_pfn instead.
>>
>> Even if a folio-based vmap is implemented, it still cannot handle mapping multiple folio ranges of physical
>>
>> memory to vmalloc regions. A range of folio is important, it maybe an offset in memfd, no need entire folio.
>>
>> So, I still consider vmap_pfn to be the optimal solution for this specific scenario. :)
> No, vmap_pfn is entirely for memory not backed by pages or folios,
> i.e. PCIe BARs and similar memory.  This must not be mixed with proper
> folio backed memory.
OK, I learn it more.
>
> So you'll need a vmap for folios to support this use case.
May can't
>
>>> historically backed by pages and now folios.
>> So by HVO, it also not backed by pages, only contains folio head, each tail pfn's page struct go away.
> And a fully folios based vmap solves that problem.

A folio may be 2MB or more large 1GB, what if we only need a little, 1M or 512MB, can vmap based on folio can solve it?

Normally, can offer 4k-page based array map it. But consider HVO, can't. That's why wanto base on pfn.

Thank you

Huan Yang

>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-18  8:39                 ` Huan Yang
@ 2025-03-18  8:44                   ` Christoph Hellwig
  2025-03-18  8:50                     ` Huan Yang
                                       ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Christoph Hellwig @ 2025-03-18  8:44 UTC (permalink / raw)
  To: Huan Yang
  Cc: Christoph Hellwig, akpm, bingbu.cao, linux-kernel, linux-mm,
	lorenzo.stoakes, opensource.kernel, rppt, ryan.roberts, urezki,
	ziy, vivek.kasireddy

On Tue, Mar 18, 2025 at 04:39:40PM +0800, Huan Yang wrote:
> A folio may be 2MB or more large 1GB, what if we only need a little, 1M or 512MB, can vmap based on folio can solve it?

Then you only map part of it by passing a length argument.  Note that
in general when you have these large folios you also don't have highmem,
so if you only map one of them, or part of one of them you don't actually
need vmap at all and can just use folio-address..

> Normally, can offer 4k-page based array map it. But consider HVO, can't. That's why wanto base on pfn.

Well, for any large folio using this 4k based page interface is
actually highly inefficient.  So let's fix that.  And my loop in
willy as Mr. Folio while you're at it.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-18  8:44                   ` Christoph Hellwig
@ 2025-03-18  8:50                     ` Huan Yang
       [not found]                     ` <20250319050359.3484-1-hdanton@sina.com>
  2025-03-19  9:09                     ` Gao Xiang
  2 siblings, 0 replies; 23+ messages in thread
From: Huan Yang @ 2025-03-18  8:50 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: akpm, bingbu.cao, linux-kernel, linux-mm, lorenzo.stoakes,
	opensource.kernel, rppt, ryan.roberts, urezki, ziy,
	vivek.kasireddy

HI Christoph

在 2025/3/18 16:44, Christoph Hellwig 写道:
> [You don't often get email from hch@lst.de. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Tue, Mar 18, 2025 at 04:39:40PM +0800, Huan Yang wrote:
>> A folio may be 2MB or more large 1GB, what if we only need a little, 1M or 512MB, can vmap based on folio can solve it?
> Then you only map part of it by passing a length argument.  Note that
> in general when you have these large folios you also don't have highmem,
> so if you only map one of them, or part of one of them you don't actually
> need vmap at all and can just use folio-address..

The situation remains complex. We are not only referring to this single folio, but it may be composed

of multiple folios and start with a small piece. For example:

Folio 1 2M - use 1M

Folio 2 2M - use 2M

....

Combine these folios into memory used by udmabuf. So, directly virtual memory useless. :)

>
>> Normally, can offer 4k-page based array map it. But consider HVO, can't. That's why wanto base on pfn.
> Well, for any large folio using this 4k based page interface is
> actually highly inefficient.  So let's fix that.  And my loop in
Yes, indeed. You mentioned good to entire folio array mapped into vmalloc, I guest here are many user like it.
> willy as Mr. Folio while you're at it.
I'd like to research it, but not too fast. :)
>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
       [not found]                     ` <20250319050359.3484-1-hdanton@sina.com>
@ 2025-03-19  8:08                       ` Christoph Hellwig
       [not found]                       ` <20250319112651.3502-1-hdanton@sina.com>
  1 sibling, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2025-03-19  8:08 UTC (permalink / raw)
  To: Hillf Danton
  Cc: Huan Yang, Christoph Hellwig, bingbu.cao, linux-kernel, linux-mm,
	opensource.kernel, urezki, vivek.kasireddy

On Wed, Mar 19, 2025 at 01:03:55PM +0800, Hillf Danton wrote:
> A quick fix is to add a vmap_pfn variant to walk around the pfn
> check in question, like the following diff (just for idea show).

No.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-18  8:44                   ` Christoph Hellwig
  2025-03-18  8:50                     ` Huan Yang
       [not found]                     ` <20250319050359.3484-1-hdanton@sina.com>
@ 2025-03-19  9:09                     ` Gao Xiang
  2025-03-20  5:31                       ` Christoph Hellwig
  2 siblings, 1 reply; 23+ messages in thread
From: Gao Xiang @ 2025-03-19  9:09 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: akpm, bingbu.cao, linux-kernel, linux-mm, lorenzo.stoakes,
	opensource.kernel, rppt, ryan.roberts, urezki, ziy,
	vivek.kasireddy, Huan Yang



On 2025/3/18 16:44, Christoph Hellwig wrote:
> On Tue, Mar 18, 2025 at 04:39:40PM +0800, Huan Yang wrote:
>> A folio may be 2MB or more large 1GB, what if we only need a little, 1M or 512MB, can vmap based on folio can solve it?
> 
> Then you only map part of it by passing a length argument.  Note that
> in general when you have these large folios you also don't have highmem,
> so if you only map one of them, or part of one of them you don't actually
> need vmap at all and can just use folio-address..
> 
>> Normally, can offer 4k-page based array map it. But consider HVO, can't. That's why wanto base on pfn.
> 
> Well, for any large folio using this 4k based page interface is
> actually highly inefficient.  So let's fix that.  And my loop in
> willy as Mr. Folio while you're at it.

The minimum map unit is page size instead of variable-size folio.

For many cases, vmap (to combine many partial folios) is useful
(instead of split all folios to order-0 folios in advance) but
I agree page array may be inefficient.

So I don't think another folio vmap() version is better overall
anyway.

Thanks,
Gao Xiang



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-19  9:09                     ` Gao Xiang
@ 2025-03-20  5:31                       ` Christoph Hellwig
  2025-03-24  2:22                         ` Huan Yang
  0 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2025-03-20  5:31 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Christoph Hellwig, akpm, bingbu.cao, linux-kernel, linux-mm,
	lorenzo.stoakes, opensource.kernel, rppt, ryan.roberts, urezki,
	ziy, vivek.kasireddy, Huan Yang

On Wed, Mar 19, 2025 at 05:09:06PM +0800, Gao Xiang wrote:
> The minimum map unit is page size instead of variable-size folio.
>
> For many cases, vmap (to combine many partial folios) is useful
> (instead of split all folios to order-0 folios in advance) but
> I agree page array may be inefficient.
>
> So I don't think another folio vmap() version is better overall
> anyway.

Then just reject the mappings for now.  vmap/vm_map_ram isn't really
intended for mapping totally arbitrary scattered memory anyway.
As mentioned before udmabuf also has a grave bug in the dma mapping
part, so just marking it broken would be another very sensible
option.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
       [not found]                       ` <20250319112651.3502-1-hdanton@sina.com>
@ 2025-03-24  2:13                         ` Huan Yang
  2025-03-25  6:32                           ` Kasireddy, Vivek
  2025-03-27 13:40                           ` Matthew Wilcox
  0 siblings, 2 replies; 23+ messages in thread
From: Huan Yang @ 2025-03-24  2:13 UTC (permalink / raw)
  To: Hillf Danton, Christoph Hellwig
  Cc: bingbu.cao, linux-kernel, linux-mm, opensource.kernel, urezki,
	vivek.kasireddy

HI Hillf,

Thanks fo your suggestion.

在 2025/3/19 19:26, Hillf Danton 写道:
> On Wed, 19 Mar 2025 09:08:51 +0100 Christoph Hellwig wrote:
>> On Wed, Mar 19, 2025 at 01:03:55PM +0800, Hillf Danton wrote:
>>> A quick fix is to add a vmap_pfn variant to walk around the pfn
>>> check in question, like the following diff (just for idea show).
>> No.
>>
> Patient to see your fix, given no low hanging peach left for Mr Folio
> in case of HVO, (feel free to ignore, who is likely about 2.6-mile
> less tough than you could be).

I now believe there are two way to resolve the HVO folio can't vmap in udmabuf.

1. simple copy vmap_pfn code, don't bother common vmap_pfn, use by itself and remove pfn_valid check.

2. implement folio array based vmap(vmap_folios), which can given a range of each folio(offset, nr_pages), so can suit HVO folio's vmap.

1 is simple and can fast fix this issue, but may not too good. I need discuss with bot Christoph and udmabuf's maintainer.

2 is hard, but may worth to research, which I also will try to do. :)




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-20  5:31                       ` Christoph Hellwig
@ 2025-03-24  2:22                         ` Huan Yang
  0 siblings, 0 replies; 23+ messages in thread
From: Huan Yang @ 2025-03-24  2:22 UTC (permalink / raw)
  To: Christoph Hellwig, Gao Xiang
  Cc: akpm, bingbu.cao, linux-kernel, linux-mm, lorenzo.stoakes,
	opensource.kernel, rppt, ryan.roberts, urezki, ziy,
	vivek.kasireddy

HI Christoph

Sorry for the later reply.

在 2025/3/20 13:31, Christoph Hellwig 写道:
> As mentioned before udmabuf also has a grave bug in the dma mapping
> part, so just marking it broken would be another very sensible

Could you please provide a more detailed description of this?

I'm not the maintainer, but I see the udmabuf's begin patch mentioned as:

A driver to let userspace turn memfd regions into dma-bufs.

Use case:  Allows qemu create dmabufs for the vga framebuffer or
virtio-gpu ressources.  Then they can be passed around to display
those guest things on the host.  To spice client for classic full
framebuffer display, and hopefully some day to wayland server for
seamless guest window display.

https://patchwork.freedesktop.org/patch/246100/

Did it also mean to do it?

I'd like to learn your thinking

Thank you,

Huan Yang

> option.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-18  8:33               ` Christoph Hellwig
  2025-03-18  8:39                 ` Huan Yang
@ 2025-03-24  2:57                 ` Matthew Wilcox
  1 sibling, 0 replies; 23+ messages in thread
From: Matthew Wilcox @ 2025-03-24  2:57 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Huan Yang, akpm, bingbu.cao, linux-kernel, linux-mm,
	lorenzo.stoakes, opensource.kernel, rppt, ryan.roberts, urezki,
	ziy, vivek.kasireddy, Vishal Moola (Oracle)

On Tue, Mar 18, 2025 at 09:33:30AM +0100, Christoph Hellwig wrote:
> On Tue, Mar 18, 2025 at 04:20:17PM +0800, Huan Yang wrote:
> > This prevents us from properly invoking vmap, which is why we have turned to using vmap_pfn instead.
> >
> > Even if a folio-based vmap is implemented, it still cannot handle mapping multiple folio ranges of physical
> >
> > memory to vmalloc regions. A range of folio is important, it maybe an offset in memfd, no need entire folio.
> >
> > So, I still consider vmap_pfn to be the optimal solution for this specific scenario. :)
> 
> No, vmap_pfn is entirely for memory not backed by pages or folios,
> i.e. PCIe BARs and similar memory.  This must not be mixed with proper
> folio backed memory.
> 
> So you'll need a vmap for folios to support this use case.

https://lore.kernel.org/linux-mm/20250131001806.92349-1-vishal.moola@gmail.com/
seems like a good match for "we need to vmap a range from a file".

Vishal has some updates since this version, and I'm sure he can send
them out after LSFMM.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-24  2:13                         ` Huan Yang
@ 2025-03-25  6:32                           ` Kasireddy, Vivek
  2025-03-25  6:46                             ` Bingbu Cao
  2025-03-27 13:40                           ` Matthew Wilcox
  1 sibling, 1 reply; 23+ messages in thread
From: Kasireddy, Vivek @ 2025-03-25  6:32 UTC (permalink / raw)
  To: Huan Yang, Hillf Danton, Christoph Hellwig
  Cc: bingbu.cao, linux-kernel, linux-mm, opensource.kernel, urezki

Hi Huan,

> Subject: Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
> 
> HI Hillf,
> 
> Thanks fo your suggestion.
> 
> 在 2025/3/19 19:26, Hillf Danton 写道:
> > On Wed, 19 Mar 2025 09:08:51 +0100 Christoph Hellwig wrote:
> >> On Wed, Mar 19, 2025 at 01:03:55PM +0800, Hillf Danton wrote:
> >>> A quick fix is to add a vmap_pfn variant to walk around the pfn
> >>> check in question, like the following diff (just for idea show).
> >> No.
> >>
> > Patient to see your fix, given no low hanging peach left for Mr Folio
> > in case of HVO, (feel free to ignore, who is likely about 2.6-mile
> > less tough than you could be).
> 
> I now believe there are two way to resolve the HVO folio can't vmap in
> udmabuf.
> 
> 1. simple copy vmap_pfn code, don't bother common vmap_pfn, use by itself
> and remove pfn_valid check.
> 
> 2. implement folio array based vmap(vmap_folios), which can given a range of
> each folio(offset, nr_pages), so can suit HVO folio's vmap.
> 
> 1 is simple and can fast fix this issue, but may not too good. I need discuss with
> bot Christoph and udmabuf's maintainer.
> 
> 2 is hard, but may worth to research, which I also will try to do. :)
Another option is to just limit udmabuf's vmap() to only shmem folios but I guess
it may not work as expected if THP is also enabled. 
Bingbu, what exactly is your use-case? Could you please describe the scenario that
requires you to use udmabuf's vmap()?

Thanks,
Vivek

> 



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-25  6:32                           ` Kasireddy, Vivek
@ 2025-03-25  6:46                             ` Bingbu Cao
  0 siblings, 0 replies; 23+ messages in thread
From: Bingbu Cao @ 2025-03-25  6:46 UTC (permalink / raw)
  To: Kasireddy, Vivek, Huan Yang, Hillf Danton, Christoph Hellwig
  Cc: linux-kernel, linux-mm, opensource.kernel, urezki

Vivek,

On 3/25/25 2:32 PM, Kasireddy, Vivek wrote:
> Hi Huan,
> 
>> Subject: Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
>>
>> HI Hillf,
>>
>> Thanks fo your suggestion.
>>
>> 在 2025/3/19 19:26, Hillf Danton 写道:
>>> On Wed, 19 Mar 2025 09:08:51 +0100 Christoph Hellwig wrote:
>>>> On Wed, Mar 19, 2025 at 01:03:55PM +0800, Hillf Danton wrote:
>>>>> A quick fix is to add a vmap_pfn variant to walk around the pfn
>>>>> check in question, like the following diff (just for idea show).
>>>> No.
>>>>
>>> Patient to see your fix, given no low hanging peach left for Mr Folio
>>> in case of HVO, (feel free to ignore, who is likely about 2.6-mile
>>> less tough than you could be).
>>
>> I now believe there are two way to resolve the HVO folio can't vmap in
>> udmabuf.
>>
>> 1. simple copy vmap_pfn code, don't bother common vmap_pfn, use by itself
>> and remove pfn_valid check.
>>
>> 2. implement folio array based vmap(vmap_folios), which can given a range of
>> each folio(offset, nr_pages), so can suit HVO folio's vmap.
>>
>> 1 is simple and can fast fix this issue, but may not too good. I need discuss with
>> bot Christoph and udmabuf's maintainer.
>>
>> 2 is hard, but may worth to research, which I also will try to do. :)
> Another option is to just limit udmabuf's vmap() to only shmem folios but I guess
> it may not work as expected if THP is also enabled. 
> Bingbu, what exactly is your use-case? Could you please describe the scenario that
> requires you to use udmabuf's vmap()?

I guess my case is a very simple one.

Userspace create the udmabuf and mmap:

dmabuf_fd = ioctl(udma_fd, UDMABUF_CREATE, &create);
userptr = mmap(NULL, len, PROT_READ | PROT_WRITE,
	  MAP_SHARED, dmafd, 0);

And driver side call dma_buf_vmap() on the dmabuf to get a virtual
address.

> 
> Thanks,
> Vivek
> 
>>
> 

-- 
Best regards,
Bingbu Cao


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-24  2:13                         ` Huan Yang
  2025-03-25  6:32                           ` Kasireddy, Vivek
@ 2025-03-27 13:40                           ` Matthew Wilcox
  2025-03-28  6:13                             ` Huan Yang
  1 sibling, 1 reply; 23+ messages in thread
From: Matthew Wilcox @ 2025-03-27 13:40 UTC (permalink / raw)
  To: Huan Yang
  Cc: Hillf Danton, Christoph Hellwig, bingbu.cao, linux-kernel,
	linux-mm, opensource.kernel, urezki, vivek.kasireddy

On Mon, Mar 24, 2025 at 10:13:03AM +0800, Huan Yang wrote:
> HI Hillf,

Hillf is banned from the mailing lists.  I suuggest not replying to
any email that he sends to you privately.

https://lore.kernel.org/all/67cf7499597e9_1198729450@dwillia2-xfh.jf.intel.com.notmuch/


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns
  2025-03-27 13:40                           ` Matthew Wilcox
@ 2025-03-28  6:13                             ` Huan Yang
  0 siblings, 0 replies; 23+ messages in thread
From: Huan Yang @ 2025-03-28  6:13 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Christoph Hellwig, bingbu.cao, linux-kernel, linux-mm,
	opensource.kernel, urezki, vivek.kasireddy


在 2025/3/27 21:40, Matthew Wilcox 写道:
> On Mon, Mar 24, 2025 at 10:13:03AM +0800, Huan Yang wrote:
>> HI Hillf,
> Hillf is banned from the mailing lists.  I suuggest not replying to
> any email that he sends to you privately.
OK
>
> https://lore.kernel.org/all/67cf7499597e9_1198729450@dwillia2-xfh.jf.intel.com.notmuch/


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2025-03-28  6:13 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-12  6:15 [PATCH] mm/vmalloc: fix mischeck pfn valid in vmap_pfns Huan Yang
2025-03-12  6:18 ` Christoph Hellwig
2025-03-17  2:12   ` Huan Yang
2025-03-17  5:29     ` Bingbu Cao
2025-03-17  5:53       ` Christoph Hellwig
2025-03-17  7:42         ` Huan Yang
2025-03-18  6:48           ` Christoph Hellwig
2025-03-18  8:20             ` Huan Yang
2025-03-18  8:33               ` Christoph Hellwig
2025-03-18  8:39                 ` Huan Yang
2025-03-18  8:44                   ` Christoph Hellwig
2025-03-18  8:50                     ` Huan Yang
     [not found]                     ` <20250319050359.3484-1-hdanton@sina.com>
2025-03-19  8:08                       ` Christoph Hellwig
     [not found]                       ` <20250319112651.3502-1-hdanton@sina.com>
2025-03-24  2:13                         ` Huan Yang
2025-03-25  6:32                           ` Kasireddy, Vivek
2025-03-25  6:46                             ` Bingbu Cao
2025-03-27 13:40                           ` Matthew Wilcox
2025-03-28  6:13                             ` Huan Yang
2025-03-19  9:09                     ` Gao Xiang
2025-03-20  5:31                       ` Christoph Hellwig
2025-03-24  2:22                         ` Huan Yang
2025-03-24  2:57                 ` Matthew Wilcox
2025-03-17  6:26       ` Huan Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox