All of lore.kernel.org
 help / color / mirror / Atom feed
From: inki.dae@samsung.com (Inki Dae)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 1/2] arm: cacheflush syscall: process only pages that are in the memory
Date: Wed, 31 Jan 2018 15:03:23 +0900	[thread overview]
Message-ID: <5A715C2B.70609@samsung.com> (raw)
In-Reply-To: <20180126213911.GY17719@n2100.armlinux.org.uk>

Hi Russell,

2018? 01? 27? 06:39? Russell King - ARM Linux ?(?) ? ?:
> On Fri, Jan 26, 2018 at 02:30:47PM +0100, Marek Szyprowski wrote:
>> Hi Russell,
>>
>> On 2018-01-26 12:32, Russell King - ARM Linux wrote:
>>> On Fri, Jan 26, 2018 at 12:14:40PM +0100, Marek Szyprowski wrote:
>>>> glibc in calls cacheflush syscall on the whole textrels section of the
>>>> relocated binaries. However, relocation usually doesn't touch all pages
>>>> of that section, so not all of them are read to memory when calling this
>>>> syscall. However flush_cache_user_range() function will unconditionally
>>>> touch all pages from the provided range, resulting additional overhead
>>>> related to reading all clean pages. Optimize this by calling
>>>> flush_cache_user_range() only on the pages that are already in the
>>>> memory.
>>> What ensures that another CPU doesn't remove a page while we're
>>> flushing it?  That will trigger a data abort, which will want to
>>> take the mmap_sem, causing a deadlock.
>>
>> I thought that taking mmap_sem will prevent pages from being removed.
>> mmap_sem has been already taken in the previous implementation of that
>> syscall, until code simplification done by commit 97c72d89ce0e ("ARM:
>> cacheflush: don't bother rounding to nearest vma").
> 
> No, you're not reading the previous code state correctly.  Take a closer
> look at that commit.
> 
> find_vma() requires that mmap_sem is held across the call as the VMA
> list is not stable without that semaphore held.  However, more
> importantly, notice that it drops the semaphore _before_ calling the
> cache flushing function (__do_cache_op()).
> 
> The point is that if __do_cache_op() faults, it will enter
> do_page_fault(), which will try to take the mmap_sem again, causing
> a deadlock.

I'm not sure but seems this patch tries to do cache-flush only in-memory pages.
So I think the page fault wouldn't happen becasue flush_cache_user_range function returns always 0.

Thanks,
Inki Dae

> 

WARNING: multiple messages have this Message-ID (diff)
From: Inki Dae <inki.dae@samsung.com>
To: Russell King - ARM Linux <linux@armlinux.org.uk>,
	Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [RFC 1/2] arm: cacheflush syscall: process only pages that are in the memory
Date: Wed, 31 Jan 2018 15:03:23 +0900	[thread overview]
Message-ID: <5A715C2B.70609@samsung.com> (raw)
In-Reply-To: <20180126213911.GY17719@n2100.armlinux.org.uk>

Hi Russell,

2018년 01월 27일 06:39에 Russell King - ARM Linux 이(가) 쓴 글:
> On Fri, Jan 26, 2018 at 02:30:47PM +0100, Marek Szyprowski wrote:
>> Hi Russell,
>>
>> On 2018-01-26 12:32, Russell King - ARM Linux wrote:
>>> On Fri, Jan 26, 2018 at 12:14:40PM +0100, Marek Szyprowski wrote:
>>>> glibc in calls cacheflush syscall on the whole textrels section of the
>>>> relocated binaries. However, relocation usually doesn't touch all pages
>>>> of that section, so not all of them are read to memory when calling this
>>>> syscall. However flush_cache_user_range() function will unconditionally
>>>> touch all pages from the provided range, resulting additional overhead
>>>> related to reading all clean pages. Optimize this by calling
>>>> flush_cache_user_range() only on the pages that are already in the
>>>> memory.
>>> What ensures that another CPU doesn't remove a page while we're
>>> flushing it?  That will trigger a data abort, which will want to
>>> take the mmap_sem, causing a deadlock.
>>
>> I thought that taking mmap_sem will prevent pages from being removed.
>> mmap_sem has been already taken in the previous implementation of that
>> syscall, until code simplification done by commit 97c72d89ce0e ("ARM:
>> cacheflush: don't bother rounding to nearest vma").
> 
> No, you're not reading the previous code state correctly.  Take a closer
> look at that commit.
> 
> find_vma() requires that mmap_sem is held across the call as the VMA
> list is not stable without that semaphore held.  However, more
> importantly, notice that it drops the semaphore _before_ calling the
> cache flushing function (__do_cache_op()).
> 
> The point is that if __do_cache_op() faults, it will enter
> do_page_fault(), which will try to take the mmap_sem again, causing
> a deadlock.

I'm not sure but seems this patch tries to do cache-flush only in-memory pages.
So I think the page fault wouldn't happen becasue flush_cache_user_range function returns always 0.

Thanks,
Inki Dae

> 

  reply	other threads:[~2018-01-31  6:03 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20180126111453eucas1p1330d8561386c3cf2bb457bc22c0d99a8@eucas1p1.samsung.com>
2018-01-26 11:14 ` [RFC 1/2] arm: cacheflush syscall: process only pages that are in the memory Marek Szyprowski
2018-01-26 11:14   ` Marek Szyprowski
2018-01-26 11:14   ` [RFC 2/2] arm64: compat: " Marek Szyprowski
2018-01-26 11:14     ` Marek Szyprowski
2018-01-26 17:41     ` Catalin Marinas
2018-01-26 17:41       ` Catalin Marinas
2018-01-26 18:02       ` Catalin Marinas
2018-01-26 18:02         ` Catalin Marinas
2018-01-26 11:32   ` [RFC 1/2] arm: " Russell King - ARM Linux
2018-01-26 11:32     ` Russell King - ARM Linux
2018-01-26 13:30     ` Marek Szyprowski
2018-01-26 13:30       ` Marek Szyprowski
2018-01-26 21:39       ` Russell King - ARM Linux
2018-01-26 21:39         ` Russell King - ARM Linux
2018-01-31  6:03         ` Inki Dae [this message]
2018-01-31  6:03           ` Inki Dae

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5A715C2B.70609@samsung.com \
    --to=inki.dae@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.