Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Li Zefan
Balbir Singh wrote:
> Li Zefan wrote:
>> Balbir Singh wrote:
>>> YAMAMOTO Takashi wrote:
> Li Zefan wrote:
>> No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
>> be VM_BUG_ON(page).
>>
>> Signed-off-by: Li Zefan <[EMAIL PROTECTED]>
>> Acked-by: KAMEZAWA Hiroyuki <[EMAIL PROTECTED]>
> pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. 
> Not sure
> why we can't bug on pc.
 pc is dereferenced before this VM_BUG_ON.

 YAMAMOTO Takashi

>>> OK, so the VM_BUG_ON needs to move to an earlier location. Agreed.
>>>
>> No, 'pc' has been dereferenced in list_for_each_entry_safe_reverse().
>>
>>
>> #define list_for_each_entry_safe_reverse(pos, n, head, member)   
>> \
>>  for (pos = list_entry((head)->prev, typeof(*pos), member),  \
>>  n = list_entry(pos->member.prev, typeof(*pos), member); \
>>^^^
>>   >member != (head);\
>>   ^^^
>>   pos = n, n = list_entry(n->member.prev, typeof(*n), member))
>>
> 
> Hmm.. We used to have a for loop with !list_empty() as a termination condition
> and VM_BUG_ON(!pc) is a spill over. With the new loop, VM_BUG_ON(!pc) does not
> make sense.
> 
> 

I see, and I'll post a new patch to just remove it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Balbir Singh
Li Zefan wrote:
> Balbir Singh wrote:
>> YAMAMOTO Takashi wrote:
 Li Zefan wrote:
> No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
> be VM_BUG_ON(page).
>
> Signed-off-by: Li Zefan <[EMAIL PROTECTED]>
> Acked-by: KAMEZAWA Hiroyuki <[EMAIL PROTECTED]>
 pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. Not 
 sure
 why we can't bug on pc.
>>> pc is dereferenced before this VM_BUG_ON.
>>>
>>> YAMAMOTO Takashi
>>>
>> OK, so the VM_BUG_ON needs to move to an earlier location. Agreed.
>>
> 
> No, 'pc' has been dereferenced in list_for_each_entry_safe_reverse().
> 
> 
> #define list_for_each_entry_safe_reverse(pos, n, head, member)
> \
>   for (pos = list_entry((head)->prev, typeof(*pos), member),  \
>   n = list_entry(pos->member.prev, typeof(*pos), member); \
>^^^
>>member != (head);\
>   ^^^
>pos = n, n = list_entry(n->member.prev, typeof(*n), member))
> 

Hmm.. We used to have a for loop with !list_empty() as a termination condition
and VM_BUG_ON(!pc) is a spill over. With the new loop, VM_BUG_ON(!pc) does not
make sense.


-- 
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Li Zefan
Balbir Singh wrote:
> YAMAMOTO Takashi wrote:
>>> Li Zefan wrote:
 No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
 be VM_BUG_ON(page).

 Signed-off-by: Li Zefan <[EMAIL PROTECTED]>
 Acked-by: KAMEZAWA Hiroyuki <[EMAIL PROTECTED]>
>>> pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. Not 
>>> sure
>>> why we can't bug on pc.
>> pc is dereferenced before this VM_BUG_ON.
>>
>> YAMAMOTO Takashi
>>
> 
> OK, so the VM_BUG_ON needs to move to an earlier location. Agreed.
> 

No, 'pc' has been dereferenced in list_for_each_entry_safe_reverse().


#define list_for_each_entry_safe_reverse(pos, n, head, member)  \
for (pos = list_entry((head)->prev, typeof(*pos), member),  \
n = list_entry(pos->member.prev, typeof(*pos), member); \
   ^^^
 >member != (head);\
  ^^^
 pos = n, n = list_entry(n->member.prev, typeof(*n), member))

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread YAMAMOTO Takashi
> No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
> be VM_BUG_ON(page).
> 
> Signed-off-by: Li Zefan <[EMAIL PROTECTED]>
> Acked-by: KAMEZAWA Hiroyuki <[EMAIL PROTECTED]>
> ---
>  mm/memcontrol.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6bded84..c2959ee 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -534,7 +534,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long 
> nr_to_scan,
>   if (scan >= nr_to_scan)
>   break;
>   page = pc->page;
> - VM_BUG_ON(!pc);
> + VM_BUG_ON(!page);

can't page be NULL here if mem_cgroup_uncharge clears pc->page behind us?
ie. bug.

