Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch "3abf380..." in next-20130607 tree
On 07/08/2013 02:43 PM, Chen Gang wrote:
> On 07/05/2013 12:06 PM, Chen Gang wrote:
>> On 06/19/2013 03:19 PM, Chen Gang wrote:
>>> On 06/19/2013 03:10 PM, Andrew Morton wrote:
On Wed, 19 Jun 2013 14:55:13 +0800 Chen Gang wrote:
>>
>> 'lru' may be used without initialized, so need regressing part of the
>> related patch.
>>
>> The related patch:
>> "3abf380 mm: remove lru parameter from __lru_cache_add and
>> lru_cache_add_lru"
>>
>> ...
>>
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -595,6 +595,7 @@ redo:
>> * unevictable page on [in]active list.
>> * We know how to handle that.
>> */
>> +lru = !!TestClearPageActive(page) +
>> page_lru_base_type(page);
>> lru_cache_add(page);
>> } else {
>> /*
That looks right. Why the heck didn't gcc-4.4.4 (at least) warn about it?
>>>
>>> Sorry I don't know either, I find it by reading code, this time.
>>>
>>> It is really necessary to continue analyzing why. In 2nd half of 2013, I
>>> have planned to make some patches outside kernel but related with kernel
>>> (e.g. LTP, gcc patches).
>>>
>>> This kind of issue is a good chance for me to start in 2nd half of 2013
>>> (start from next month).
>>>
>>> So if no others reply for it, I will start analyzing it in the next
>>> month, and plan to finish within a month (before 2013-07-31).
>>>
>>>
>>> Welcome additional suggestions or completions.
>>>
>>> Thanks.
>>>
I create a related bug for it in gcc, please reference:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57856
Next, I will communicate about it with gcc guys, if any member is
interested in it, please tracing it in gcc bugzilla (bug57856).
Now, truly end of this thread in kernel mailing list.
Thanks.
>
> Under the gcc which from the source code in svn. it still has this
> issue. I should communicate with gcc mailing list (or their bugzilla)
> for it.
>
> I got gcc source code from svn, "configure && make && make install".
>
> [root@gchenlinux linux-next]# which gcc
> /usr/local/bin/gcc
> [root@gchenlinux linux-next]# gcc -v
> Using built-in specs.
> COLLECT_GCC=gcc
> COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/x86_64-unknown-linux-gnu/4.9.0/lto-wrapper
> Target: x86_64-unknown-linux-gnu
> Configured with: ./configure
> Thread model: posix
> gcc version 4.9.0 20130704 (experimental) (GCC)
>
>
> I think, this thread is the end under kernel mailing list. ;-)
>
> Thanks.
>
>>
>> Under gcc 4.7.2 20120921 (Red Hat 4.7.2-2) also cause this issue.
>>
>> The root cause is:
>>
>> for putback_lur_page() in mm/vmscan.c for next-20130621 tree.
>> the compiler assumes "lru == LRU_UNEVICTABLE" instead of report warnings
>> (uninitializing lru)
>>
>> The details are below, and the related info and warn are in
>> attachments, please check, thanks.
>>
>> Next, I will compile gcc compiler with the gcc latest code, if also has
>> this issue, I should communicate with gcc mailing list for it.
>>
>> Thanks.
>>
>> --analyzing
>> begin-
>>
>> /* source code in mm/vmscan.c for next-20130621 */
>>
>> 580 void putback_lru_page(struct page *page)
>> 581 {
>> 582 int lru;
>> 583 int was_unevictable = PageUnevictable(page);
>> 584
>> 585 VM_BUG_ON(PageLRU(page));
>> 586
>> 587 redo:
>> 588 ClearPageUnevictable(page);
>> 589
>> 590 if (page_evictable(page)) {
>> 591 /*
>> 592 * For evictable pages, we can use the cache.
>> 593 * In event of a race, worst case is we end up with an
>> 594 * unevictable page on [in]active list.
>> 595 * We know how to handle that.
>> 596 */
>> 597 lru_cache_add(page);
>> 598 } else {
>> 599 /*
>> 600 * Put unevictable pages directly on zone's unevictable
>> 601 * list.
>> 602 */
>> 603 lru = LRU_UNEVICTABLE;
>> 604 add_page_to_unevictable_list(page);
>> 605 /*
>> 606 * When racing with an mlock or AS_UNEVICTABLE clearing
>> 607 * (page is unlocked) make sure that if the other thread
>> 608 * does not observe our setting of PG_lru and fails
>> 609 * isolation/check_move_unevictable_pages,
>> 610 * we see PG_mlocked/AS_UNEVICTABLE cleared below and
>> move
>> 611 * the page back to the evictable list.
>> 612 *
>> 613 * The other side is TestClearPageMlocked() or
>> shmem_lock().
>> 614 */
>> 615 smp_mb();
>> 616 }
>> 617
>> 618 /*
>> 619 * page's status can
Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch "3abf380..." in next-20130607 tree
On 07/05/2013 12:06 PM, Chen Gang wrote:
> On 06/19/2013 03:19 PM, Chen Gang wrote:
>> On 06/19/2013 03:10 PM, Andrew Morton wrote:
>>> On Wed, 19 Jun 2013 14:55:13 +0800 Chen Gang wrote:
>>>
>
> 'lru' may be used without initialized, so need regressing part of the
> related patch.
>
> The related patch:
> "3abf380 mm: remove lru parameter from __lru_cache_add and
> lru_cache_add_lru"
>
> ...
>
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -595,6 +595,7 @@ redo:
>* unevictable page on [in]active list.
>* We know how to handle that.
>*/
> + lru = !!TestClearPageActive(page) + page_lru_base_type(page);
> lru_cache_add(page);
> } else {
> /*
>>> That looks right. Why the heck didn't gcc-4.4.4 (at least) warn about it?
>>>
>>
>> Sorry I don't know either, I find it by reading code, this time.
>>
>> It is really necessary to continue analyzing why. In 2nd half of 2013, I
>> have planned to make some patches outside kernel but related with kernel
>> (e.g. LTP, gcc patches).
>>
>> This kind of issue is a good chance for me to start in 2nd half of 2013
>> (start from next month).
>>
>> So if no others reply for it, I will start analyzing it in the next
>> month, and plan to finish within a month (before 2013-07-31).
>>
>>
>> Welcome additional suggestions or completions.
>>
>> Thanks.
>>
Under the gcc which from the source code in svn. it still has this
issue. I should communicate with gcc mailing list (or their bugzilla)
for it.
I got gcc source code from svn, "configure && make && make install".
[root@gchenlinux linux-next]# which gcc
/usr/local/bin/gcc
[root@gchenlinux linux-next]# gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/x86_64-unknown-linux-gnu/4.9.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ./configure
Thread model: posix
gcc version 4.9.0 20130704 (experimental) (GCC)
I think, this thread is the end under kernel mailing list. ;-)
Thanks.
>
> Under gcc 4.7.2 20120921 (Red Hat 4.7.2-2) also cause this issue.
>
> The root cause is:
>
> for putback_lur_page() in mm/vmscan.c for next-20130621 tree.
> the compiler assumes "lru == LRU_UNEVICTABLE" instead of report warnings
> (uninitializing lru)
>
> The details are below, and the related info and warn are in
> attachments, please check, thanks.
>
> Next, I will compile gcc compiler with the gcc latest code, if also has
> this issue, I should communicate with gcc mailing list for it.
>
> Thanks.
>
> --analyzing begin-
>
> /* source code in mm/vmscan.c for next-20130621 */
>
> 580 void putback_lru_page(struct page *page)
> 581 {
> 582 int lru;
> 583 int was_unevictable = PageUnevictable(page);
> 584
> 585 VM_BUG_ON(PageLRU(page));
> 586
> 587 redo:
> 588 ClearPageUnevictable(page);
> 589
> 590 if (page_evictable(page)) {
> 591 /*
> 592 * For evictable pages, we can use the cache.
> 593 * In event of a race, worst case is we end up with an
> 594 * unevictable page on [in]active list.
> 595 * We know how to handle that.
> 596 */
> 597 lru_cache_add(page);
> 598 } else {
> 599 /*
> 600 * Put unevictable pages directly on zone's unevictable
> 601 * list.
> 602 */
> 603 lru = LRU_UNEVICTABLE;
> 604 add_page_to_unevictable_list(page);
> 605 /*
> 606 * When racing with an mlock or AS_UNEVICTABLE clearing
> 607 * (page is unlocked) make sure that if the other thread
> 608 * does not observe our setting of PG_lru and fails
> 609 * isolation/check_move_unevictable_pages,
> 610 * we see PG_mlocked/AS_UNEVICTABLE cleared below and
> move
> 611 * the page back to the evictable list.
> 612 *
> 613 * The other side is TestClearPageMlocked() or
> shmem_lock().
> 614 */
> 615 smp_mb();
> 616 }
> 617
> 618 /*
> 619 * page's status can change while we move it among lru. If an
> evictable
> 620 * page is on unevictable list, it never be freed. To avoid that,
> 621 * check after we added it to the list, again.
> 622 */
> 623 if (lru == LRU_UNEVICTABLE && page_evictable(page)) {
> 624 if (!isolate_lru_page(page)) {
> 625 put_page(page);
> 626 goto redo;
> 627 }
> 628 /* This means someone else dropped this page from LRU
> 629
Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch 3abf380... in next-20130607 tree
On 07/05/2013 12:06 PM, Chen Gang wrote:
On 06/19/2013 03:19 PM, Chen Gang wrote:
On 06/19/2013 03:10 PM, Andrew Morton wrote:
On Wed, 19 Jun 2013 14:55:13 +0800 Chen Gang gang.c...@asianux.com wrote:
'lru' may be used without initialized, so need regressing part of the
related patch.
The related patch:
3abf380 mm: remove lru parameter from __lru_cache_add and
lru_cache_add_lru
...
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -595,6 +595,7 @@ redo:
* unevictable page on [in]active list.
* We know how to handle that.
*/
+ lru = !!TestClearPageActive(page) + page_lru_base_type(page);
lru_cache_add(page);
} else {
/*
That looks right. Why the heck didn't gcc-4.4.4 (at least) warn about it?
Sorry I don't know either, I find it by reading code, this time.
It is really necessary to continue analyzing why. In 2nd half of 2013, I
have planned to make some patches outside kernel but related with kernel
(e.g. LTP, gcc patches).
This kind of issue is a good chance for me to start in 2nd half of 2013
(start from next month).
So if no others reply for it, I will start analyzing it in the next
month, and plan to finish within a month (before 2013-07-31).
Welcome additional suggestions or completions.
Thanks.
Under the gcc which from the source code in svn. it still has this
issue. I should communicate with gcc mailing list (or their bugzilla)
for it.
I got gcc source code from svn, configure make make install.
[root@gchenlinux linux-next]# which gcc
/usr/local/bin/gcc
[root@gchenlinux linux-next]# gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/x86_64-unknown-linux-gnu/4.9.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ./configure
Thread model: posix
gcc version 4.9.0 20130704 (experimental) (GCC)
I think, this thread is the end under kernel mailing list. ;-)
Thanks.
Under gcc 4.7.2 20120921 (Red Hat 4.7.2-2) also cause this issue.
The root cause is:
for putback_lur_page() in mm/vmscan.c for next-20130621 tree.
the compiler assumes lru == LRU_UNEVICTABLE instead of report warnings
(uninitializing lru)
The details are below, and the related info and warn are in
attachments, please check, thanks.
Next, I will compile gcc compiler with the gcc latest code, if also has
this issue, I should communicate with gcc mailing list for it.
Thanks.
--analyzing begin-
/* source code in mm/vmscan.c for next-20130621 */
580 void putback_lru_page(struct page *page)
581 {
582 int lru;
583 int was_unevictable = PageUnevictable(page);
584
585 VM_BUG_ON(PageLRU(page));
586
587 redo:
588 ClearPageUnevictable(page);
589
590 if (page_evictable(page)) {
591 /*
592 * For evictable pages, we can use the cache.
593 * In event of a race, worst case is we end up with an
594 * unevictable page on [in]active list.
595 * We know how to handle that.
596 */
597 lru_cache_add(page);
598 } else {
599 /*
600 * Put unevictable pages directly on zone's unevictable
601 * list.
602 */
603 lru = LRU_UNEVICTABLE;
604 add_page_to_unevictable_list(page);
605 /*
606 * When racing with an mlock or AS_UNEVICTABLE clearing
607 * (page is unlocked) make sure that if the other thread
608 * does not observe our setting of PG_lru and fails
609 * isolation/check_move_unevictable_pages,
610 * we see PG_mlocked/AS_UNEVICTABLE cleared below and
move
611 * the page back to the evictable list.
612 *
613 * The other side is TestClearPageMlocked() or
shmem_lock().
614 */
615 smp_mb();
616 }
617
618 /*
619 * page's status can change while we move it among lru. If an
evictable
620 * page is on unevictable list, it never be freed. To avoid that,
621 * check after we added it to the list, again.
622 */
623 if (lru == LRU_UNEVICTABLE page_evictable(page)) {
624 if (!isolate_lru_page(page)) {
625 put_page(page);
626 goto redo;
627 }
628 /* This means someone else dropped this page from LRU
629 * So, it will be freed or putback to LRU again. There is
630 * nothing to do here.
631 */
632 }
633
634 if (was_unevictable lru != LRU_UNEVICTABLE)
635
Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch 3abf380... in next-20130607 tree
On 07/08/2013 02:43 PM, Chen Gang wrote:
On 07/05/2013 12:06 PM, Chen Gang wrote:
On 06/19/2013 03:19 PM, Chen Gang wrote:
On 06/19/2013 03:10 PM, Andrew Morton wrote:
On Wed, 19 Jun 2013 14:55:13 +0800 Chen Gang gang.c...@asianux.com wrote:
'lru' may be used without initialized, so need regressing part of the
related patch.
The related patch:
3abf380 mm: remove lru parameter from __lru_cache_add and
lru_cache_add_lru
...
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -595,6 +595,7 @@ redo:
* unevictable page on [in]active list.
* We know how to handle that.
*/
+lru = !!TestClearPageActive(page) +
page_lru_base_type(page);
lru_cache_add(page);
} else {
/*
That looks right. Why the heck didn't gcc-4.4.4 (at least) warn about it?
Sorry I don't know either, I find it by reading code, this time.
It is really necessary to continue analyzing why. In 2nd half of 2013, I
have planned to make some patches outside kernel but related with kernel
(e.g. LTP, gcc patches).
This kind of issue is a good chance for me to start in 2nd half of 2013
(start from next month).
So if no others reply for it, I will start analyzing it in the next
month, and plan to finish within a month (before 2013-07-31).
Welcome additional suggestions or completions.
Thanks.
I create a related bug for it in gcc, please reference:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57856
Next, I will communicate about it with gcc guys, if any member is
interested in it, please tracing it in gcc bugzilla (bug57856).
Now, truly end of this thread in kernel mailing list.
Thanks.
Under the gcc which from the source code in svn. it still has this
issue. I should communicate with gcc mailing list (or their bugzilla)
for it.
I got gcc source code from svn, configure make make install.
[root@gchenlinux linux-next]# which gcc
/usr/local/bin/gcc
[root@gchenlinux linux-next]# gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/x86_64-unknown-linux-gnu/4.9.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ./configure
Thread model: posix
gcc version 4.9.0 20130704 (experimental) (GCC)
I think, this thread is the end under kernel mailing list. ;-)
Thanks.
Under gcc 4.7.2 20120921 (Red Hat 4.7.2-2) also cause this issue.
The root cause is:
for putback_lur_page() in mm/vmscan.c for next-20130621 tree.
the compiler assumes lru == LRU_UNEVICTABLE instead of report warnings
(uninitializing lru)
The details are below, and the related info and warn are in
attachments, please check, thanks.
Next, I will compile gcc compiler with the gcc latest code, if also has
this issue, I should communicate with gcc mailing list for it.
Thanks.
--analyzing
begin-
/* source code in mm/vmscan.c for next-20130621 */
580 void putback_lru_page(struct page *page)
581 {
582 int lru;
583 int was_unevictable = PageUnevictable(page);
584
585 VM_BUG_ON(PageLRU(page));
586
587 redo:
588 ClearPageUnevictable(page);
589
590 if (page_evictable(page)) {
591 /*
592 * For evictable pages, we can use the cache.
593 * In event of a race, worst case is we end up with an
594 * unevictable page on [in]active list.
595 * We know how to handle that.
596 */
597 lru_cache_add(page);
598 } else {
599 /*
600 * Put unevictable pages directly on zone's unevictable
601 * list.
602 */
603 lru = LRU_UNEVICTABLE;
604 add_page_to_unevictable_list(page);
605 /*
606 * When racing with an mlock or AS_UNEVICTABLE clearing
607 * (page is unlocked) make sure that if the other thread
608 * does not observe our setting of PG_lru and fails
609 * isolation/check_move_unevictable_pages,
610 * we see PG_mlocked/AS_UNEVICTABLE cleared below and
move
611 * the page back to the evictable list.
612 *
613 * The other side is TestClearPageMlocked() or
shmem_lock().
614 */
615 smp_mb();
616 }
617
618 /*
619 * page's status can change while we move it among lru. If an
evictable
620 * page is on unevictable list, it never be freed. To avoid
that,
621 * check after we added it to the list, again.
622 */
623 if (lru == LRU_UNEVICTABLE page_evictable(page)) {
624 if (!isolate_lru_page(page)) {
625
Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch "3abf380..." in next-20130607 tree
On 06/19/2013 04:53 PM, Mel Gorman wrote:
> On Wed, Jun 19, 2013 at 02:55:13PM +0800, Chen Gang wrote:
>> >
>> > 'lru' may be used without initialized, so need regressing part of the
>> > related patch.
>> >
>> > The related patch:
>> > "3abf380 mm: remove lru parameter from __lru_cache_add and
>> > lru_cache_add_lru"
>> >
>> >
>> > Signed-off-by: Chen Gang
>> > ---
>> > mm/vmscan.c |1 +
>> > 1 files changed, 1 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/mm/vmscan.c b/mm/vmscan.c
>> > index fe73724..e92b1858 100644
>> > --- a/mm/vmscan.c
>> > +++ b/mm/vmscan.c
>> > @@ -595,6 +595,7 @@ redo:
>> > * unevictable page on [in]active list.
>> > * We know how to handle that.
>> > */
>> > + lru = !!TestClearPageActive(page) + page_lru_base_type(page);
>> >lru_cache_add(page);
> Thanks for catching this but I have one question. Why are you clearing
> the active bit?
>
Oh, it is my fault, I only want to regress part of the original patch,
did not notice clearing the active bit.
> Before 3abf380 we did
>
> active = TestClearPageActive(page);
> lru = active + page_lru_base_type(page);
> lru_cache_add_lru(page, lru);
>
> so if the page was active before then it gets added to the active list. When
> 3abf380 is applied. it becomes.
>
> Leave PageActive alone
> lru_cache_add(page);
> until __pagevec_lru_add -> __pagevec_lru_add_fn
> int file = page_is_file_cache(page);
> int active = PageActive(page);
> enum lru_list lru = page_lru(page);
>
> After your patch it's
>
> Clear PageActive
> lru_cache_add(page)
> ..
> always add to inactive list
>
> I do not think you intended to do this and if you did, it deserves far
> more comment than being a compile warning fix. In putback_lru_page we only
> care about whether the lru was unevictable or not. Hence I think what you
> meant to do was simply
>
> lru = page_lru_base_type(page);
>
> If you agree then can you resend a revised version to Andrew please?
Yes, I should do, but excuse me, I do not quite know about 'revised
version'.
I guess it means I need still send the related patch which base on the
original one, e.g. for next-20130618:
diff begin-
diff --git a/mm/vmscan.c b/mm/vmscan.c
index fe73724..d03facb 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -595,6 +595,7 @@ redo:
* unevictable page on [in]active list.
* We know how to handle that.
*/
+ lru = page_lru_base_type(page);
lru_cache_add(page);
} else {
/*
diff end---
Is it correct ?
Thanks.
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch "3abf380..." in next-20130607 tree
On Wed, Jun 19, 2013 at 02:55:13PM +0800, Chen Gang wrote: > > 'lru' may be used without initialized, so need regressing part of the > related patch. > > The related patch: > "3abf380 mm: remove lru parameter from __lru_cache_add and > lru_cache_add_lru" > > > Signed-off-by: Chen Gang > --- > mm/vmscan.c |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index fe73724..e92b1858 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -595,6 +595,7 @@ redo: >* unevictable page on [in]active list. >* We know how to handle that. >*/ > + lru = !!TestClearPageActive(page) + page_lru_base_type(page); > lru_cache_add(page); Thanks for catching this but I have one question. Why are you clearing the active bit? Before 3abf380 we did active = TestClearPageActive(page); lru = active + page_lru_base_type(page); lru_cache_add_lru(page, lru); so if the page was active before then it gets added to the active list. When 3abf380 is applied. it becomes. Leave PageActive alone lru_cache_add(page); . until __pagevec_lru_add -> __pagevec_lru_add_fn int file = page_is_file_cache(page); int active = PageActive(page); enum lru_list lru = page_lru(page); After your patch it's Clear PageActive lru_cache_add(page) ... always add to inactive list I do not think you intended to do this and if you did, it deserves far more comment than being a compile warning fix. In putback_lru_page we only care about whether the lru was unevictable or not. Hence I think what you meant to do was simply lru = page_lru_base_type(page); If you agree then can you resend a revised version to Andrew please? -- Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch "3abf380..." in next-20130607 tree
On 06/19/2013 03:10 PM, Andrew Morton wrote:
> On Wed, 19 Jun 2013 14:55:13 +0800 Chen Gang wrote:
>
>> >
>> > 'lru' may be used without initialized, so need regressing part of the
>> > related patch.
>> >
>> > The related patch:
>> > "3abf380 mm: remove lru parameter from __lru_cache_add and
>> > lru_cache_add_lru"
>> >
>> > ...
>> >
>> > --- a/mm/vmscan.c
>> > +++ b/mm/vmscan.c
>> > @@ -595,6 +595,7 @@ redo:
>> > * unevictable page on [in]active list.
>> > * We know how to handle that.
>> > */
>> > + lru = !!TestClearPageActive(page) + page_lru_base_type(page);
>> >lru_cache_add(page);
>> >} else {
>> >/*
> That looks right. Why the heck didn't gcc-4.4.4 (at least) warn about it?
>
Sorry I don't know either, I find it by reading code, this time.
It is really necessary to continue analyzing why. In 2nd half of 2013, I
have planned to make some patches outside kernel but related with kernel
(e.g. LTP, gcc patches).
This kind of issue is a good chance for me to start in 2nd half of 2013
(start from next month).
So if no others reply for it, I will start analyzing it in the next
month, and plan to finish within a month (before 2013-07-31).
Welcome additional suggestions or completions.
Thanks.
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch "3abf380..." in next-20130607 tree
On Wed, 19 Jun 2013 14:55:13 +0800 Chen Gang wrote:
>
> 'lru' may be used without initialized, so need regressing part of the
> related patch.
>
> The related patch:
> "3abf380 mm: remove lru parameter from __lru_cache_add and
> lru_cache_add_lru"
>
> ...
>
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -595,6 +595,7 @@ redo:
>* unevictable page on [in]active list.
>* We know how to handle that.
>*/
> + lru = !!TestClearPageActive(page) + page_lru_base_type(page);
> lru_cache_add(page);
> } else {
> /*
That looks right. Why the heck didn't gcc-4.4.4 (at least) warn about it?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch 3abf380... in next-20130607 tree
On Wed, 19 Jun 2013 14:55:13 +0800 Chen Gang gang.c...@asianux.com wrote:
'lru' may be used without initialized, so need regressing part of the
related patch.
The related patch:
3abf380 mm: remove lru parameter from __lru_cache_add and
lru_cache_add_lru
...
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -595,6 +595,7 @@ redo:
* unevictable page on [in]active list.
* We know how to handle that.
*/
+ lru = !!TestClearPageActive(page) + page_lru_base_type(page);
lru_cache_add(page);
} else {
/*
That looks right. Why the heck didn't gcc-4.4.4 (at least) warn about it?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch 3abf380... in next-20130607 tree
On 06/19/2013 03:10 PM, Andrew Morton wrote:
On Wed, 19 Jun 2013 14:55:13 +0800 Chen Gang gang.c...@asianux.com wrote:
'lru' may be used without initialized, so need regressing part of the
related patch.
The related patch:
3abf380 mm: remove lru parameter from __lru_cache_add and
lru_cache_add_lru
...
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -595,6 +595,7 @@ redo:
* unevictable page on [in]active list.
* We know how to handle that.
*/
+ lru = !!TestClearPageActive(page) + page_lru_base_type(page);
lru_cache_add(page);
} else {
/*
That looks right. Why the heck didn't gcc-4.4.4 (at least) warn about it?
Sorry I don't know either, I find it by reading code, this time.
It is really necessary to continue analyzing why. In 2nd half of 2013, I
have planned to make some patches outside kernel but related with kernel
(e.g. LTP, gcc patches).
This kind of issue is a good chance for me to start in 2nd half of 2013
(start from next month).
So if no others reply for it, I will start analyzing it in the next
month, and plan to finish within a month (before 2013-07-31).
Welcome additional suggestions or completions.
Thanks.
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch 3abf380... in next-20130607 tree
On Wed, Jun 19, 2013 at 02:55:13PM +0800, Chen Gang wrote: 'lru' may be used without initialized, so need regressing part of the related patch. The related patch: 3abf380 mm: remove lru parameter from __lru_cache_add and lru_cache_add_lru Signed-off-by: Chen Gang gang.c...@asianux.com --- mm/vmscan.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/mm/vmscan.c b/mm/vmscan.c index fe73724..e92b1858 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -595,6 +595,7 @@ redo: * unevictable page on [in]active list. * We know how to handle that. */ + lru = !!TestClearPageActive(page) + page_lru_base_type(page); lru_cache_add(page); Thanks for catching this but I have one question. Why are you clearing the active bit? Before 3abf380 we did active = TestClearPageActive(page); lru = active + page_lru_base_type(page); lru_cache_add_lru(page, lru); so if the page was active before then it gets added to the active list. When 3abf380 is applied. it becomes. Leave PageActive alone lru_cache_add(page); . until __pagevec_lru_add - __pagevec_lru_add_fn int file = page_is_file_cache(page); int active = PageActive(page); enum lru_list lru = page_lru(page); After your patch it's Clear PageActive lru_cache_add(page) ... always add to inactive list I do not think you intended to do this and if you did, it deserves far more comment than being a compile warning fix. In putback_lru_page we only care about whether the lru was unevictable or not. Hence I think what you meant to do was simply lru = page_lru_base_type(page); If you agree then can you resend a revised version to Andrew please? -- Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm/vmscan.c: 'lru' may be used without initialized after the patch 3abf380... in next-20130607 tree
On 06/19/2013 04:53 PM, Mel Gorman wrote:
On Wed, Jun 19, 2013 at 02:55:13PM +0800, Chen Gang wrote:
'lru' may be used without initialized, so need regressing part of the
related patch.
The related patch:
3abf380 mm: remove lru parameter from __lru_cache_add and
lru_cache_add_lru
Signed-off-by: Chen Gang gang.c...@asianux.com
---
mm/vmscan.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index fe73724..e92b1858 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -595,6 +595,7 @@ redo:
* unevictable page on [in]active list.
* We know how to handle that.
*/
+ lru = !!TestClearPageActive(page) + page_lru_base_type(page);
lru_cache_add(page);
Thanks for catching this but I have one question. Why are you clearing
the active bit?
Oh, it is my fault, I only want to regress part of the original patch,
did not notice clearing the active bit.
Before 3abf380 we did
active = TestClearPageActive(page);
lru = active + page_lru_base_type(page);
lru_cache_add_lru(page, lru);
so if the page was active before then it gets added to the active list. When
3abf380 is applied. it becomes.
Leave PageActive alone
lru_cache_add(page);
until __pagevec_lru_add - __pagevec_lru_add_fn
int file = page_is_file_cache(page);
int active = PageActive(page);
enum lru_list lru = page_lru(page);
After your patch it's
Clear PageActive
lru_cache_add(page)
..
always add to inactive list
I do not think you intended to do this and if you did, it deserves far
more comment than being a compile warning fix. In putback_lru_page we only
care about whether the lru was unevictable or not. Hence I think what you
meant to do was simply
lru = page_lru_base_type(page);
If you agree then can you resend a revised version to Andrew please?
Yes, I should do, but excuse me, I do not quite know about 'revised
version'.
I guess it means I need still send the related patch which base on the
original one, e.g. for next-20130618:
diff begin-
diff --git a/mm/vmscan.c b/mm/vmscan.c
index fe73724..d03facb 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -595,6 +595,7 @@ redo:
* unevictable page on [in]active list.
* We know how to handle that.
*/
+ lru = page_lru_base_type(page);
lru_cache_add(page);
} else {
/*
diff end---
Is it correct ?
Thanks.
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
