writeback: remove unused parameter from balance_dirty_pages()

PATCH Applied Tahsin Erdogan 3 participants 5 messages Sep 27, 2017 – Oct 2, 2017

Subsystems (2)

Memory management Page cache

lore

Share & tools

lore

Tahsin Erdogan

author

2017-09-27 22:13 UTC #0 lore raw

PATCH 1/1 writeback: remove unused parameter from balance_dirty_pages()

"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 <[email protected]>

---
 mm/page-writeback.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

mm/page-writeback.c +2 -3

index 0b9c5cbe8eba..d89663f00e93 100644
@@ -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);
 }

Signature · 7 lines

2.14.2.822.g60be5d43e6-goog

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

Acked-by on v1 1/1

2017-10-02 07:56 UTC #1 lore raw

Quoted 3 lines · 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")

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?

Quoted 1 line

Signed-off-by Tahsin Erdogan <[email protected]>

Acked-by Michal Hocko <[email protected]>

Quoted 30 lines

---
 mm/page-writeback.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

mm/page-writeback.c +2 -3

index 0b9c5cbe8eba..d89663f00e93 100644
@@ -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);
 }

Signature · 1 line

2.14.2.822.g60be5d43e6-goog

Signature · 2 lines

Michal Hocko
SUSE Labs

Tahsin Erdogan

author

2017-10-02 17:20 UTC #2 lore raw

Quoted 3 lines · Michal Hocko

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

2017-10-02 18:02 UTC #3 lore raw

Quoted 8 lines · Tahsin Erdogan

Quoted 3 lines · Michal Hocko
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.

Signature · 8 lines

Michal Hocko
SUSE Labs

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

Johannes Weiner

Acked-by

2017-10-02 19:54 UTC #4 lore raw

In reply to Michal Hocko

Quoted 8 lines · Michal Hocko

Quoted 3 lines · 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")
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

Quoted 3 lines

Quoted 1 line
Signed-off-by Tahsin Erdogan <[email protected]>
Acked-by Michal Hocko <[email protected]>

Acked-by Johannes Weiner <[email protected]>