[PATCH] writeback: remove unused parameter from balance_dirty_pages()

[PATCH] writeback: remove unused parameter from balance_dirty_pages()

[Tahsin Erdogan]

"mapping" parameter to balance_dirty_pages() is not used anymore.

Fixes: dfb8ae567835 ("writeback: let balance_dirty_pages() work on the matching cgroup bdi_writeback")

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
 mm/page-writeback.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0b9c5cbe8eba..d89663f00e93 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1559,8 +1559,7 @@ static inline void wb_dirty_limits(struct dirty_throttle_control *dtc)
  * If we're over `background_thresh' then the writeback threads are woken to
  * perform some writeout.
  */
-static void balance_dirty_pages(struct address_space *mapping,
-				struct bdi_writeback *wb,
+static void balance_dirty_pages(struct bdi_writeback *wb,
 				unsigned long pages_dirtied)
 {
 	struct dirty_throttle_control gdtc_stor = { GDTC_INIT(wb) };
@@ -1910,7 +1909,7 @@ void balance_dirty_pages_ratelimited(struct address_space *mapping)
 	preempt_enable();
 
 	if (unlikely(current->nr_dirtied >= ratelimit))
-		balance_dirty_pages(mapping, wb, current->nr_dirtied);
+		balance_dirty_pages(wb, current->nr_dirtied);
 
 	wb_put(wb);
 }
-- 
2.14.2.822.g60be5d43e6-goog

[PATCH] writeback: remove unused parameter from balance_dirty_pages()

[Tahsin Erdogan]

"mapping" parameter to balance_dirty_pages() is not used anymore.

Fixes: dfb8ae567835 ("writeback: let balance_dirty_pages() work on the matching cgroup bdi_writeback")

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
 mm/page-writeback.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0b9c5cbe8eba..d89663f00e93 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1559,8 +1559,7 @@ static inline void wb_dirty_limits(struct dirty_throttle_control *dtc)
  * If we're over `background_thresh' then the writeback threads are woken to
  * perform some writeout.
  */
-static void balance_dirty_pages(struct address_space *mapping,
-				struct bdi_writeback *wb,
+static void balance_dirty_pages(struct bdi_writeback *wb,
 				unsigned long pages_dirtied)
 {
 	struct dirty_throttle_control gdtc_stor = { GDTC_INIT(wb) };
@@ -1910,7 +1909,7 @@ void balance_dirty_pages_ratelimited(struct address_space *mapping)
 	preempt_enable();
 
 	if (unlikely(current->nr_dirtied >= ratelimit))
-		balance_dirty_pages(mapping, wb, current->nr_dirtied);
+		balance_dirty_pages(wb, current->nr_dirtied);
 
 	wb_put(wb);
 }
-- 
2.14.2.822.g60be5d43e6-goog

--
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] writeback: remove unused parameter from balance_dirty_pages()

[Michal Hocko]

On Wed 27-09-17 15:13:11, Tahsin Erdogan wrote:
> "mapping" parameter to balance_dirty_pages() is not used anymore.
> 
> Fixes: dfb8ae567835 ("writeback: let balance_dirty_pages() work on the matching cgroup bdi_writeback")

balance_dirty_pages_ratelimited doesn't really need mapping as well. All
it needs is the inode and we already have it in callers. So would it
make sense to refactor a bit further and make its argument an inode?

> Signed-off-by: Tahsin Erdogan <tahsin@google.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/page-writeback.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 0b9c5cbe8eba..d89663f00e93 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -1559,8 +1559,7 @@ static inline void wb_dirty_limits(struct dirty_throttle_control *dtc)
>   * If we're over `background_thresh' then the writeback threads are woken to
>   * perform some writeout.
>   */
> -static void balance_dirty_pages(struct address_space *mapping,
> -				struct bdi_writeback *wb,
> +static void balance_dirty_pages(struct bdi_writeback *wb,
>  				unsigned long pages_dirtied)
>  {
>  	struct dirty_throttle_control gdtc_stor = { GDTC_INIT(wb) };
> @@ -1910,7 +1909,7 @@ void balance_dirty_pages_ratelimited(struct address_space *mapping)
>  	preempt_enable();
>  
>  	if (unlikely(current->nr_dirtied >= ratelimit))
> -		balance_dirty_pages(mapping, wb, current->nr_dirtied);
> +		balance_dirty_pages(wb, current->nr_dirtied);
>  
>  	wb_put(wb);
>  }
> -- 
> 2.14.2.822.g60be5d43e6-goog
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] writeback: remove unused parameter from balance_dirty_pages()

[Michal Hocko]

On Wed 27-09-17 15:13:11, Tahsin Erdogan wrote:
> "mapping" parameter to balance_dirty_pages() is not used anymore.
> 
> Fixes: dfb8ae567835 ("writeback: let balance_dirty_pages() work on the matching cgroup bdi_writeback")

balance_dirty_pages_ratelimited doesn't really need mapping as well. All
it needs is the inode and we already have it in callers. So would it
make sense to refactor a bit further and make its argument an inode?

> Signed-off-by: Tahsin Erdogan <tahsin@google.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/page-writeback.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 0b9c5cbe8eba..d89663f00e93 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -1559,8 +1559,7 @@ static inline void wb_dirty_limits(struct dirty_throttle_control *dtc)
>   * If we're over `background_thresh' then the writeback threads are woken to
>   * perform some writeout.
>   */
> -static void balance_dirty_pages(struct address_space *mapping,
> -				struct bdi_writeback *wb,
> +static void balance_dirty_pages(struct bdi_writeback *wb,
>  				unsigned long pages_dirtied)
>  {
>  	struct dirty_throttle_control gdtc_stor = { GDTC_INIT(wb) };
> @@ -1910,7 +1909,7 @@ void balance_dirty_pages_ratelimited(struct address_space *mapping)
>  	preempt_enable();
>  
>  	if (unlikely(current->nr_dirtied >= ratelimit))
> -		balance_dirty_pages(mapping, wb, current->nr_dirtied);
> +		balance_dirty_pages(wb, current->nr_dirtied);
>  
>  	wb_put(wb);
>  }
> -- 
> 2.14.2.822.g60be5d43e6-goog
> 

-- 
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] writeback: remove unused parameter from balance_dirty_pages()

[Tahsin Erdogan]

On Mon, Oct 2, 2017 at 12:56 AM, Michal Hocko <mhocko@kernel.org> wrote:
> balance_dirty_pages_ratelimited doesn't really need mapping as well. All
> it needs is the inode and we already have it in callers. So would it
> make sense to refactor a bit further and make its argument an inode?

My only concern is that, balance_dirty_pages_ratelimited() is an
exported function so changing its signature could potentially break
some drivers?

Re: [PATCH] writeback: remove unused parameter from balance_dirty_pages()

[Tahsin Erdogan]

On Mon, Oct 2, 2017 at 12:56 AM, Michal Hocko <mhocko@kernel.org> wrote:
> balance_dirty_pages_ratelimited doesn't really need mapping as well. All
> it needs is the inode and we already have it in callers. So would it
> make sense to refactor a bit further and make its argument an inode?

My only concern is that, balance_dirty_pages_ratelimited() is an
exported function so changing its signature could potentially break
some drivers?

--
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] writeback: remove unused parameter from balance_dirty_pages()

