2013-06-14 07:31:00

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH 1/8] mm/writeback: fix wb_do_writeback exported unsafely

There is just one caller in fs-writeback.c call wb_do_writeback and
current codes unnecessary export it in header file, this patch fix
it by changing wb_do_writeback to static function.

Signed-off-by: Wanpeng Li <[email protected]>
---
fs/fs-writeback.c | 2 +-
include/linux/writeback.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 3be5718..f892dec 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -959,7 +959,7 @@ static long wb_check_old_data_flush(struct bdi_writeback *wb)
/*
* Retrieve work items and do the writeback they describe
*/
-long wb_do_writeback(struct bdi_writeback *wb, int force_wait)
+static long wb_do_writeback(struct bdi_writeback *wb, int force_wait)
{
struct backing_dev_info *bdi = wb->bdi;
struct wb_writeback_work *work;
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 579a500..e27468e 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -94,7 +94,6 @@ int try_to_writeback_inodes_sb_nr(struct super_block *, unsigned long nr,
void sync_inodes_sb(struct super_block *);
long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
enum wb_reason reason);
-long wb_do_writeback(struct bdi_writeback *wb, int force_wait);
void wakeup_flusher_threads(long nr_pages, enum wb_reason reason);
void inode_wait_for_writeback(struct inode *inode);

--
1.8.1.2


2013-06-14 07:31:08

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH 3/8] mm/writeback: Don't check force_wait to handle bdi->work_list

