On Fri 16-03-18 22:14:24, Tetsuo Handa wrote: > f2fs is doing > > page = f2fs_pagecache_get_page(inode->i_mapping, 0, FGP_LOCK|FGP_NOWAIT, 0); > > which calls > > struct page *pagecache_get_page(inode->i_mapping, 0, FGP_LOCK|FGP_NOWAIT, 0); > > . Then, can't we define > > static inline struct page *find_trylock_page(struct address_space *mapping, > pgoff_t offset) > { > return pagecache_get_page(mapping, offset, FGP_LOCK|FGP_NOWAIT, 0); > } > > and replace find_lock_page() with find_trylock_page() ? I haven't checked whether we have enough users of this pattern to create a helper. > Also, won't > > ---------- > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index 34ce3ebf..0cfc329 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -479,6 +479,8 @@ static inline int trylock_page(struct page *page) > static inline void lock_page(struct page *page) > { > might_sleep(); > + WARN_ONCE(current->flags & PF_MEMALLOC, > + "lock_page() from reclaim context might deadlock"); > if (!trylock_page(page)) > __lock_page(page); > } lock_page is called from many (semi)hot paths so I wouldn't add additional code there. Maybe we can hide it in VM_WARN. I would have to think much more to be sure this won't lead to some strange false positives. I suspect it won't but wouldn't bet my head on that. In any case, you can try to send a patch and we can stick it into mmotm and have it there for few cycles to see what falls out... -- Michal Hocko SUSE Labs