lkml.org 
[lkml]   [2010]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 2/3] Retry page fault when blocking on disk transfer.
From
On Fri, Oct 8, 2010 at 6:24 AM, Rik van Riel <riel@redhat.com> wrote:
>> +static inline int lock_page_or_retry(struct page *page, struct mm_struct
>> *mm,
>> +                                    unsigned int flags)
>> +{
>> +       if (trylock_page(page))
>> +               return 1;
>> +       if (!(flags&  FAULT_FLAG_ALLOW_RETRY)) {
>> +               __lock_page(page);
>> +               return 1;
>> +       }
>> +
>> +       up_read(&mm->mmap_sem);
>> +       wait_on_page_locked(page);
>> +       return 0;
>> +}
>
> Wait a moment.  Your other patch 2/3 also has a
> lock_page_or_retry function.  That one is in
> filemap.c and takes slightly different arguments,
> to do essentially the same thing...
>
> +/*
> + * Lock the page, unless this would block and the caller indicated that it
> + * can handle a retry.
> + */
> +static int lock_page_or_retry(struct page *page,
> +                             struct vm_area_struct *vma, struct vm_fault
> *vmf)
> +{
>
> Is there a way the two functions can be merged
> into one?

Yes, this would be easy to do.

The argument against it would be loss of inlining and, in the filemap
version, the need to reference vma and vmf fields to find out the mm
and flags values. We'd like to avoid doing that at least in the fast
path when trylock_page succeeds - though, now that I think about it,
both could be avoided with an inline function in a header returning
trylock_page(page) || _lock_page_or_retry(mm, flags)

Hmmm, this is actually quite similar to how other functions in
pagemap.h / filemap.c are done...
I'll send an updated series using this suggestion.

--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2010-10-08 22:09    [W:0.084 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site