On Tue 13-07-21 07:25:05, Christoph Hellwig wrote:
> Still looks good. That being said the additional conditional locking in
> filemap_fault makes it fall over the readbility cliff for me. Something
> like this on top of your series would help:
Yeah, that's better. Applied, thanks.
Honza
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index fd3f94d36c49..0fad08331cf4 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3040,21 +3040,23 @@ vm_fault_t filemap_fault(struct vm_fault *vmf)
> * Do we have something in the page cache already?
> */
> page = find_get_page(mapping, offset);
> - if (likely(page) && !(vmf->flags & FAULT_FLAG_TRIED)) {
> + if (likely(page)) {
> /*
> - * We found the page, so try async readahead before
> - * waiting for the lock.
> + * We found the page, so try async readahead before waiting for
> + * the lock.
> */
> - fpin = do_async_mmap_readahead(vmf, page);
> - } else if (!page) {
> + if (!(vmf->flags & FAULT_FLAG_TRIED))
> + fpin = do_async_mmap_readahead(vmf, page);
> + if (unlikely(!PageUptodate(page))) {
> + filemap_invalidate_lock_shared(mapping);
> + mapping_locked = true;
> + }
> + } else {
> /* No page in the page cache at all */
> count_vm_event(PGMAJFAULT);
> count_memcg_event_mm(vmf->vma->vm_mm, PGMAJFAULT);
> ret = VM_FAULT_MAJOR;
> fpin = do_sync_mmap_readahead(vmf);
> - }
> -
> - if (!page) {
> retry_find:
> /*
> * See comment in filemap_create_page() why we need
> @@ -3073,9 +3075,6 @@ vm_fault_t filemap_fault(struct vm_fault *vmf)
> filemap_invalidate_unlock_shared(mapping);
> return VM_FAULT_OOM;
> }
> - } else if (unlikely(!PageUptodate(page))) {
> - filemap_invalidate_lock_shared(mapping);
> - mapping_locked = true;
> }
>
> if (!lock_page_maybe_drop_mmap(vmf, page, &fpin))
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
