Messages in this thread | | | Date | Tue, 9 Jan 2024 15:34:58 +0000 | From | Jonathan Cameron <> | Subject | Re: [PATCH v2 1/2] cxl/cdat: Handle cdat table build errors |
| |
On Mon, 8 Jan 2024 18:48:48 -0800 Ira Weiny <ira.weiny@intel.com> wrote:
> 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.
https://elixir.bootlin.com/qemu/latest/source/hw/mem/cxl_type3.c#L183 https://elixir.bootlin.com/qemu/v8.1.0/source/hw/mem/cxl_type3.c#L171 been there a while, but meh, too many branches floating around :)
> > Fan do you have a newer version than your 2023-11-16 branch? >
> Ira >
| |