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

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

[Wanpeng Li]

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 <liwanp@linux.vnet.ibm.com>
---
 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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/8] mm/writeback: remove wb_reason_name

[Wanpeng Li]

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

Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
---
 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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

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 <liwanp@linux.vnet.ibm.com>
---
 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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

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 <liwanp@linux.vnet.ibm.com>
---
 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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Tejun Heo]

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

On Fri, Jun 14, 2013 at 10:57:10AM -0700, Tejun Heo wrote:
>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?
>

Good point, I will update them in next version, thanks for your review.
;-)

Regards,
Wanpeng Li 

>Thanks.
>
>-- 
>tejun
>
>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org.  For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

On Fri, Jun 14, 2013 at 10:57:10AM -0700, Tejun Heo wrote:
>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?
>

Good point, I will update them in next version, thanks for your review.
;-)

Regards,
Wanpeng Li 

>Thanks.
>
>-- 
>tejun
>
>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org.  For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

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 <liwanp@linux.vnet.ibm.com>
---
 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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

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 <liwanp@linux.vnet.ibm.com>
---
 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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

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 <liwanp@linux.vnet.ibm.com>
---
 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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Kirill A. Shutemov]

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 <liwanp@linux.vnet.ibm.com>

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

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 <liwanp@linux.vnet.ibm.com>
---
 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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Michal Hocko]

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 <liwanp@linux.vnet.ibm.com>
> ---
>  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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

On Fri, Jun 14, 2013 at 11:02:17AM +0200, 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);
>

I don't think this can be found in 3.10-rc1 ~ 3.10-rc5. ;-) 
Since Tejun's patchset commit 839a8e86("writeback: replace 
custom worker pool implementation with unbound workqueue")
merged, just one caller in fs/fs-writeback.c now. 

>Have you tested this at all?
>

I test them all against 3.10-rc5. ;-)

>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>> ---
>>  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
>
>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org.  For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

On Fri, Jun 14, 2013 at 11:02:17AM +0200, 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);
>

I don't think this can be found in 3.10-rc1 ~ 3.10-rc5. ;-) 
Since Tejun's patchset commit 839a8e86("writeback: replace 
custom worker pool implementation with unbound workqueue")
merged, just one caller in fs/fs-writeback.c now. 

>Have you tested this at all?
>

I test them all against 3.10-rc5. ;-)

>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>> ---
>>  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
>
>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org.  For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Kirill A. Shutemov]

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 <liwanp@linux.vnet.ibm.com>

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Michal Hocko]

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 <liwanp@linux.vnet.ibm.com>
> 
> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> 
> -- 
>  Kirill A. Shutemov
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Zhang Yanfei]

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 <liwanp@linux.vnet.ibm.com>
>>
>> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>>
>> -- 
>>  Kirill A. Shutemov
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 


-- 
Thanks.
Zhang Yanfei

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Haicheng Li]

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 <liwanp@linux.vnet.ibm.com>

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 majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

On Fri, Jun 14, 2013 at 05:31:21PM +0800, Haicheng Li wrote:
>
>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 <liwanp@linux.vnet.ibm.com>
>
>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.

Oh, I miss your commit, anyway, good to see your work. ;-)

>> ---
>>  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 majordomo@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Wanpeng Li]

On Fri, Jun 14, 2013 at 05:31:21PM +0800, Haicheng Li wrote:
>
>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 <liwanp@linux.vnet.ibm.com>
>
>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.

Oh, I miss your commit, anyway, good to see your work. ;-)

>> ---
>>  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 majordomo@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>