Re: why are some low-level MM routines being exported?

[Minchan Kim <minchan.kim@gmail.com>]

Cced mm guys.

On Mon, Apr 5, 2010 at 3:20 PM, Minchan Kim <minchan.kim@gmail.com> wrote:
> On Mon, Apr 5, 2010 at 2:30 PM, Jörn Engel <joern@logfs.org> 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@logfs.org> 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@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>