* [PATCH] arm: cacheflush: get rid of restarting block
@ 2014-11-25 15:18 Vladimir Murzin
2014-11-27 4:02 ` Chanho Min
0 siblings, 1 reply; 5+ messages in thread
From: Vladimir Murzin @ 2014-11-25 15:18 UTC (permalink / raw)
To: linux-arm-kernel
We cannot restart cacheflush safely if a process provides user-defined
signal handler and signal is pending. In this case -EINTR is returned
and it is expected that process re-invokes syscall. However, there are
a few problems with that:
* looks like nobody bothers checking return value from cacheflush
* but if it did, we don't provide the restart address for that, so the
process has to use the same range again
* ...and again, what might lead to looping forever
So, remove cacheflush restarting code and terminate cache flushing
as early as fatal signal is pending.
Cc: stable at vger.kernel.org # 3.12+
Reported-by: Chanho Min <chanho.min@lge.com>
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
---
Hi,
There is my proposal for the problem reported in [1]. Since that
thread has not been progressing much, I'm addressing the problem in
separate patch.
Comments?
[1] https://lkml.org/lkml/2014/11/13/29
arch/arm/include/asm/thread_info.h | 11 -----------
arch/arm/kernel/traps.c | 31 ++-----------------------------
2 files changed, 2 insertions(+), 40 deletions(-)
diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
index fc44d37..ce73ab6 100644
--- a/arch/arm/include/asm/thread_info.h
+++ b/arch/arm/include/asm/thread_info.h
@@ -44,16 +44,6 @@ struct cpu_context_save {
__u32 extra[2]; /* Xscale 'acc' register, etc */
};
-struct arm_restart_block {
- union {
- /* For user cache flushing */
- struct {
- unsigned long start;
- unsigned long end;
- } cache;
- };
-};
-
/*
* low level task data that entry.S needs immediate access to.
* __switch_to() assumes cpu_context follows immediately after cpu_domain.
@@ -79,7 +69,6 @@ struct thread_info {
unsigned long thumbee_state; /* ThumbEE Handler Base register */
#endif
struct restart_block restart_block;
- struct arm_restart_block arm_restart_block;
};
#define INIT_THREAD_INFO(tsk) \
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 0c8b108..8dd03ee 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -533,8 +533,6 @@ static int bad_syscall(int n, struct pt_regs *regs)
return regs->ARM_r0;
}
-static long do_cache_op_restart(struct restart_block *);
-
static inline int
__do_cache_op(unsigned long start, unsigned long end)
{
@@ -543,24 +541,8 @@ __do_cache_op(unsigned long start, unsigned long end)
do {
unsigned long chunk = min(PAGE_SIZE, end - start);
- if (signal_pending(current)) {
- struct thread_info *ti = current_thread_info();
-
- ti->restart_block = (struct restart_block) {
- .fn = do_cache_op_restart,
- };
-
- ti->arm_restart_block = (struct arm_restart_block) {
- {
- .cache = {
- .start = start,
- .end = end,
- },
- },
- };
-
- return -ERESTART_RESTARTBLOCK;
- }
+ if (fatal_signal_pending(current))
+ return 0;
ret = flush_cache_user_range(start, start + chunk);
if (ret)
@@ -573,15 +555,6 @@ __do_cache_op(unsigned long start, unsigned long end)
return 0;
}
-static long do_cache_op_restart(struct restart_block *unused)
-{
- struct arm_restart_block *restart_block;
-
- restart_block = ¤t_thread_info()->arm_restart_block;
- return __do_cache_op(restart_block->cache.start,
- restart_block->cache.end);
-}
-
static inline int
do_cache_op(unsigned long start, unsigned long end, int flags)
{
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] arm: cacheflush: get rid of restarting block
2014-11-25 15:18 [PATCH] arm: cacheflush: get rid of restarting block Vladimir Murzin
@ 2014-11-27 4:02 ` Chanho Min
2014-11-27 10:03 ` Will Deacon
0 siblings, 1 reply; 5+ messages in thread
From: Chanho Min @ 2014-11-27 4:02 UTC (permalink / raw)
To: linux-arm-kernel
> -----Original Message-----
> From: Vladimir Murzin [mailto:vladimir.murzin at arm.com]
> Sent: Wednesday, November 26, 2014 12:19 AM
> To: linux-arm-kernel at lists.infradead.org; linux at arm.linux.org.uk
> Cc: chanho.min at lge.com; will.deacon at arm.com; stable at vger.kernel.org
> Subject: [PATCH] arm: cacheflush: get rid of restarting block
>
> We cannot restart cacheflush safely if a process provides user-defined
> signal handler and signal is pending. In this case -EINTR is returned
> and it is expected that process re-invokes syscall. However, there are
> a few problems with that:
> * looks like nobody bothers checking return value from cacheflush
> * but if it did, we don't provide the restart address for that, so the
> process has to use the same range again
> * ...and again, what might lead to looping forever
>
> So, remove cacheflush restarting code and terminate cache flushing
> as early as fatal signal is pending.
>
> Cc: stable at vger.kernel.org # 3.12+
> Reported-by: Chanho Min <chanho.min@lge.com>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
> ---
> Hi,
>
> There is my proposal for the problem reported in [1]. Since that
> thread has not been progressing much, I'm addressing the problem in
> separate patch.
>
> Comments?
Thanks, We hope this to be applied.
>
> [1] https://lkml.org/lkml/2014/11/13/29
>
> arch/arm/include/asm/thread_info.h | 11 -----------
> arch/arm/kernel/traps.c | 31 ++-----------------------------
> 2 files changed, 2 insertions(+), 40 deletions(-)
>
> diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
> index fc44d37..ce73ab6 100644
> --- a/arch/arm/include/asm/thread_info.h
> +++ b/arch/arm/include/asm/thread_info.h
> @@ -44,16 +44,6 @@ struct cpu_context_save {
> __u32 extra[2]; /* Xscale 'acc' register, etc */
> };
>
> -struct arm_restart_block {
> - union {
> - /* For user cache flushing */
> - struct {
> - unsigned long start;
> - unsigned long end;
> - } cache;
> - };
> -};
> -
> /*
> * low level task data that entry.S needs immediate access to.
> * __switch_to() assumes cpu_context follows immediately after cpu_domain.
> @@ -79,7 +69,6 @@ struct thread_info {
> unsigned long thumbee_state; /* ThumbEE Handler Base register */
> #endif
> struct restart_block restart_block;
> - struct arm_restart_block arm_restart_block;
> };
>
> #define INIT_THREAD_INFO(tsk) \
> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> index 0c8b108..8dd03ee 100644
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -533,8 +533,6 @@ static int bad_syscall(int n, struct pt_regs *regs)
> return regs->ARM_r0;
> }
>
> -static long do_cache_op_restart(struct restart_block *);
> -
> static inline int
> __do_cache_op(unsigned long start, unsigned long end)
> {
> @@ -543,24 +541,8 @@ __do_cache_op(unsigned long start, unsigned long end)
> do {
> unsigned long chunk = min(PAGE_SIZE, end - start);
>
> - if (signal_pending(current)) {
> - struct thread_info *ti = current_thread_info();
> -
> - ti->restart_block = (struct restart_block) {
> - .fn = do_cache_op_restart,
> - };
> -
> - ti->arm_restart_block = (struct arm_restart_block) {
> - {
> - .cache = {
> - .start = start,
> - .end = end,
> - },
> - },
> - };
> -
> - return -ERESTART_RESTARTBLOCK;
> - }
> + if (fatal_signal_pending(current))
> + return 0;
>
> ret = flush_cache_user_range(start, start + chunk);
> if (ret)
> @@ -573,15 +555,6 @@ __do_cache_op(unsigned long start, unsigned long end)
> return 0;
> }
>
> -static long do_cache_op_restart(struct restart_block *unused)
> -{
> - struct arm_restart_block *restart_block;
> -
> - restart_block = ¤t_thread_info()->arm_restart_block;
> - return __do_cache_op(restart_block->cache.start,
> - restart_block->cache.end);
> -}
> -
> static inline int
> do_cache_op(unsigned long start, unsigned long end, int flags)
> {
> --
> 1.7.9.5
Chanho
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] arm: cacheflush: get rid of restarting block
2014-11-27 4:02 ` Chanho Min
@ 2014-11-27 10:03 ` Will Deacon
2014-11-27 10:43 ` Vladimir Murzin
0 siblings, 1 reply; 5+ messages in thread
From: Will Deacon @ 2014-11-27 10:03 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 27, 2014 at 04:02:24AM +0000, Chanho Min wrote:
> > -----Original Message-----
> > From: Vladimir Murzin [mailto:vladimir.murzin at arm.com]
> > Sent: Wednesday, November 26, 2014 12:19 AM
> > To: linux-arm-kernel at lists.infradead.org; linux at arm.linux.org.uk
> > Cc: chanho.min at lge.com; will.deacon at arm.com; stable at vger.kernel.org
> > Subject: [PATCH] arm: cacheflush: get rid of restarting block
> >
> > We cannot restart cacheflush safely if a process provides user-defined
> > signal handler and signal is pending. In this case -EINTR is returned
> > and it is expected that process re-invokes syscall. However, there are
> > a few problems with that:
> > * looks like nobody bothers checking return value from cacheflush
> > * but if it did, we don't provide the restart address for that, so the
> > process has to use the same range again
> > * ...and again, what might lead to looping forever
> >
> > So, remove cacheflush restarting code and terminate cache flushing
> > as early as fatal signal is pending.
> >
> > Cc: stable at vger.kernel.org # 3.12+
> > Reported-by: Chanho Min <chanho.min@lge.com>
> > Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> > Acked-by: Will Deacon <will.deacon@arm.com>
> > ---
> > Hi,
> >
> > There is my proposal for the problem reported in [1]. Since that
> > thread has not been progressing much, I'm addressing the problem in
> > separate patch.
> >
> > Comments?
> Thanks, We hope this to be applied.
Ok, good. In which case, can you put it into the patch system please,
Vladimir?
Will
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] arm: cacheflush: get rid of restarting block
2014-11-27 10:03 ` Will Deacon
@ 2014-11-27 10:43 ` Vladimir Murzin
2014-11-27 10:45 ` Vladimir Murzin
0 siblings, 1 reply; 5+ messages in thread
From: Vladimir Murzin @ 2014-11-27 10:43 UTC (permalink / raw)
To: linux-arm-kernel
On 27/11/14 10:03, Will Deacon wrote:
> On Thu, Nov 27, 2014 at 04:02:24AM +0000, Chanho Min wrote:
>>> -----Original Message-----
>>> From: Vladimir Murzin [mailto:vladimir.murzin at arm.com]
>>> Sent: Wednesday, November 26, 2014 12:19 AM
>>> To: linux-arm-kernel at lists.infradead.org; linux at arm.linux.org.uk
>>> Cc: chanho.min at lge.com; will.deacon at arm.com; stable at vger.kernel.org
>>> Subject: [PATCH] arm: cacheflush: get rid of restarting block
>>>
>>> We cannot restart cacheflush safely if a process provides user-defined
>>> signal handler and signal is pending. In this case -EINTR is returned
>>> and it is expected that process re-invokes syscall. However, there are
>>> a few problems with that:
>>> * looks like nobody bothers checking return value from cacheflush
>>> * but if it did, we don't provide the restart address for that, so the
>>> process has to use the same range again
>>> * ...and again, what might lead to looping forever
>>>
>>> So, remove cacheflush restarting code and terminate cache flushing
>>> as early as fatal signal is pending.
>>>
>>> Cc: stable at vger.kernel.org # 3.12+
>>> Reported-by: Chanho Min <chanho.min@lge.com>
>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>>> Acked-by: Will Deacon <will.deacon@arm.com>
>>> ---
>>> Hi,
>>>
>>> There is my proposal for the problem reported in [1]. Since that
>>> thread has not been progressing much, I'm addressing the problem in
>>> separate patch.
>>>
>>> Comments?
>> Thanks, We hope this to be applied.
>
> Ok, good. In which case, can you put it into the patch system please,
> Vladimir?
Already in patch system as 8226/1.
Vladimir
>
> Will
>
>
>
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] arm: cacheflush: get rid of restarting block
2014-11-27 10:43 ` Vladimir Murzin
@ 2014-11-27 10:45 ` Vladimir Murzin
0 siblings, 0 replies; 5+ messages in thread
From: Vladimir Murzin @ 2014-11-27 10:45 UTC (permalink / raw)
To: linux-arm-kernel
On 27/11/14 10:43, Vladimir Murzin wrote:
> On 27/11/14 10:03, Will Deacon wrote:
>> On Thu, Nov 27, 2014 at 04:02:24AM +0000, Chanho Min wrote:
>>>> -----Original Message-----
>>>> From: Vladimir Murzin [mailto:vladimir.murzin at arm.com]
>>>> Sent: Wednesday, November 26, 2014 12:19 AM
>>>> To: linux-arm-kernel at lists.infradead.org; linux at arm.linux.org.uk
>>>> Cc: chanho.min at lge.com; will.deacon at arm.com; stable at vger.kernel.org
>>>> Subject: [PATCH] arm: cacheflush: get rid of restarting block
>>>>
>>>> We cannot restart cacheflush safely if a process provides user-defined
>>>> signal handler and signal is pending. In this case -EINTR is returned
>>>> and it is expected that process re-invokes syscall. However, there are
>>>> a few problems with that:
>>>> * looks like nobody bothers checking return value from cacheflush
>>>> * but if it did, we don't provide the restart address for that, so the
>>>> process has to use the same range again
>>>> * ...and again, what might lead to looping forever
>>>>
>>>> So, remove cacheflush restarting code and terminate cache flushing
>>>> as early as fatal signal is pending.
>>>>
>>>> Cc: stable at vger.kernel.org # 3.12+
>>>> Reported-by: Chanho Min <chanho.min@lge.com>
>>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>>>> Acked-by: Will Deacon <will.deacon@arm.com>
>>>> ---
>>>> Hi,
>>>>
>>>> There is my proposal for the problem reported in [1]. Since that
>>>> thread has not been progressing much, I'm addressing the problem in
>>>> separate patch.
>>>>
>>>> Comments?
>>> Thanks, We hope this to be applied.
>>
>> Ok, good. In which case, can you put it into the patch system please,
>> Vladimir?
>
> Already in patch system as 8226/1.
>
> Vladimir
>
Sorry for banner - I should have checked that I use proper smtp server :(
Vladimir
>>
>> Will
>>
>>
>>
>
>
> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>
> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782
>
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-11-27 10:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-25 15:18 [PATCH] arm: cacheflush: get rid of restarting block Vladimir Murzin
2014-11-27 4:02 ` Chanho Min
2014-11-27 10:03 ` Will Deacon
2014-11-27 10:43 ` Vladimir Murzin
2014-11-27 10:45 ` Vladimir Murzin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).