After commit 839a8e86("writeback: replace custom worker pool implementation
with unbound workqueue"), bdi_writeback_workfn runs off bdi_writeback->dwork,
on each execution, it processes bdi->work_list and reschedules if there are
more things to do instead of flush any work that race with us existing. It is
unecessary to check force_wait in wb_do_writeback since it is always 0 after
the mentioned commit. This patch remove the force_wait in wb_do_writeback.

Signed-off-by: Wanpeng Li <[email protected]>
---
fs/fs-writeback.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index f892dec..e15aa97 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -959,7 +959,7 @@ static long wb_check_old_data_flush(struct bdi_writeback *wb)
/*
* Retrieve work items and do the writeback they describe
*/
-static long wb_do_writeback(struct bdi_writeback *wb, int force_wait)
+static long wb_do_writeback(struct bdi_writeback *wb)
{
struct backing_dev_info *bdi = wb->bdi;
struct wb_writeback_work *work;
@@ -967,12 +967,6 @@ static long wb_do_writeback(struct bdi_writeback *wb, int force_wait)

set_bit(BDI_writeback_running, &wb->bdi->state);
while ((work = get_next_work_item(bdi)) != NULL) {
- /*
- * Override sync mode, in case we must wait for completion
- * because this thread is exiting now.
- */
- if (force_wait)
- work->sync_mode = WB_SYNC_ALL;

trace_writeback_exec(bdi, work);

@@ -1021,7 +1015,7 @@ void bdi_writeback_workfn(struct work_struct *work)
* rescuer as work_list needs to be drained.
*/
do {
- pages_written = wb_do_writeback(wb, 0);
+ pages_written = wb_do_writeback(wb);
trace_writeback_pages_written(pages_written);
} while (!list_empty(&bdi->work_list));
} else {
--
1.8.1.2

2013-06-14 07:31:19

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH 7/8] mm/thp: fix doc for transparent huge zero page

Transparent huge zero page is used during the page fault instead of
in khugepaged.

# ls /sys/kernel/mm/transparent_hugepage/
defrag enabled khugepaged use_zero_page
# ls /sys/kernel/mm/transparent_hugepage/khugepaged/
alloc_sleep_millisecs defrag full_scans max_ptes_none pages_collapsed pages_to_scan scan_sleep_millisecs

This patch corrects the documentation just like the codes done.

Signed-off-by: Wanpeng Li <[email protected]>
---
Documentation/vm/transhuge.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/vm/transhuge.txt b/Documentation/vm/transhuge.txt
index 8785fb8..4a63953 100644
--- a/Documentation/vm/transhuge.txt
+++ b/Documentation/vm/transhuge.txt
@@ -120,8 +120,8 @@ By default kernel tries to use huge zero page on read page fault.
It's possible to disable huge zero page by writing 0 or enable it
back by writing 1:

-echo 0 >/sys/kernel/mm/transparent_hugepage/khugepaged/use_zero_page
-echo 1 >/sys/kernel/mm/transparent_hugepage/khugepaged/use_zero_page
+echo 0 >/sys/kernel/mm/transparent_hugepage/use_zero_page
+echo 1 >/sys/kernel/mm/transparent_hugepage/use_zero_page

khugepaged will be automatically started when
transparent_hugepage/enabled is set to "always" or "madvise, and it'll
--
1.8.1.2

2013-06-14 07:31:27

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH 8/8] mm/pgtable: Don't accumulate addr during pgd prepopulate pmd

The old codes accumulate addr to get right pmd, however,
currently pmds are preallocated and transfered as a parameter,
there is unnecessary to accumulate addr variable any more, this
patch remove it.

Signed-off-by: Wanpeng Li <[email protected]>
---
arch/x86/mm/pgtable.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index 17fda6a..cb787da 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -240,7 +240,6 @@ static void pgd_mop_up_pmds(struct mm_struct *mm, pgd_t *pgdp)
static void pgd_prepopulate_pmd(struct mm_struct *mm, pgd_t *pgd, pmd_t *pmds[])
{
pud_t *pud;
- unsigned long addr;
int i;

if (PREALLOCATED_PMDS == 0) /* Work around gcc-3.4.x bug */
@@ -248,8 +247,7 @@ static void pgd_prepopulate_pmd(struct mm_struct *mm, pgd_t *pgd, pmd_t *pmds[])

pud = pud_offset(pgd, 0);

- for (addr = i = 0; i < PREALLOCATED_PMDS;
- i++, pud++, addr += PUD_SIZE) {
+ for (i = 0; i < PREALLOCATED_PMDS; i++, pud++) {
pmd_t *pmd = pmds[i];

if (i >= KERNEL_PGD_BOUNDARY)
--
1.8.1.2

2013-06-14 07:31:25

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH 6/8] mm/page_alloc: fix doc for numa_zonelist_order

The default zonelist order selecter will select "node" order if any node's
DMA zone comprises greater than 70% of its local memory instead of 60%,
according to default_zonelist_order::low_kmem_size > total * 70/100.

Signed-off-by: Wanpeng Li <[email protected]>
---
Documentation/sysctl/vm.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
index dcc75a9..36ecc26 100644
--- a/Documentation/sysctl/vm.txt
+++ b/Documentation/sysctl/vm.txt
@@ -510,7 +510,7 @@ Specify "[Dd]efault" to request automatic configuration. Autoconfiguration
will select "node" order in following case.
(1) if the DMA zone does not exist or
(2) if the DMA zone comprises greater than 50% of the available memory or
-(3) if any node's DMA zone comprises greater than 60% of its local memory and
+(3) if any node's DMA zone comprises greater than 70% of its local memory and
the amount of local memory is big enough.

Otherwise, "zone" order will be selected. Default order is recommended unless
--
1.8.1.2

2013-06-14 07:32:01

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH 5/8] mm/page_alloc: fix blank in show_free_areas

There is a blank in show_free_areas which lead to dump messages aren't
aligned. This patch remove blank.

Before patch:

[155219.720141] active_anon:50675 inactive_anon:35273 isolated_anon:0
[155219.720141] active_file:215421 inactive_file:344268 isolated_file:0
[155219.720141] unevictable:0 dirty:35 writeback:0 unstable:0
[155219.720141] free:1334870 slab_reclaimable:28833 slab_unreclaimable:5115
[155219.720141] mapped:25233 shmem:35511 pagetables:1705 bounce:0
[155219.720141] free_cma:0

After patch:

[ 73.913889] active_anon:39578 inactive_anon:32082 isolated_anon:0
[ 73.913889] active_file:14621 inactive_file:57993 isolated_file:0
[ 73.913889] unevictable:0dirty:263 writeback:0 unstable:0
[ 73.913889] free:1865614 slab_reclaimable:3264 slab_unreclaimable:4566
[ 73.913889] mapped:21192 shmem:32327 pagetables:1572 bounce:0
[ 73.913889] free_cma:0

Signed-off-by: Wanpeng Li <[email protected]>
---
mm/page_alloc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c3edb62..3c5ba4e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3001,12 +3001,12 @@ void show_free_areas(unsigned int filter)
}

printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
- " active_file:%lu inactive_file:%lu isolated_file:%lu\n"
- " unevictable:%lu"
- " dirty:%lu writeback:%lu unstable:%lu\n"
- " free:%lu slab_reclaimable:%lu slab_unreclaimable:%lu\n"
- " mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
- " free_cma:%lu\n",
+ "active_file:%lu inactive_file:%lu isolated_file:%lu\n"
+ "unevictable:%lu"
+ "dirty:%lu writeback:%lu unstable:%lu\n"
+ "free:%lu slab_reclaimable:%lu slab_unreclaimable:%lu\n"
+ "mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
+ "free_cma:%lu\n",
global_page_state(NR_ACTIVE_ANON),
global_page_state(NR_INACTIVE_ANON),
global_page_state(NR_ISOLATED_ANON),
--
1.8.1.2

2013-06-14 07:31:05

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH 2/8] mm/writeback: remove wb_reason_name

