All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Ian Munsie <imunsie@au1.ibm.com>, mpe <mpe@ellerman.id.au>
Cc: cbe-oss-dev <cbe-oss-dev@lists.ozlabs.org>,
	mikey <mikey@neuling.org>, arnd <arnd@arndb.de>,
	jk <jk@ozlabs.org>, greg <greg@kroah.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@ozlabs.org>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	imunsie <imunsie@au1.ibm.com>, anton <anton@samba.org>
Subject: Re: CXL: Fix PSL error due to duplicate segment table entries
Date: Mon, 27 Oct 2014 17:41:00 +1100 (AEDT)	[thread overview]
Message-ID: <20141027064100.D12B514007D@ozlabs.org> (raw)
In-Reply-To: <1414383875-20835-1-git-send-email-imunsie@au.ibm.com>

On Mon, 2014-27-10 at 04:24:35 UTC, Ian Munsie wrote:
> From: Ian Munsie <imunsie@au1.ibm.com>
> 
> In certain circumstances the PSL can send an interrupt for a segment

Define PSL before using it please.

> miss that the kernel has already handled. This can happen if multiple
> translations for the same segment are queued in the PSL before the
> kernel has restarted the first translation.
> 
> The CXL driver did not expect this situation and did not check if a

does not and does not, you haven't patched it yet.

> segment had already been handled. This could cause a duplicate segment
> table entry which in turn caused a PSL error taking down the card.
> 
> This patch fixes the issue by checking for existing entries in the
> segment table that match the segment it is trying to insert to avoid
> inserting duplicate entries.
> 
> Some of the code has been refactored to simplify it - the segment table
> hash has been moved from cxl_load_segment to find_free_sste where it is

Any reason that's not a separate patch?

> used and we have disabled the secondary hash in the segment table to
> reduce the number of entries that need to be tested from 16 to 8. Due to
> the large segment sizes we use it is extremely unlikely that the
> secondary hash would ever have been used in practice, so this should not
> have any negative impacts and may even improve performance.

Any reason that's not a separate patch?

> copro_calculate_slb will now mask the ESID by the correct mask for 1T vs

Didn't, but will after this patch?

> 256M segments. This has no effect by itself as the extra bits were
> ignored, but it makes debugging the segment table entries easier and
> means that we can directly compare the ESID values for duplicates
> without needing to worry about masking in the comparison.

Separate patch?

cheers

WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Ian Munsie <imunsie@au1.ibm.com>, mpe <mpe@ellerman.id.au>
Cc: cbe-oss-dev <cbe-oss-dev@lists.ozlabs.org>,
	mikey <mikey@neuling.org>, arnd <arnd@arndb.de>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	greg <greg@kroah.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@ozlabs.org>, anton <anton@samba.org>,
	imunsie <imunsie@au1.ibm.com>, jk <jk@ozlabs.org>
Subject: Re: CXL: Fix PSL error due to duplicate segment table entries
Date: Mon, 27 Oct 2014 17:41:00 +1100 (AEDT)	[thread overview]
Message-ID: <20141027064100.D12B514007D@ozlabs.org> (raw)
In-Reply-To: <1414383875-20835-1-git-send-email-imunsie@au.ibm.com>

On Mon, 2014-27-10 at 04:24:35 UTC, Ian Munsie wrote:
> From: Ian Munsie <imunsie@au1.ibm.com>
> 
> In certain circumstances the PSL can send an interrupt for a segment

Define PSL before using it please.

> miss that the kernel has already handled. This can happen if multiple
> translations for the same segment are queued in the PSL before the
> kernel has restarted the first translation.
> 
> The CXL driver did not expect this situation and did not check if a

does not and does not, you haven't patched it yet.

> segment had already been handled. This could cause a duplicate segment
> table entry which in turn caused a PSL error taking down the card.
> 
> This patch fixes the issue by checking for existing entries in the
> segment table that match the segment it is trying to insert to avoid
> inserting duplicate entries.
> 
> Some of the code has been refactored to simplify it - the segment table
> hash has been moved from cxl_load_segment to find_free_sste where it is

Any reason that's not a separate patch?

> used and we have disabled the secondary hash in the segment table to
> reduce the number of entries that need to be tested from 16 to 8. Due to
> the large segment sizes we use it is extremely unlikely that the
> secondary hash would ever have been used in practice, so this should not
> have any negative impacts and may even improve performance.

Any reason that's not a separate patch?

> copro_calculate_slb will now mask the ESID by the correct mask for 1T vs

Didn't, but will after this patch?

> 256M segments. This has no effect by itself as the extra bits were
> ignored, but it makes debugging the segment table entries easier and
> means that we can directly compare the ESID values for duplicates
> without needing to worry about masking in the comparison.

Separate patch?

cheers

  reply	other threads:[~2014-10-27  6:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-27  4:24 [PATCH] CXL: Fix PSL error due to duplicate segment table entries Ian Munsie
2014-10-27  4:24 ` Ian Munsie
2014-10-27  6:41 ` Michael Ellerman [this message]
2014-10-27  6:41   ` Michael Ellerman
2014-10-28  0:17   ` Ian Munsie
2014-10-28  0:17     ` Ian Munsie
2014-10-27 14:38 ` [PATCH] " Aneesh Kumar K.V
2014-10-27 14:38   ` Aneesh Kumar K.V
2014-10-28  0:20   ` Ian Munsie
2014-10-28  0:20     ` Ian Munsie

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=20141027064100.D12B514007D@ozlabs.org \
    --to=mpe@ellerman.id.au \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=anton@samba.org \
    --cc=arnd@arndb.de \
    --cc=cbe-oss-dev@lists.ozlabs.org \
    --cc=greg@kroah.com \
    --cc=imunsie@au1.ibm.com \
    --cc=jk@ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=mikey@neuling.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.