linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: benh@kernel.crashing.org, mpe@ellerman.id.au, anton@samba.org,
	paulus@samba.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v5 08/12] powerpc: Add support to mask perf interrupts and replay them
Date: Thu, 5 Jan 2017 16:05:38 +1000	[thread overview]
Message-ID: <20170105160538.1fdb1b3c@roar.ozlabs.ibm.com> (raw)
In-Reply-To: <fff30cf9-a589-2a6d-ec87-40a92fb084a2@linux.vnet.ibm.com>

On Wed, 4 Jan 2017 22:21:05 +0530
Madhavan Srinivasan <maddy@linux.vnet.ibm.com> wrote:

> On Wednesday 04 January 2017 06:08 PM, Nicholas Piggin wrote:
> > On Wed,  4 Jan 2017 17:19:46 +0530
> > Madhavan Srinivasan <maddy@linux.vnet.ibm.com> wrote:
> >  
> >> @@ -134,7 +137,7 @@ static inline bool arch_irqs_disabled(void)
> >>   	_was_enabled = local_paca->soft_enabled;	\
> >>   	local_paca->soft_enabled = IRQ_DISABLE_MASK_LINUX;\
> >>   	local_paca->irq_happened |= PACA_IRQ_HARD_DIS;	\
> >> -	if (!(_was_enabled & IRQ_DISABLE_MASK_LINUX))	\
> >> +	if (!(_was_enabled & IRQ_DISABLE_MASK_ALL))	\
> >>   		trace_hardirqs_off();			\
> >>   } while(0)  
> > Hang on, maybe there's some confusion about this. trace_hardirqs_off() is
> > for Linux irqs (i.e., local_irq_disable()), so that should continue to
> > test just the LINUX mask I think. Otherwise this
> >
> >      powerpc_local_pmu_disable();
> >      hard_irq_disable();  
> 
> Currently we set both bits for pmu soft disable
> 
>                  flags = 
> soft_disabled_mask_or_return(IRQ_DISABLE_MASK_LINUX | \
> IRQ_DISABLE_MASK_PMU);                  \
> 
> So yes in the above seq, we will miss the pmu bit. But since 
> trace_hardirqs_off()
> is for _LINUX, instead will it not be safer to OR it?
> 
>          local_paca->soft_disabled_mask |= IRQ_DISABLE_MASK_LINUX;\

I would just say set all unconditionally. Despite "Linux" IRQs being the
special case, I would much prefer it to be written as much as possible
with all mask bits being equal, and then cases where LINUX bits need to
be handled differently built on that.

We might end up adding more bits, or even splitting the current single
LINUX bit into several others.

hard disable effectively masks all the interrupts, so then it seems we
should set them all in the disabled mask rather than just one. The
special case would then be just the test for the LINUX bit when deciding
whether to call trace_hardirqs_off().

Thanks,
Nick

  reply	other threads:[~2017-01-05  6:05 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-04 11:49 [PATCH v5 00/12]powerpc: "paca->soft_enabled" based local atomic operation implementation Madhavan Srinivasan
2017-01-04 11:49 ` [PATCH v5 01/12] powerpc: Add #defs for paca->soft_enabled flags Madhavan Srinivasan
2017-01-04 11:49 ` [PATCH v5 02/12] powerpc: move set_soft_enabled() and rename Madhavan Srinivasan
2017-01-04 11:49 ` [PATCH v5 03/12] powerpc: Use soft_enabled_set api to update paca->soft_enabled Madhavan Srinivasan
2017-01-04 11:49 ` [PATCH v5 04/12] powerpc: Add soft_enabled manipulation functions Madhavan Srinivasan
2017-01-04 11:49 ` [PATCH v5 05/12] powerpc: reverse the soft_enable logic Madhavan Srinivasan
2017-01-04 11:49 ` [PATCH v5 06/12] powerpc: Avoid using EXCEPTION_PROLOG_1 macro in MASKABLE_* Madhavan Srinivasan
2017-01-04 11:49 ` [PATCH v5 07/12] Add support to take additional parameter in MASKABLE_* macro Madhavan Srinivasan
2017-01-04 11:49 ` [PATCH v5 08/12] powerpc: Add support to mask perf interrupts and replay them Madhavan Srinivasan
2017-01-04 12:38   ` Nicholas Piggin
2017-01-04 16:51     ` Madhavan Srinivasan
2017-01-05  6:05       ` Nicholas Piggin [this message]
2017-01-04 11:49 ` [PATCH v5 09/12] powerpc:Add new kconfig IRQ_DEBUG_SUPPORT Madhavan Srinivasan
2017-01-04 11:49 ` [PATCH v5 10/12] powerpc: Add new set of soft_enabled_ functions Madhavan Srinivasan
2017-01-04 11:49 ` [PATCH v5 11/12] powerpc: rewrite local_t using soft_irq Madhavan Srinivasan
2017-01-04 11:49 ` [PATCH v5 12/12] powerpc: Rename soft_enabled to soft_disabled_mask Madhavan Srinivasan

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=20170105160538.1fdb1b3c@roar.ozlabs.ibm.com \
    --to=npiggin@gmail.com \
    --cc=anton@samba.org \
    --cc=benh@kernel.crashing.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.vnet.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.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 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).