wb_reason_name is not used any more, this patch remove it.

Signed-off-by: Wanpeng Li <[email protected]>
---
include/linux/writeback.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index e27468e..8b5cec4 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -51,7 +51,6 @@ enum wb_reason {

WB_REASON_MAX,
};
-extern const char *wb_reason_name[];

/*
* A control structure which tells the writeback code what to do. These are
--
1.8.1.2

2013-06-14 07:32:25

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH 4/8] mm/writeback: rename WB_REASON_FORKER_THREAD to WB_REASON_WORKER_THREAD

After commit 839a8e86("writeback: replace custom worker pool implementation
with unbound workqueue"), there is no bdi forker thread any more. This patch
rename WB_REASON_FORKER_THREAD to WB_REASON_WORKER_THREAD since works are
done by emergency worker.

Signed-off-by: Wanpeng Li <[email protected]>
---
fs/fs-writeback.c | 2 +-
include/linux/writeback.h | 2 +-
include/trace/events/writeback.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index e15aa97..87d91d9 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1025,7 +1025,7 @@ void bdi_writeback_workfn(struct work_struct *work)
* enough for efficient IO.
*/
pages_written = writeback_inodes_wb(&bdi->wb, 1024,
- WB_REASON_FORKER_THREAD);
+ WB_REASON_WORKER_THREAD);
trace_writeback_pages_written(pages_written);
}

diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 8b5cec4..c153073 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -47,7 +47,7 @@ enum wb_reason {
WB_REASON_LAPTOP_TIMER,
WB_REASON_FREE_MORE_MEM,
WB_REASON_FS_FREE_SPACE,
- WB_REASON_FORKER_THREAD,
+ WB_REASON_WORKER_THREAD,

WB_REASON_MAX,
};
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 464ea82..f3b33f6 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -28,7 +28,7 @@
{WB_REASON_LAPTOP_TIMER, "laptop_timer"}, \
{WB_REASON_FREE_MORE_MEM, "free_more_memory"}, \
{WB_REASON_FS_FREE_SPACE, "fs_free_space"}, \
- {WB_REASON_FORKER_THREAD, "forker_thread"}
+ {WB_REASON_WORKER_THREAD, "worker_thread"}

struct wb_writeback_work;

--
1.8.1.2

2013-06-14 08:25:59

by Kirill A. Shutemov

[permalink] [raw]
Subject: RE: [PATCH 7/8] mm/thp: fix doc for transparent huge zero page

Wanpeng Li wrote:
> Transparent huge zero page is used during the page fault instead of
> in khugepaged.
>
> # ls /sys/kernel/mm/transparent_hugepage/
> defrag enabled khugepaged use_zero_page
> # ls /sys/kernel/mm/transparent_hugepage/khugepaged/
> alloc_sleep_millisecs defrag full_scans max_ptes_none pages_collapsed pages_to_scan scan_sleep_millisecs
>
> This patch corrects the documentation just like the codes done.
>
> Signed-off-by: Wanpeng Li <[email protected]>