[Michal Hocko]

On Mon 02-10-17 10:20:37, Tahsin Erdogan wrote:
> On Mon, Oct 2, 2017 at 12:56 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > balance_dirty_pages_ratelimited doesn't really need mapping as well. All
> > it needs is the inode and we already have it in callers. So would it
> > make sense to refactor a bit further and make its argument an inode?
> 
> My only concern is that, balance_dirty_pages_ratelimited() is an
> exported function so changing its signature could potentially break
> some drivers?

All in-kernel drivers would have to be updated of course but exported
symbols are not considered a stable API. It's not like we would want to
change this for no good reason so the change should be done only if
this makes sense in general. This is something for IO/FS guys to tell.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] writeback: remove unused parameter from balance_dirty_pages()

[Michal Hocko]

On Mon 02-10-17 10:20:37, Tahsin Erdogan wrote:
> On Mon, Oct 2, 2017 at 12:56 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > balance_dirty_pages_ratelimited doesn't really need mapping as well. All
> > it needs is the inode and we already have it in callers. So would it
> > make sense to refactor a bit further and make its argument an inode?
> 
> My only concern is that, balance_dirty_pages_ratelimited() is an
> exported function so changing its signature could potentially break
> some drivers?

All in-kernel drivers would have to be updated of course but exported
symbols are not considered a stable API. It's not like we would want to
change this for no good reason so the change should be done only if
this makes sense in general. This is something for IO/FS guys to tell.

-- 
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] writeback: remove unused parameter from balance_dirty_pages()

[Johannes Weiner]

On Mon, Oct 02, 2017 at 09:56:16AM +0200, Michal Hocko wrote:
> On Wed 27-09-17 15:13:11, Tahsin Erdogan wrote:
> > "mapping" parameter to balance_dirty_pages() is not used anymore.
> > 
> > Fixes: dfb8ae567835 ("writeback: let balance_dirty_pages() work on the matching cgroup bdi_writeback")
> 
> balance_dirty_pages_ratelimited doesn't really need mapping as well. All
> it needs is the inode and we already have it in callers. So would it
> make sense to refactor a bit further and make its argument an inode?

It's nicer to keep this a "page cache" interface, as its primary
callsites are in mm/memory.c and mm/filemap.c:

	$ git grep -c 'inode' mm/filemap.c mm/memory.c 
	mm/filemap.c:38
	$ git grep -c 'mapping' mm/filemap.c mm/memory.c 
	mm/filemap.c:260
	mm/memory.c:93

> > Signed-off-by: Tahsin Erdogan <tahsin@google.com>
> 
> Acked-by: Michal Hocko <mhocko@suse.com>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Re: [PATCH] writeback: remove unused parameter from balance_dirty_pages()

[Johannes Weiner]

On Mon, Oct 02, 2017 at 09:56:16AM +0200, Michal Hocko wrote:
> On Wed 27-09-17 15:13:11, Tahsin Erdogan wrote:
> > "mapping" parameter to balance_dirty_pages() is not used anymore.
> > 
> > Fixes: dfb8ae567835 ("writeback: let balance_dirty_pages() work on the matching cgroup bdi_writeback")
> 
> balance_dirty_pages_ratelimited doesn't really need mapping as well. All
> it needs is the inode and we already have it in callers. So would it
> make sense to refactor a bit further and make its argument an inode?

It's nicer to keep this a "page cache" interface, as its primary
callsites are in mm/memory.c and mm/filemap.c:

	$ git grep -c 'inode' mm/filemap.c mm/memory.c 
	mm/filemap.c:38
	$ git grep -c 'mapping' mm/filemap.c mm/memory.c 
	mm/filemap.c:260
	mm/memory.c:93

> > Signed-off-by: Tahsin Erdogan <tahsin@google.com>
> 
> Acked-by: Michal Hocko <mhocko@suse.com>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

--
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>