On Wed, Jun 16, 2021 at 06:30:43PM -0700, Andrew Morton wrote:

On Wed, 16 Jun 2021 00:54:15 -0400 Johannes Weiner [off-list ref] wrote:

quoted

On Wed, Jun 16, 2021 at 11:20:08AM +1000, Dave Chinner wrote:

quoted

On Tue, Jun 15, 2021 at 02:50:09PM -0400, Johannes Weiner wrote:

quoted

On Tue, Jun 15, 2021 at 04:26:40PM +1000, Dave Chinner wrote:

quoted

On Mon, Jun 14, 2021 at 05:19:04PM -0400, Johannes Weiner wrote:

quoted

@@ -1123,6 +1125,9 @@ static int __remove_mapping(struct address_space *mapping, struct page *page,
 			shadow = workingset_eviction(page, target_memcg);
 		__delete_from_page_cache(page, shadow);
 		xa_unlock_irq(&mapping->i_pages);
+		if (mapping_shrinkable(mapping))
+			inode_add_lru(mapping->host);
+		spin_unlock(&mapping->host->i_lock);
 

No. Inode locks have absolutely no place serialising core vmscan
algorithms.

What if, and hear me out on this one, core vmscan algorithms change
the state of the inode?

Then the core vmscan algorithm has a layering violation.

You're just playing a word game here.

Don't think so.  David is quite correct in saying that vmscan shouldn't
mess with inode state unless it's via address_space_operations?

It seemed to me the complaint was more about vmscan propagating this
state into the inode in general - effecting fs inode acquisitions and
LRU manipulations from the page reclaim callstack - regardless of
whether they are open-coded or indirect through API functions?

Since I mentioned better encapsulation but received no response...