Acked-by: Kirill A. Shutemov <[email protected]>

--
Kirill A. Shutemov

2013-06-14 09:02:25

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH 1/8] mm/writeback: fix wb_do_writeback exported unsafely

On Fri 14-06-13 15:30:34, Wanpeng Li wrote:
> There is just one caller in fs-writeback.c call wb_do_writeback and
> current codes unnecessary export it in header file, this patch fix
> it by changing wb_do_writeback to static function.

So what?

Besides that git grep wb_do_writeback tells that
mm/backing-dev.c: wb_do_writeback(me, 0);

Have you tested this at all?

> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> fs/fs-writeback.c | 2 +-
> include/linux/writeback.h | 1 -
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 3be5718..f892dec 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -959,7 +959,7 @@ static long wb_check_old_data_flush(struct bdi_writeback *wb)
> /*
> * Retrieve work items and do the writeback they describe
> */
> -long wb_do_writeback(struct bdi_writeback *wb, int force_wait)
> +static long wb_do_writeback(struct bdi_writeback *wb, int force_wait)
> {
> struct backing_dev_info *bdi = wb->bdi;
> struct wb_writeback_work *work;
> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> index 579a500..e27468e 100644
> --- a/include/linux/writeback.h
> +++ b/include/linux/writeback.h
> @@ -94,7 +94,6 @@ int try_to_writeback_inodes_sb_nr(struct super_block *, unsigned long nr,
> void sync_inodes_sb(struct super_block *);
> long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
> enum wb_reason reason);
> -long wb_do_writeback(struct bdi_writeback *wb, int force_wait);
> void wakeup_flusher_threads(long nr_pages, enum wb_reason reason);
> void inode_wait_for_writeback(struct inode *inode);
>
> --
> 1.8.1.2
>

--
Michal Hocko
SUSE Labs

2013-06-14 09:27:13

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH 1/8] mm/writeback: fix wb_do_writeback exported unsafely

Michal Hocko wrote:
> On Fri 14-06-13 15:30:34, Wanpeng Li wrote:
> > There is just one caller in fs-writeback.c call wb_do_writeback and
> > current codes unnecessary export it in header file, this patch fix
> > it by changing wb_do_writeback to static function.
>
> So what?
>
> Besides that git grep wb_do_writeback tells that
> mm/backing-dev.c: wb_do_writeback(me, 0);
>
> Have you tested this at all?

Commit 839a8e8 removes that.

> > Signed-off-by: Wanpeng Li <[email protected]>

Acked-by: Kirill A. Shutemov <[email protected]>

--
Kirill A. Shutemov

2013-06-14 09:31:51

by Haicheng Li

[permalink] [raw]
Subject: Re: [PATCH 1/8] mm/writeback: fix wb_do_writeback exported unsafely


On Fri, Jun 14, 2013 at 03:30:34PM +0800, Wanpeng Li wrote:
> There is just one caller in fs-writeback.c call wb_do_writeback and
> current codes unnecessary export it in header file, this patch fix
> it by changing wb_do_writeback to static function.
>
> Signed-off-by: Wanpeng Li <[email protected]>

Hi Wanpeng,

A simliar patch has been merged in -next tree with commit#: 836f29bbb0f7a08dbdf1ed3ee704ef8aea81e56f

