Received-SPF: pass (google.com: domain of linux-kernel+bounces-20297-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Mon, 8 Jan 2024 18:48:48 -0800
From: Ira Weiny <ira.weiny@intel.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>, Ira Weiny
	<ira.weiny@intel.com>
CC: fan <nifan.cxl@gmail.com>, Dave Jiang <dave.jiang@intel.com>,
	<linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Huai-Cheng Kuo
	<hchkuo@avery-design.com.tw>
Subject: Re: [PATCH v2 1/2] cxl/cdat: Handle cdat table build errors
Message-ID: <659cb410470dc_8dbbe29426@iweiny-mobl.notmuch>
References: <20231117-fix-cdat-cs-v2-0-715399976d4d@intel.com>
 <20231117-fix-cdat-cs-v2-1-715399976d4d@intel.com>
 <ZYHWZjsnQwcBEDpd@debian>
 <658346b553a01_34d57294d4@iweiny-mobl.notmuch>
 <20240108150318.00006eba@Huawei.com>
 <659c1d888a5fc_8737f29495@iweiny-mobl.notmuch>
 <20240108180042.00005443@Huawei.com>
 <20240108180553.00001577@Huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240108180553.00001577@Huawei.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?0fsw+/KmDyhlXpL7C5ipJgBMvGkcfp+taL3zERKLpuKyBrvRmV8AdXhj9QJ2?=
 =?us-ascii?Q?qNvfYHeomA4gGnvhV7w/uze9En4Oaaca2Rg00RJNzUdbWomNufCDXUj5NcAZ?=
 =?us-ascii?Q?5nRkSvZfSVjazQyqRvYzxVUPJmLCVhaRBIk9yCVo6I/bQkjXAvhaYpcuFXpT?=
 =?us-ascii?Q?1/08TMo+VibbP+0wDoafWaUHdt2IUSZ22aD6D3eWykf4/hUdOcOSbJhM83OO?=
 =?us-ascii?Q?6o6oy7wxJxp6Cq+s54K+bAK4HnOVu/7WawinswM8f2y2A0G6DpQxaDW4LVel?=
 =?us-ascii?Q?9zsdibmhAqOa9fI93yz9194mt0jO/FMRyIDBqERtRM7UKDCeNrZEvUEHyoxi?=
 =?us-ascii?Q?tQRz2TmD7dy+XmSkEgdmDyd87uR8vnHYB7CT3yw8rHyzKEdE8U+ilx87IyL/?=
 =?us-ascii?Q?PWZt+CMqckPfYIDv3ZHaTJCXHNCkNONvfde27hQpdg6OcuOmhb7WC6D/sdE6?=
 =?us-ascii?Q?HJiudEwYPxe/z/U5N6MaSLeC3lXTUHuMvRN+CGNiY3Y8TQRcjRN8NW0gjQAq?=
 =?us-ascii?Q?tLw+eAZpkSnXOzF5bhjNE09XYtGQiUnUgtnvLr1nIpCqSaP8RLhfmcJq01p9?=
 =?us-ascii?Q?dUOCl/JrYk/AxIAMJEPP/6nyrr8DCx7znuRAT/L6Au2CdVLCVJV0nk7YUJuK?=
 =?us-ascii?Q?0u+ui3NuOGrgpznEdfB1Zs5xg7ozYzyukn+OTDOVNxYPPEtNKDbGqyEwduY+?=
 =?us-ascii?Q?wQYISTqBYvx0L95jRL2F5FWR17wYSQOTLIkb/1Piyl2TLEKZpdBA799vGwqf?=
 =?us-ascii?Q?5kcWSmt8MSJuBxEovPlItaG3zZj/23/2beV7Qj4UDSDoCA+R8y4qA2FXN88p?=
 =?us-ascii?Q?VxHZsZlRKztfoxDZAPNak/XAkLnL9gds/TCBY2WqO6i2Zj/fwMznOIpBK2Fy?=
 =?us-ascii?Q?AM/M1LeCMXmDhLJAn55KhtKHIdIt4C8Drl+gadW/mI9u2D0dwDnNe6+Pf0O1?=
 =?us-ascii?Q?Zi8kiDL1ZmVXN810C1BbbpQCvJW8gvuV/bp1aArmhGZi7G3tJkQExjEZEVNi?=
 =?us-ascii?Q?jXu48b6yr+tKMU75s9uCLtp4ijBYADWeDYojBot39GsrtvueKdbI3T2Ptj0o?=
 =?us-ascii?Q?01D4P61WC9/6XxEm7BctucGLwfknEUuxPGGtMi3DY78xRlqGVvZmoUwR8krI?=
 =?us-ascii?Q?kUColzLxkdDFxvT5EJSKt4s0JSUwBj/fu/cd9mNajTimEHjt9AoRoR7mwasc?=
 =?us-ascii?Q?/f8+gmwJ412iHNn1jpZyI2RuAjZxrjoxtz1cQuWWD9uF0SIzzaQfZc9VDzMs?=
 =?us-ascii?Q?jsnLXrRA66CWdHphaaUWAFPpYxzTgb669PsSWbKZw2gYp+l9Yzd5KErT9aVa?=
 =?us-ascii?Q?CWhBOimqBf/5uM/d01As5hRHdpYbTgRQu8I5wg+Gcg8ZGZuXmcSJkO8v9QHC?=
 =?us-ascii?Q?0Utz/OtVSI460qta5Xbb/bZ1ZJSMJV5f6+p+yoa33jjaI9AdgvixPr26cVL3?=
 =?us-ascii?Q?JZA/m/8FzxwSeUmfSSjKSQmnyl9uwhGCZLh9xdsNJSYN/DcnhsvBy+Aya257?=
 =?us-ascii?Q?yjLl6T9qxDfKXZbwdcoqfRq6vYw7tEG30kwV09MGQxecvji1neYiNpCqepV+?=
 =?us-ascii?Q?P1uzJGZgw79aRvQJv9SvGHf86g6tnCfDU8DnOOsNst3nd1CDT2+BqQSCgUOQ?=
 =?us-ascii?Q?bwG+z4A7PzYNAiQBkXM6NTbNVMhwhQ4MMXtmg+7V2dbL?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b1bec5b6-2b99-4041-557b-08dc10bd83f4
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jan 2024 02:48:53.0366
 (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: 3WcSH9J0xbWKSTrhd+d61OFfu9zx7k2HR3Ovob46xl6QmUxt0SjruMWxeryM8YQMBXvYjTqeEN1373ErvLh9hw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR11MB5701
X-OriginatorOrg: intel.com

Jonathan Cameron wrote:

[snip]

> > > > > 
> > > > > I did not go that far as I am unsure as well.    
> > > > Memory allocations in qemu don't fail (well if they do it crashes)
> > > > Side effect of using glib which makes for simpler cases.
> > > > https://docs.gtk.org/glib/func.malloc.html
> > > > 
> > > > There shouldn't even be any checks :(  I'll fix that up at somepoint
> > > > across all the CXL emulation.  Sometimes reviewers noticed and
> > > > we dropped it at earlier stages, but clearly didn't catch them all.
> > > > 
> > > > Which come to think of it is why this error condition is in practice
> > > > not actually buggy as the code won't ever manage to return -ENOMEM and
> > > > I don't think there are other error codes.    
> > > 
> > > Ah.  Ok but in that case I would say that build_cdat_table() should never
> > > return < 0 to be clear at this level what can happen.
> > > 
> > > Would you like a patch for that?  (/me assumes you dropped this patch)  
> > 
> > Probably needs to first rip out all the -ENOMEM returns that got into
> > the CXL code in general, then tidy up the return type to be unsigned.
> > 
> > If you want to do that it would be welcome!
> Actually.  Build_cdat_table() can return errors just not for this reason.
> 
> host_memory_backend_get_memory() can fail for example.

I must be on a different version because I don't see that.

>
> So original patch is good
> as is, just that the discussion of memory allocation failure threw me
> off and should be cleaned up separately.
> 

I did this testing on Fan's DCD version...  :-/  ... probably very out of
date.

Fan do you have a newer version than your 2023-11-16 branch?

Ira