From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="hRYI5zWZ" Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.31]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F0C3CAA for ; Tue, 12 Dec 2023 18:13:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1702433624; x=1733969624; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=1PMAyvKAy7zoBw3d0krkf1EWrCx1NHuXddCqDc8iX9o=; b=hRYI5zWZ/qNR5eg3/t+JWaVz+9EVwyytvCevi+o/nqd7MCuwCgCH6GtV a7M2vr63K3MgvGXSnc3IxJrxsemBf8VKrbgfiXLBw01acLLax15DyTBlL 8XBCOEB1oVQIPgFXnvVK94JuAd2M7XKAKVwR/WVoSxfX+d1y8Sr9njU0P wIqIcq0H20j0Nv+5pr2PmYuv2FGBHXnXB+qcb0gkbTqflqhbSC3OnNuyk PX9ZGXfkKBzZwYrutWbwvOfiHHKt0E3ZAP71eH5Wn5gceJIJIDA+dlUk2 KTyO9DqIjU0WKyJlZUj0h0G3q6/eVTXjstwyIoyxYgJP6B/XlnAJE3i38 A==; X-IronPort-AV: E=McAfee;i="6600,9927,10922"; a="459220432" X-IronPort-AV: E=Sophos;i="6.04,271,1695711600"; d="scan'208";a="459220432" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Dec 2023 18:13:44 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10922"; a="917496528" X-IronPort-AV: E=Sophos;i="6.04,271,1695711600"; d="scan'208";a="917496528" Received: from aschofie-mobl2.amr.corp.intel.com (HELO aschofie-mobl2) ([10.209.111.12]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Dec 2023 18:13:43 -0800 Date: Tue, 12 Dec 2023 18:13:42 -0800 From: Alison Schofield To: Dan Williams Cc: Vishal Verma , nvdimm@lists.linux.dev, linux-cxl@vger.kernel.org Subject: Re: [ndctl PATCH v5 3/5] cxl/list: collect and parse the poison list records Message-ID: References: <65714c6b472ee_269bd29417@dwillia2-mobl3.amr.corp.intel.com.notmuch> Precedence: bulk X-Mailing-List: linux-cxl@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <65714c6b472ee_269bd29417@dwillia2-mobl3.amr.corp.intel.com.notmuch> On Wed, Dec 06, 2023 at 08:39:07PM -0800, Dan Williams wrote: > alison.schofield@ wrote: > > From: Alison Schofield > > snip > > + struct json_object *jerrors, *jpoison, *jobj = NULL; > > + struct jlist_node *jnode, *next; > > + struct event_ctx ectx = { > > + .event_name = "cxl_poison", > > + .event_pid = getpid(), > > + .system = "cxl", > > + }; > > + int rc, count = 0; > > + > > + list_head_init(&ectx.jlist_head); > > + rc = cxl_parse_events(inst, &ectx); > > This pattern really feels like it wants a cxl_for_each_event() -style > helper rather than require the end caller to open code list usage. > Basically cxl_parse_events() is a helper that should stay local to > cxl/monitor.c. This new cxl_for_each_event() would become used > internally by cxl_parse_events() and let > util_cxl_poison_events_to_json() do its own per-event iteration. > snip & concat'ing your next comment: > So we're building a json_object internal to cxl_parse_events() only to > turn around and extract details out of that object that tell us this > event was not of interest, or to create yet another json object? > > I think this implementation has a chance to be significantly less > complicated if the event list can be iterated directly without this > temporary json_object parsing. DaveJ actually already implemented a method to include a 'private' parsing function in the event_ctx structure. I didn't use it, but but rather used the generic cxl_event_to_json helper, and then parsed that all over again to refine for poison list output. I think reorganizing w a private event_ctx->parse_event will streamline as you suggest. Alison