On Mon, Jul 06, 2020 at 09:52:34PM -0700, Hugh Dickins wrote: > On Mon, 6 Jul 2020, Matthew Wilcox wrote: > > On Mon, Jul 06, 2020 at 05:15:09PM +0800, Alex Shi wrote: > > > Hi Kirill & Johannes & Matthew, > > Adding Kirill, who was in patch's Cc list but not mail's Cc list. > > I asked Alex to direct this one particularly to Kirill and Johannes > and Matthew because (and I regret that the commit message still does > not make this at all clear) this patch changes the lock ordering: > which for years has been lru_lock outside memcg move_lock outside > i_pages lock, but here inverted to lru_lock inside i_pages lock. > > I don't see a strong reason to have them one way round or the other, > and think Alex is right that they can safely be reversed here: but > he doesn't actually give any reason for doing so (if cleanup, then > I think the cleanup should have been taken further), and no reason > for doing so as part of this series. I've looked around and changing order of lru_lock wrt. i_pages lock seems safe. I don't have much experience with memcg move_lock. Alex, if you are going ahead with the patch, please document the locking order. We have some locking orders listed at the beginning of filemap.c and rmap.c. local_irq_disable() also deserves a comment. -- Kirill A. Shutemov
