public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Yuntao Wang <yuntao.wang@linux.dev>
To: tglx@linutronix.de
Cc: bp@alien8.de, daniel.sneddon@linux.intel.com,
	dave.hansen@linux.intel.com, hpa@zytor.com,
	linux-kernel@vger.kernel.org, mingo@redhat.com,
	peterz@infradead.org, thorsten.blum@toblux.com,
	tony.luck@intel.com, x86@kernel.org, yuntao.wang@linux.dev
Subject: Re: [PATCH] x86/apic: Fix the issues in x2apic_disable()
Date: Mon, 12 Aug 2024 23:53:58 +0800	[thread overview]
Message-ID: <20240812155358.890025-1-yuntao.wang@linux.dev> (raw)
In-Reply-To: <87a5hhsspm.ffs@tglx>

On Mon, 12 Aug 2024 16:48:05 +0200, Thomas Gleixner <tglx@linutronix.de> wrote:

> On Mon, Aug 12 2024 at 18:08, Yuntao Wang wrote:
> >  static __init void x2apic_disable(void)
> >  {
> > -	u32 x2apic_id, state = x2apic_state;
> > +	u32 x2apic_id;
> >  
> > -	x2apic_mode = 0;
> > -	x2apic_state = X2APIC_DISABLED;
> > -
> > -	if (state != X2APIC_ON)
> > -		return;
> > +	if (x2apic_state < X2APIC_ON)
> > +		goto out;
> 
> There is no point in overwriting the state in case it is < ON, no?

Are you saying that we should replace 'goto out' with a return statement?

However, when x2apic_disable() is called, it's possible that x2apic_state
is X2APIC_OFF. In that case, we should set x2apic_state to X2APIC_DISABLED.

So, I think overwriting the state is necessary.

Thanks,
Yuntao

> Thanks,
> 
>         tglx

  reply	other threads:[~2024-08-12 15:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-12 10:08 [PATCH] x86/apic: Fix the issues in x2apic_disable() Yuntao Wang
2024-08-12 14:48 ` Thomas Gleixner
2024-08-12 15:53   ` Yuntao Wang [this message]
2024-08-12 18:00     ` Thomas Gleixner
2024-08-13  1:48       ` [PATCH v2] " Yuntao Wang
2024-08-13 13:21         ` [tip: x86/urgent] x86/apic: Make x2apic_disable() work correctly tip-bot2 for Yuntao Wang

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=20240812155358.890025-1-yuntao.wang@linux.dev \
    --to=yuntao.wang@linux.dev \
    --cc=bp@alien8.de \
    --cc=daniel.sneddon@linux.intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=thorsten.blum@toblux.com \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.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