Linux CXL
 help / color / mirror / Atom feed
From: Ira Weiny <ira.weiny@intel.com>
To: Huaisheng Ye <huaisheng.ye@intel.com>,
	Ira Weiny <ira.weiny@intel.com>, <dan.j.williams@intel.com>
Cc: <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<pei.p.jia@intel.com>
Subject: Re: [PATCH] cxl/core/mbox: get next_persistent_bytes by next_persistent_cap
Date: Wed, 6 Dec 2023 20:51:31 -0800	[thread overview]
Message-ID: <65714f53394a3_1b2839294ce@iweiny-mobl.notmuch> (raw)
In-Reply-To: <5fed153a-6668-c980-c573-3a31077c52ac@intel.com>

Huaisheng Ye wrote:
> 
> 
> On 2023-12-06 05:48, Ira Weiny wrote:
> > Huaisheng Ye wrote:
> >> According to CXL 2.0 8.2.9.5.2.1 table 176, the next Persistent
> >> Bytes should be calculated by next Persistent Capacity.
> >>
> >> Signed-off-by: Huaisheng Ye <huaisheng.ye@intel.com>
> > Do we have a fixes tag for this?  Was there a bug associated with this
> > find?
> >
> > Ira
> 
> Hi Ira,

Hey...  First off thanks for the patch.  This is obviously something which
is wrong.  I'm just trying to get more details about how much work should
be done to fix this.

> 
> This bug could be found in cxl-next branch.

This looks like it has been a bug for a while.

59f8d1510739e   (Dan Williams   2023-06-14 18:30:02 -0700       1071) mds->next_persistent_bytes =        
4faf31b43468c   (Dan Williams   2021-09-08 22:12:32 -0700       1072) le64_to_cpu(pi.next_volatile_cap) * CXL_CAPACITY_MULTIPLIER;

Both of those commits were refactoring so the fixes goes back a bit more.

My question is: what problem is this causing for the user and should we ID
which commit's this fixes for potential backporting to stable kernels?

Do you have that information?

> It looks like just cxl-pci would calculate next_persistent_bytes during
> probe. Afterwards, it will no longer be accessed by current cxl drivers.
> 
> For v78 ndctl, cxl_cmd_partition_get_next_persistent_size is calculated from
> next_persistent, which is correct.

This too seems to be old behavior.

4f588b964dccf   (Alison Schofield       2022-02-22 11:56:03 -0800 4142)cxl_cmd_partition_get_next_persistent_size(struct cxl_cmd *cmd)

> 
> May I have your Reviewed-by?

Not yet.  I want to know should this be backported and what problems this
causes.  The next values are not going to take effect until the next
reboot/reset of the device.  So they are basically informational, Right?.
Is that why you did not add a fixes to the patch?  If so mention that a
fixes is not needed.  If not, explain why this is something a user might
see and lets figure out what fixes tags to add so this gets backported.

Thanks,
Ira

      reply	other threads:[~2023-12-07  4:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-01 16:40 [PATCH] cxl/core/mbox: get next_persistent_bytes by next_persistent_cap Huaisheng Ye
2023-12-05 21:48 ` Ira Weiny
2023-12-07  2:29   ` Huaisheng Ye
2023-12-07  4:51     ` Ira Weiny [this message]

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=65714f53394a3_1b2839294ce@iweiny-mobl.notmuch \
    --to=ira.weiny@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=huaisheng.ye@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pei.p.jia@intel.com \
    /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