YAMAMOTO Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Balbir Singh
YAMAMOTO Takashi wrote:
>> Li Zefan wrote:
>>> No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
>>> be VM_BUG_ON(page).
>>>
>>> Signed-off-by: Li Zefan <[EMAIL PROTECTED]>
>>> Acked-by: KAMEZAWA Hiroyuki <[EMAIL PROTECTED]>
>> pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. Not 
>> sure
>> why we can't bug on pc.
> 
> pc is dereferenced before this VM_BUG_ON.
> 
> YAMAMOTO Takashi
> 

OK, so the VM_BUG_ON needs to move to an earlier location. Agreed.

-- 
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread YAMAMOTO Takashi
> Li Zefan wrote:
> > No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
> > be VM_BUG_ON(page).
> > 
> > Signed-off-by: Li Zefan <[EMAIL PROTECTED]>
> > Acked-by: KAMEZAWA Hiroyuki <[EMAIL PROTECTED]>
> 
> pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. Not 
> sure
> why we can't bug on pc.

pc is dereferenced before this VM_BUG_ON.

YAMAMOTO Takashi

> 
> 
> 
> -- 
>   Warm Regards,
>   Balbir Singh
>   Linux Technology Center
>   IBM, ISTL
> ___
> Containers mailing list
> [EMAIL PROTECTED]
> https://lists.linux-foundation.org/mailman/listinfo/containers
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Balbir Singh
Li Zefan wrote:
> No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
> be VM_BUG_ON(page).
> 
> Signed-off-by: Li Zefan <[EMAIL PROTECTED]>
> Acked-by: KAMEZAWA Hiroyuki <[EMAIL PROTECTED]>

pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. Not sure
why we can't bug on pc.



-- 
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Li Zefan
No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
be VM_BUG_ON(page).

Signed-off-by: Li Zefan <[EMAIL PROTECTED]>
Acked-by: KAMEZAWA Hiroyuki <[EMAIL PROTECTED]>
---
 mm/memcontrol.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6bded84..c2959ee 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -534,7 +534,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long 
nr_to_scan,
if (scan >= nr_to_scan)
break;
page = pc->page;
-   VM_BUG_ON(!pc);
+   VM_BUG_ON(!page);
 
if (unlikely(!PageLRU(page)))
continue;
-- 
1.5.4.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Li Zefan
No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
be VM_BUG_ON(page).

Signed-off-by: Li Zefan [EMAIL PROTECTED]
Acked-by: KAMEZAWA Hiroyuki [EMAIL PROTECTED]
---
 mm/memcontrol.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6bded84..c2959ee 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -534,7 +534,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long 
nr_to_scan,
if (scan = nr_to_scan)
break;
page = pc-page;
-   VM_BUG_ON(!pc);
+   VM_BUG_ON(!page);
 
if (unlikely(!PageLRU(page)))
continue;
-- 
1.5.4.rc3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Balbir Singh
Li Zefan wrote:
 No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
 be VM_BUG_ON(page).
 
 Signed-off-by: Li Zefan [EMAIL PROTECTED]
 Acked-by: KAMEZAWA Hiroyuki [EMAIL PROTECTED]

pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. Not sure
why we can't bug on pc.



-- 
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread YAMAMOTO Takashi
 Li Zefan wrote:
  No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
  be VM_BUG_ON(page).
  
  Signed-off-by: Li Zefan [EMAIL PROTECTED]
  Acked-by: KAMEZAWA Hiroyuki [EMAIL PROTECTED]
 
 pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. Not 
 sure
 why we can't bug on pc.

pc is dereferenced before this VM_BUG_ON.

YAMAMOTO Takashi

 
 
 
 -- 
   Warm Regards,
   Balbir Singh
   Linux Technology Center
   IBM, ISTL
 ___
 Containers mailing list
 [EMAIL PROTECTED]
 https://lists.linux-foundation.org/mailman/listinfo/containers
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Balbir Singh
YAMAMOTO Takashi wrote:
 Li Zefan wrote:
 No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
 be VM_BUG_ON(page).

 Signed-off-by: Li Zefan [EMAIL PROTECTED]
 Acked-by: KAMEZAWA Hiroyuki [EMAIL PROTECTED]
 pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. Not 
 sure
 why we can't bug on pc.
 
 pc is dereferenced before this VM_BUG_ON.
 
 YAMAMOTO Takashi
 

OK, so the VM_BUG_ON needs to move to an earlier location. Agreed.