BTW, actually this should have nothing to do with safety, just unnecessary to export it globally.
> ---
> fs/fs-writeback.c | 2 +-
> include/linux/writeback.h | 1 -
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 3be5718..f892dec 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -959,7 +959,7 @@ static long wb_check_old_data_flush(struct bdi_writeback *wb)
> /*
> * Retrieve work items and do the writeback they describe
> */
> -long wb_do_writeback(struct bdi_writeback *wb, int force_wait)
> +static long wb_do_writeback(struct bdi_writeback *wb, int force_wait)
> {
> struct backing_dev_info *bdi = wb->bdi;
> struct wb_writeback_work *work;
> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> index 579a500..e27468e 100644
> --- a/include/linux/writeback.h
> +++ b/include/linux/writeback.h
> @@ -94,7 +94,6 @@ int try_to_writeback_inodes_sb_nr(struct super_block *, unsigned long nr,
> void sync_inodes_sb(struct super_block *);
> long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
> enum wb_reason reason);
> -long wb_do_writeback(struct bdi_writeback *wb, int force_wait);
> void wakeup_flusher_threads(long nr_pages, enum wb_reason reason);
> void inode_wait_for_writeback(struct inode *inode);
>
> --
> 1.8.1.2
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

2013-06-14 09:32:04

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH 1/8] mm/writeback: fix wb_do_writeback exported unsafely

On Fri 14-06-13 12:29:52, Kirill A. Shutemov wrote:
> Michal Hocko wrote:
> > On Fri 14-06-13 15:30:34, Wanpeng Li wrote:
> > > There is just one caller in fs-writeback.c call wb_do_writeback and
> > > current codes unnecessary export it in header file, this patch fix
> > > it by changing wb_do_writeback to static function.
> >
> > So what?
> >
> > Besides that git grep wb_do_writeback tells that
> > mm/backing-dev.c: wb_do_writeback(me, 0);
> >
> > Have you tested this at all?
>
> Commit 839a8e8 removes that.

OK, I didn't check the most up-to-date tree. Sorry about this confusion.
I do not object to cleanups like this but it should be clear they are
cleanups. "fix wb_do_writeback exported unsafely" sounds like a fix
rather than a cleanup

> > > Signed-off-by: Wanpeng Li <[email protected]>
>
> Acked-by: Kirill A. Shutemov <[email protected]>
>
> --
> Kirill A. Shutemov
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

--
Michal Hocko
SUSE Labs

2013-06-14 09:41:41

by Zhang Yanfei

[permalink] [raw]
Subject: Re: [PATCH 1/8] mm/writeback: fix wb_do_writeback exported unsafely

On 06/14/2013 05:31 PM, Michal Hocko wrote:
> On Fri 14-06-13 12:29:52, Kirill A. Shutemov wrote:
>> Michal Hocko wrote:
>>> On Fri 14-06-13 15:30:34, Wanpeng Li wrote:
>>>> There is just one caller in fs-writeback.c call wb_do_writeback and
>>>> current codes unnecessary export it in header file, this patch fix
>>>> it by changing wb_do_writeback to static function.
>>>
>>> So what?
>>>
>>> Besides that git grep wb_do_writeback tells that
>>> mm/backing-dev.c: wb_do_writeback(me, 0);
>>>
>>> Have you tested this at all?
>>
>> Commit 839a8e8 removes that.
>
> OK, I didn't check the most up-to-date tree. Sorry about this confusion.
> I do not object to cleanups like this but it should be clear they are
> cleanups. "fix wb_do_writeback exported unsafely" sounds like a fix
> rather than a cleanup

Agreed.

>
>>>> Signed-off-by: Wanpeng Li <[email protected]>
>>
>> Acked-by: Kirill A. Shutemov <[email protected]>
>>
>> --
>> Kirill A. Shutemov
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to [email protected]. For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>


--
Thanks.
Zhang Yanfei

2013-06-14 17:57:18

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH 4/8] mm/writeback: rename WB_REASON_FORKER_THREAD to WB_REASON_WORKER_THREAD

On Fri, Jun 14, 2013 at 03:30:37PM +0800, Wanpeng Li wrote:
> After commit 839a8e86("writeback: replace custom worker pool implementation
> with unbound workqueue"), there is no bdi forker thread any more. This patch
> rename WB_REASON_FORKER_THREAD to WB_REASON_WORKER_THREAD since works are
> done by emergency worker.

This is somewhat userland visible and we'll be exposing exactly the
same information with just a different name. While the string doesn't
match the current implementation exactly, I don't think we need to
change it. Maybe add a comment there saying why it has a mismatching
name is enough?

Thanks.

--
tejun