Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk()
@ 2020-01-15  6:09 Yang Shi
  2020-01-15 12:08 ` Li Xinhai
  2020-01-27 18:17 ` Qian Cai
  0 siblings, 2 replies; 10+ messages in thread
From: Yang Shi @ 2020-01-15  6:09 UTC (permalink / raw)
  To: akpm; +Cc: yang.shi, linux-mm, linux-kernel

The VM_BUG_ON() is already used by queue_pages_test_walk(), it sounds
better to dump more debug information by using VM_BUG_ON_VMA() to help
debugging.

Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
---
 mm/mempolicy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 067cf7d..801d45d 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -621,7 +621,7 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end,
 	unsigned long flags = qp->flags;
 
 	/* range check first */
-	VM_BUG_ON((vma->vm_start > start) || (vma->vm_end < end));
+	VM_BUG_ON_VMA((vma->vm_start > start) || (vma->vm_end < end), vma);
 
 	if (!qp->first) {
 		qp->first = vma;
-- 
1.8.3.1



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

* Re: [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk()
  2020-01-15  6:09 [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk() Yang Shi
@ 2020-01-15 12:08 ` Li Xinhai
  2020-01-15 17:27   ` Yang Shi
  2020-01-27 18:17 ` Qian Cai
  1 sibling, 1 reply; 10+ messages in thread
From: Li Xinhai @ 2020-01-15 12:08 UTC (permalink / raw)
  To: yang.shi, akpm; +Cc: yang.shi, linux-mm, linux-kernel

On 2020-01-15 at 14:09 Yang Shi wrote:
>The VM_BUG_ON() is already used by queue_pages_test_walk(), it sounds
>better to dump more debug information by using VM_BUG_ON_VMA() to help
>debugging.
>
>Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com> 

The .test_walk() is to be called from pagewalk with the rule that 'start'
and 'end' must within range of vma, in case the rule is broke, we detect
it. This is not quite relevant to a bug of particular vma. 

>---
> mm/mempolicy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>index 067cf7d..801d45d 100644
>--- a/mm/mempolicy.c
>+++ b/mm/mempolicy.c
>@@ -621,7 +621,7 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end,
> unsigned long flags = qp->flags;
>
> /* range check first */
>-	VM_BUG_ON((vma->vm_start > start) || (vma->vm_end < end));
>+	VM_BUG_ON_VMA((vma->vm_start > start) || (vma->vm_end < end), vma);
>
> if (!qp->first) {
> qp->first = vma;
>--
>1.8.3.1
>
>

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

* Re: [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk()
  2020-01-15 12:08 ` Li Xinhai
@ 2020-01-15 17:27   ` Yang Shi
  2020-01-16 15:52     ` Li Xinhai
  0 siblings, 1 reply; 10+ messages in thread
From: Yang Shi @ 2020-01-15 17:27 UTC (permalink / raw)
  To: Li Xinhai, akpm; +Cc: linux-mm, linux-kernel



On 1/15/20 4:08 AM, Li Xinhai wrote:
> On 2020-01-15 at 14:09 Yang Shi wrote:
>> The VM_BUG_ON() is already used by queue_pages_test_walk(), it sounds
>> better to dump more debug information by using VM_BUG_ON_VMA() to help
>> debugging.
>>
>> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
> The .test_walk() is to be called from pagewalk with the rule that 'start'
> and 'end' must within range of vma, in case the rule is broke, we detect
> it. This is not quite relevant to a bug of particular vma.

But when you run into VMA range check failure, isn't it helpful to dump 
the VMA range information to ease debugging? And, VM_BUG_ON is already 
used in the code, I'm supposed the users may prefer more debug 
information dumped for debug kernel.

>
>> ---
>> mm/mempolicy.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>> index 067cf7d..801d45d 100644
>> --- a/mm/mempolicy.c
>> +++ b/mm/mempolicy.c
>> @@ -621,7 +621,7 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end,
>> unsigned long flags = qp->flags;
>>
>> /* range check first */
>> -	VM_BUG_ON((vma->vm_start > start) || (vma->vm_end < end));
>> +	VM_BUG_ON_VMA((vma->vm_start > start) || (vma->vm_end < end), vma);
>>
>> if (!qp->first) {
>> qp->first = vma;
>> --
>> 1.8.3.1
>>
> >



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

* Re: [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk()
  2020-01-15 17:27   ` Yang Shi
@ 2020-01-16 15:52     ` Li Xinhai
  2020-01-27 16:59       ` Yang Shi
  0 siblings, 1 reply; 10+ messages in thread
From: Li Xinhai @ 2020-01-16 15:52 UTC (permalink / raw)
  To: yang.shi, akpm; +Cc: linux-mm, linux-kernel

On 2020-01-16 at 01:27 Yang Shi wrote:
>
>
>On 1/15/20 4:08 AM, Li Xinhai wrote:
>> On 2020-01-15 at 14:09 Yang Shi wrote:
>>> The VM_BUG_ON() is already used by queue_pages_test_walk(), it sounds
>>> better to dump more debug information by using VM_BUG_ON_VMA() to help
>>> debugging.
>>>
>>> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
>> The .test_walk() is to be called from pagewalk with the rule that 'start'
>> and 'end' must within range of vma, in case the rule is broke, we detect
>> it. This is not quite relevant to a bug of particular vma.
>
>But when you run into VMA range check failure, isn't it helpful to dump
>the VMA range information to ease debugging? And, VM_BUG_ON is already
>used in the code, I'm supposed the users may prefer more debug
>information dumped for debug kernel.
> 
Got your point, it is already used better put more information.
>>
>>> ---
>>> mm/mempolicy.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>>> index 067cf7d..801d45d 100644
>>> --- a/mm/mempolicy.c
>>> +++ b/mm/mempolicy.c
>>> @@ -621,7 +621,7 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end,
>>> unsigned long flags = qp->flags;
>>>
>>> /* range check first */
>>> -	VM_BUG_ON((vma->vm_start > start) || (vma->vm_end < end));
>>> +	VM_BUG_ON_VMA((vma->vm_start > start) || (vma->vm_end < end), vma);
>>>
>>> if (!qp->first) {
>>> qp->first = vma;
>>> --
>>> 1.8.3.1
>>>
>> >
>

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

* Re: [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk()
  2020-01-16 15:52     ` Li Xinhai
@ 2020-01-27 16:59       ` Yang Shi
  0 siblings, 0 replies; 10+ messages in thread
From: Yang Shi @ 2020-01-27 16:59 UTC (permalink / raw)
  To: Li Xinhai, akpm; +Cc: linux-mm, linux-kernel



On 1/16/20 7:52 AM, Li Xinhai wrote:
> On 2020-01-16 at 01:27 Yang Shi wrote:
>>
>> On 1/15/20 4:08 AM, Li Xinhai wrote:
>>> On 2020-01-15 at 14:09 Yang Shi wrote:
>>>> The VM_BUG_ON() is already used by queue_pages_test_walk(), it sounds
>>>> better to dump more debug information by using VM_BUG_ON_VMA() to help
>>>> debugging.
>>>>
>>>> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
>>> The .test_walk() is to be called from pagewalk with the rule that 'start'
>>> and 'end' must within range of vma, in case the rule is broke, we detect
>>> it. This is not quite relevant to a bug of particular vma.
>> But when you run into VMA range check failure, isn't it helpful to dump
>> the VMA range information to ease debugging? And, VM_BUG_ON is already
>> used in the code, I'm supposed the users may prefer more debug
>> information dumped for debug kernel.
>>
> Got your point, it is already used better put more information.

Hi Andrew,

Would you like to take this patch for v5.6 or v5.7? It looks Xinhai 
agrees with my point.

Thanks,
Yang

>>>> ---
>>>> mm/mempolicy.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>>>> index 067cf7d..801d45d 100644
>>>> --- a/mm/mempolicy.c
>>>> +++ b/mm/mempolicy.c
>>>> @@ -621,7 +621,7 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end,
>>>> unsigned long flags = qp->flags;
>>>>
>>>> /* range check first */
>>>> -	VM_BUG_ON((vma->vm_start > start) || (vma->vm_end < end));
>>>> +	VM_BUG_ON_VMA((vma->vm_start > start) || (vma->vm_end < end), vma);
>>>>
>>>> if (!qp->first) {
>>>> qp->first = vma;
>>>> --
>>>> 1.8.3.1
>>>>
>>>>
> >



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

* Re: [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk()
  2020-01-15  6:09 [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk() Yang Shi
  2020-01-15 12:08 ` Li Xinhai
@ 2020-01-27 18:17 ` Qian Cai
  2020-01-27 19:57   ` Yang Shi
  1 sibling, 1 reply; 10+ messages in thread
From: Qian Cai @ 2020-01-27 18:17 UTC (permalink / raw)
  To: Yang Shi; +Cc: akpm, linux-mm, linux-kernel



> On Jan 15, 2020, at 1:09 AM, Yang Shi <yang.shi@linux.alibaba.com> wrote:
> 
> The VM_BUG_ON() is already used by queue_pages_test_walk(), it sounds
> better to dump more debug information by using VM_BUG_ON_VMA() to help
> debugging.

What’s the problem this is trying to resolve? Was there an existing bug would trigger this?

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

* Re: [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk()
  2020-01-27 18:17 ` Qian Cai
@ 2020-01-27 19:57   ` Yang Shi
  2020-01-27 20:23     ` Qian Cai
  0 siblings, 1 reply; 10+ messages in thread
From: Yang Shi @ 2020-01-27 19:57 UTC (permalink / raw)
  To: Qian Cai; +Cc: akpm, linux-mm, linux-kernel



On 1/27/20 10:17 AM, Qian Cai wrote:
>
>> On Jan 15, 2020, at 1:09 AM, Yang Shi <yang.shi@linux.alibaba.com> wrote:
>>
>> The VM_BUG_ON() is already used by queue_pages_test_walk(), it sounds
>> better to dump more debug information by using VM_BUG_ON_VMA() to help
>> debugging.
> What’s the problem this is trying to resolve? Was there an existing bug would trigger this?

Dumping more information to help debugging. I don't run into related bug 
personally.



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

* Re: [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk()
  2020-01-27 19:57   ` Yang Shi
@ 2020-01-27 20:23     ` Qian Cai
  2020-02-14  5:26       ` Andrew Morton
  0 siblings, 1 reply; 10+ messages in thread
From: Qian Cai @ 2020-01-27 20:23 UTC (permalink / raw)
  To: Yang Shi; +Cc: akpm, linux-mm, linux-kernel



> On Jan 27, 2020, at 2:57 PM, Yang Shi <yang.shi@linux.alibaba.com> wrote:
> 
> Dumping more information to help debugging. I don't run into related bug personally.

This is a relatively weak justification for merging. If we are keeping accepting those mindless debugging patches, the workload will be unbearable for all.

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

* Re: [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk()
  2020-01-27 20:23     ` Qian Cai
@ 2020-02-14  5:26       ` Andrew Morton
  2020-02-14 12:24         ` Qian Cai
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2020-02-14  5:26 UTC (permalink / raw)
  To: Qian Cai; +Cc: Yang Shi, linux-mm, linux-kernel, Li Xinhai

On Mon, 27 Jan 2020 15:23:08 -0500 Qian Cai <cai@lca.pw> wrote:

> 
> 
> > On Jan 27, 2020, at 2:57 PM, Yang Shi <yang.shi@linux.alibaba.com> wrote:
> > 
> > Dumping more information to help debugging. I don't run into related bug personally.
> 
> This is a relatively weak justification for merging. If we are keeping accepting those mindless debugging patches, the workload will be unbearable for all.

I think it's OK.  If this ever triggers the kernel is dead, so the
volume of output isn't a problem.  And if it triggers, the more info
the better.



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

* Re: [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk()
  2020-02-14  5:26       ` Andrew Morton
@ 2020-02-14 12:24         ` Qian Cai
  0 siblings, 0 replies; 10+ messages in thread
From: Qian Cai @ 2020-02-14 12:24 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Yang Shi, linux-mm, linux-kernel, Li Xinhai



> On Feb 14, 2020, at 12:26 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> I think it's OK.  If this ever triggers the kernel is dead, so the
> volume of output isn't a problem.  And if it triggers, the more info
> the better.

Well, I could think of millions of ways to just add more info for those theoretical assertion places where we will eventually be running out of review bandwidth if people start to “abuse” it. Anyway, if maintainers are willing to take risks on that path, I’ll not complain.

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

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-15  6:09 [PATCH] mm: mempolicy: use VM_BUG_ON_VMA in queue_pages_test_walk() Yang Shi
2020-01-15 12:08 ` Li Xinhai
2020-01-15 17:27   ` Yang Shi
2020-01-16 15:52     ` Li Xinhai
2020-01-27 16:59       ` Yang Shi
2020-01-27 18:17 ` Qian Cai
2020-01-27 19:57   ` Yang Shi
2020-01-27 20:23     ` Qian Cai
2020-02-14  5:26       ` Andrew Morton
2020-02-14 12:24         ` Qian Cai

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git