Received-SPF: pass (google.com: domain of linux-kernel+bounces-132090-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 4 Apr 2024 13:18:50 -0700
From: Ira Weiny <ira.weiny@intel.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>, Ira Weiny
	<ira.weiny@intel.com>
CC: Dave Jiang <dave.jiang@intel.com>, Davidlohr Bueso <dave@stgolabs.net>,
	Alison Schofield <alison.schofield@intel.com>, Vishal Verma
	<vishal.l.verma@intel.com>, Dan Williams <dan.j.williams@intel.com>,
	<linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Kwangjin Ko
	<kwangjin.ko@sk.com>
Subject: Re: [PATCH] cxl/test: Enhance event testing
Message-ID: <660f0b2ab8484_e7f1629469@iweiny-mobl.notmuch>
References: <20240401-enhance-event-test-v1-1-6669a524ed38@intel.com>
 <20240403164145.00007bcd@Huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240403164145.00007bcd@Huawei.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?RdDNtNIVgoAOvX52PjNRazo9Ei1sQPjaFM4zJfKTXwwhE58ES1HAaefagGwv?=
 =?us-ascii?Q?Tgq7oeR7as3OgmMO/c/LX8xGIqpokRitNidiuHnZYlGYTJ0FvgZazpiDAsk0?=
 =?us-ascii?Q?aHcFQd/3pxllrj2VmseyMH0KZv5Pr2vO4c/+y4D3i3ECwBgEh5S4rRDzu0qc?=
 =?us-ascii?Q?juKcwkGNnZVmDioSjAYe/WcqIATOlu9HXlkaPz2Zz8bILYQ9q1UdQZEIZXVC?=
 =?us-ascii?Q?kAe6ZFB4CUNov0UDwnmMnZL4iRvBexZkRMHj2j1ltfgnohaddbFULg62sP/Z?=
 =?us-ascii?Q?oF/809fimvUTOjnSq+hlnyx14bSdBQnjBosgzSsxTRVxi3Cuk3PjkC0nl3Rb?=
 =?us-ascii?Q?HY5SYwe3JSXcsv8tGdI+mZcZjO0pY/nyAAL8zW2yEezMhayN4BYKU9WdXW1I?=
 =?us-ascii?Q?L+0b3iuMDZZ5/6sTu+kvubISp+EPG9l8nWTnUBjOA4M5+C8z/f2PpBadF0cK?=
 =?us-ascii?Q?ZnFW1J2KJPMCiH5i3TU2UoYpI+kb7AVKiI14ci7wowJ/YVnbI9PTS1L8dMo4?=
 =?us-ascii?Q?PU4lqUcIen+FD2I6JTGNrjKpwH5XVlWfC+pemR8osPWXCiETVlDH2jY0EoQ1?=
 =?us-ascii?Q?nrjSjKk0ySC2T9184r+G3aUj+Yc7IQVukxHTvFfmGOWkABbCBY1rI7ghXoRf?=
 =?us-ascii?Q?bRnl69Tsr2+cO8DekzEJIhtmtrgF0YSAJ+WD856y8MIDMD8J/eMQlkyu+WLh?=
 =?us-ascii?Q?oGLq965tPLuQgpKnsugZ1xW0i/ErlOMDso9nQTRJSfL8n4ZCad+w/RuFiCNH?=
 =?us-ascii?Q?pwT2ykpIl9ePH4M74AXoIa6oe8rqLU6Mp7pFWVeq+788g+9V3YfwjHD/0EWk?=
 =?us-ascii?Q?G9MhWkn0wVEyLhZsaUTFMyowoCNxMusU7HEtN9CdXwCvZ+LNU3Ns/harnuIK?=
 =?us-ascii?Q?vXbnlpk6i9H/NS/x/RSWcxPSRQ04REgm5oJXubwpbCGgYm2v7HhQqkUcOLMV?=
 =?us-ascii?Q?gBl4OslFdu4jsa0zpCc3DjHlG1gVD7FOYrryuxewpJa+luvkZl63XpmAb/TY?=
 =?us-ascii?Q?ommitvbGBY4kVGtoi3g0b4QdHuxTHT+DI6xmQAUGmUBKvO31R6Eu83OXAz+F?=
 =?us-ascii?Q?ymG/IQlKtX7AJQdOEv8eeH6q/esvL2mHfsfrZj/vPEQytPAR5NCv51IbwX+u?=
 =?us-ascii?Q?mHRgCNAIt1+sylsOkn1QnynvVXtT57ONg41JDLoPqK7PLfl3Smjm01mcwB0h?=
 =?us-ascii?Q?S9eYAY/bmPoYTvdcncr1gIIHsqMsY2GGVYG5E1rXVKiv/WqM8tVkXrePwc/c?=
 =?us-ascii?Q?RXybIOZzbFRWezTIRzw+BXyfHrbTTZ0gDexJVePSM4E2qOLQSlmExmOJqzFI?=
 =?us-ascii?Q?Yg+cLoeyNJ1FNkr9UELAyMWaw+Ab9LIhfBfFine2rnmgIwII8IMven10nUN9?=
 =?us-ascii?Q?jmrsy/X3KEDJMnio4gJiWGCwHqTdwd3/zvZs0YhQAY1TD4lfpi7YuI9VNK+Q?=
 =?us-ascii?Q?fgpzSlXHOepxo9PE6K+5orYWio9q4jXWsR+ujpDySyAY5AhLOO4WaCWF2X8X?=
 =?us-ascii?Q?UL8lOm+f957X+Fs4ebJG4G6y1etHpQC4v2k1zfluVB4dhmKeutPjkHZ/jvRx?=
 =?us-ascii?Q?4LPI5cce0gBuaRhayBz60REgvHlUvnYV3w8ozq+4?=
X-MS-Exchange-CrossTenant-Network-Message-Id: de916046-6808-4a3a-cdce-08dc54e4750a
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2024 20:18:57.5677
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: d1fQ7b87lO3zhfD7UFjoOT8gp5lCkoLAxSTsGh2Pg2mOn9vqPDtGNLEVGXTKhfqK1vINFM1Jp0gNqSDo+/LN4g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4643
X-OriginatorOrg: intel.com

Jonathan Cameron wrote:
> On Mon, 01 Apr 2024 22:31:58 -0700
> Ira Weiny <ira.weiny@intel.com> wrote:
> 
> > An issue was found in the processing of event logs when the output
> > buffer length was not reset.[1]
> > 
> > This bug was not caught with cxl-test for 2 reasons.  First, the test
> > harness mbox_send command [mock_get_event()] does not set the output
> > size based on the amount of data returned like the hardware command
> > does.  Second, the simplistic event log testing always returned the same
> > number of elements per-get command.
> > 
> > Enhance the simulation of the event log mailbox to better match the bug
> > found with real hardware to cover potential regressions.
> > 
> > NOTE: These changes will cause cxl-events.sh in ndctl to fail without
> > the fix from Kwangjin.  However, no changes to the user space test was
> > required.  Therefore ndctl itself will be compatible with old or new
> > kernels once both patches land in the new kernel.
> 
> Good to state how many events are read out (22 I think) as that allows
> reader of this patch to see that it will cycle all the way around, so we
> will get the 4 to 1 reduction between two reads ensuring things work
> correctly when smaller number of events are returned.

It is actually:

num_overflow_expected=1
num_fatal_expected=2
num_failure_expected=16
num_info_expected=3

..because each log is read separately.  But the 16 failures is what tests
this.

> 
> 1, 2, 3, 4, 1, 2, 3, 4, 1, 1 
> I think.

1, 2, 3

> > 
> > [1] Link: https://lore.kernel.org/all/20240401091057.1044-1-kwangjin.ko@sk.com/
> > 
> > Cc: Kwangjin Ko <kwangjin.ko@sk.com>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Thanks, Dave do you want me to reroll with an updated commit message?

Ira

> 
> 
> > ---
> >  tools/testing/cxl/test/mem.c | 19 +++++++++++++++----
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> > index 35ee41e435ab..6584443144de 100644
> > --- a/tools/testing/cxl/test/mem.c
> > +++ b/tools/testing/cxl/test/mem.c
> > @@ -127,7 +127,7 @@ static struct {
> >  #define CXL_TEST_EVENT_CNT_MAX 15
> >  
> >  /* Set a number of events to return at a time for simulation.  */
> > -#define CXL_TEST_EVENT_CNT 3
> > +#define CXL_TEST_EVENT_RET_MAX 4
> >  
> >  struct mock_event_log {
> >  	u16 clear_idx;
> > @@ -222,6 +222,12 @@ static void mes_add_event(struct mock_event_store *mes,
> >  	log->nr_events++;
> >  }
> >  
> > +/*
> > + * Vary the number of events returned to simulate events occuring while the
> > + * logs are being read.
> > + */
> > +static int ret_limit = 0;
> > +
> >  static int mock_get_event(struct device *dev, struct cxl_mbox_cmd *cmd)
> >  {
> >  	struct cxl_get_event_payload *pl;
> > @@ -233,14 +239,18 @@ static int mock_get_event(struct device *dev, struct cxl_mbox_cmd *cmd)
> >  	if (cmd->size_in != sizeof(log_type))
> >  		return -EINVAL;
> >  
> > -	if (cmd->size_out < struct_size(pl, records, CXL_TEST_EVENT_CNT))
> > +	ret_limit = (ret_limit + 1) % CXL_TEST_EVENT_RET_MAX;
> > +	if (!ret_limit)
> > +		ret_limit = 1;
> > +
> > +	if (cmd->size_out < struct_size(pl, records, ret_limit))
> >  		return -EINVAL;
> >  
> >  	log_type = *((u8 *)cmd->payload_in);
> >  	if (log_type >= CXL_EVENT_TYPE_MAX)
> >  		return -EINVAL;
> >  
> > -	memset(cmd->payload_out, 0, cmd->size_out);
> > +	memset(cmd->payload_out, 0, struct_size(pl, records, 0));
> >  
> >  	log = event_find_log(dev, log_type);
> >  	if (!log || event_log_empty(log))
> > @@ -248,7 +258,7 @@ static int mock_get_event(struct device *dev, struct cxl_mbox_cmd *cmd)
> >  
> >  	pl = cmd->payload_out;
> >  
> > -	for (i = 0; i < CXL_TEST_EVENT_CNT && !event_log_empty(log); i++) {
> > +	for (i = 0; i < ret_limit && !event_log_empty(log); i++) {
> >  		memcpy(&pl->records[i], event_get_current(log),
> >  		       sizeof(pl->records[i]));
> >  		pl->records[i].event.generic.hdr.handle =
> > @@ -256,6 +266,7 @@ static int mock_get_event(struct device *dev, struct cxl_mbox_cmd *cmd)
> >  		log->cur_idx++;
> >  	}
> >  
> > +	cmd->size_out = struct_size(pl, records, i);
> >  	pl->record_count = cpu_to_le16(i);
> >  	if (!event_log_empty(log))
> >  		pl->flags |= CXL_GET_EVENT_FLAG_MORE_RECORDS;
> > 
> > ---
> > base-commit: 8d025e2092e29bfd13e56c78e22af25fac83c8ec
> > change-id: 20240401-enhance-event-test-cebeff5189cd
> > 
> > Best regards,
>