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