Cced mm guys. On Mon, Apr 5, 2010 at 3:20 PM, Minchan Kim <minchan.kim@xxxxxxxxx> wrote: > On Mon, Apr 5, 2010 at 2:30 PM, Jörn Engel <joern@xxxxxxxxx> wrote: >> On Mon, 5 April 2010 09:59:18 +0900, Minchan Kim wrote: >>> On Mon, Apr 5, 2010 at 4:55 AM, Jörn Engel <joern@xxxxxxxxx> wrote: >>> > On Mon, 5 April 2010 01:21:52 +0900, Minchan Kim wrote: >>> >> > >>> >> Until now, other file system don't need it. >>> >> Why do you need? >>> > >>> > To avoid deadlocks. You tell logfs to write out some locked page, logfs >>> > determines that it needs to run garbage collection first. Garbage >>> > collection can read any page. If it called find_or_create_page() for >>> > the locked page, you have a deadlock. >>> >>> Could you do it with add_to_page_cache and pagevec_lru_add_file? >> >> Maybe. But how would that be an improvement? >> >> As I see it, logfs needs a variant of find_or_create_page() that does >> not block on any pages waiting for logfs GC. Currently that variant >> lives under fs/logfs/ and uses add_to_page_cache_lru(). If there are >> valid reasons against exporting add_to_page_cache_lru(), the right >> solution is to move the logfs variant to mm/, not to rewrite it. >> >> If you want to change the implementation from using >> add_to_page_cache_lru() to using add_to_page_cache() and >> pagevec_lru_add_file(), then you should have a better reason than not >> exporting add_to_page_cache_lru(). If the new implementation was any >> better, I would gladly take it. > > Previously I said, what I have a concern is that if file systems or > some modules abuses > add_to_page_cache_lru, it might system LRU list wrong so then system > go to hell. > Of course, if we use it carefully, it can be good but how do you make sure it? > > I am not a file system expert but as I read comment of read_cache_pages > "Hides the details of the LRU cache etc from the filesystem", I > thought it is not good that > file system handle LRU list directly. At least, we have been trying for years. > > If we can do it with current functions without big cost, I think it's > rather good than exporting > new function. Until 18bc0bbd162e3, we didn't export that but all file > systems works well. > In addition, when the patch is merged, any mm guys seem to be not > reviewed it, too. > > I hope just ring at the bell to remain record to justify why we need > exporting new function > although we can do it with existing functions. > > If any other mm guys don't oppose it, I would be not against that, either. > > -- > Kind regards, > Minchan Kim > -- Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href