-- 
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread YAMAMOTO Takashi
 No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
 be VM_BUG_ON(page).
 
 Signed-off-by: Li Zefan [EMAIL PROTECTED]
 Acked-by: KAMEZAWA Hiroyuki [EMAIL PROTECTED]
 ---
  mm/memcontrol.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/mm/memcontrol.c b/mm/memcontrol.c
 index 6bded84..c2959ee 100644
 --- a/mm/memcontrol.c
 +++ b/mm/memcontrol.c
 @@ -534,7 +534,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long 
 nr_to_scan,
   if (scan = nr_to_scan)
   break;
   page = pc-page;
 - VM_BUG_ON(!pc);
 + VM_BUG_ON(!page);

can't page be NULL here if mem_cgroup_uncharge clears pc-page behind us?
ie. bug.

YAMAMOTO Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Li Zefan
Balbir Singh wrote:
 YAMAMOTO Takashi wrote:
 Li Zefan wrote:
 No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
 be VM_BUG_ON(page).

 Signed-off-by: Li Zefan [EMAIL PROTECTED]
 Acked-by: KAMEZAWA Hiroyuki [EMAIL PROTECTED]
 pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. Not 
 sure
 why we can't bug on pc.
 pc is dereferenced before this VM_BUG_ON.

 YAMAMOTO Takashi

 
 OK, so the VM_BUG_ON needs to move to an earlier location. Agreed.
 

No, 'pc' has been dereferenced in list_for_each_entry_safe_reverse().


#define list_for_each_entry_safe_reverse(pos, n, head, member)  \
for (pos = list_entry((head)-prev, typeof(*pos), member),  \
n = list_entry(pos-member.prev, typeof(*pos), member); \
   ^^^
 pos-member != (head);\
  ^^^
 pos = n, n = list_entry(n-member.prev, typeof(*n), member))

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Balbir Singh
Li Zefan wrote:
 Balbir Singh wrote:
 YAMAMOTO Takashi wrote:
 Li Zefan wrote:
 No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
 be VM_BUG_ON(page).

 Signed-off-by: Li Zefan [EMAIL PROTECTED]
 Acked-by: KAMEZAWA Hiroyuki [EMAIL PROTECTED]
 pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. Not 
 sure
 why we can't bug on pc.
 pc is dereferenced before this VM_BUG_ON.

 YAMAMOTO Takashi

 OK, so the VM_BUG_ON needs to move to an earlier location. Agreed.

 
 No, 'pc' has been dereferenced in list_for_each_entry_safe_reverse().
 
 
 #define list_for_each_entry_safe_reverse(pos, n, head, member)
 \
   for (pos = list_entry((head)-prev, typeof(*pos), member),  \
   n = list_entry(pos-member.prev, typeof(*pos), member); \
^^^
pos-member != (head);\
   ^^^
pos = n, n = list_entry(n-member.prev, typeof(*n), member))
 

Hmm.. We used to have a for loop with !list_empty() as a termination condition
and VM_BUG_ON(!pc) is a spill over. With the new loop, VM_BUG_ON(!pc) does not
make sense.


-- 
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] memcgroup: fix typo in VM_BUG_ON()

2008-02-17 Thread Li Zefan
Balbir Singh wrote:
 Li Zefan wrote:
 Balbir Singh wrote:
 YAMAMOTO Takashi wrote:
 Li Zefan wrote:
 No need for VM_BUG_ON(pc), since 'pc' is the list entry. This should
 be VM_BUG_ON(page).

 Signed-off-by: Li Zefan [EMAIL PROTECTED]
 Acked-by: KAMEZAWA Hiroyuki [EMAIL PROTECTED]
 pc is of type page_cgroup and we use list_for_each_entry_safe_reverse. 
 Not sure
 why we can't bug on pc.
 pc is dereferenced before this VM_BUG_ON.

 YAMAMOTO Takashi

 OK, so the VM_BUG_ON needs to move to an earlier location. Agreed.

 No, 'pc' has been dereferenced in list_for_each_entry_safe_reverse().


 #define list_for_each_entry_safe_reverse(pos, n, head, member)   
 \
  for (pos = list_entry((head)-prev, typeof(*pos), member),  \
  n = list_entry(pos-member.prev, typeof(*pos), member); \
^^^
   pos-member != (head);\
   ^^^
   pos = n, n = list_entry(n-member.prev, typeof(*n), member))

 
 Hmm.. We used to have a for loop with !list_empty() as a termination condition
 and VM_BUG_ON(!pc) is a spill over. With the new loop, VM_BUG_ON(!pc) does not
 make sense.
 
 

I see, and I'll post a new patch to just remove it.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/