Messages in this thread | | | Date | Tue, 29 Aug 2023 17:16:59 -0700 | From | Ira Weiny <> | Subject | Re: [PATCH RFC v2 09/18] cxl/mem: Read extents on memory device discovery |
| |
Jonathan Cameron wrote: > On Mon, 28 Aug 2023 22:21:00 -0700 > Ira Weiny <ira.weiny@intel.com> wrote: >
[snip]
I'll go through each review but I had to respond to this one...
> > > + .payload_out = &dc_extents, > > + .min_out = 1, > > + }; > > + > > + rc = cxl_internal_send_cmd(mds, &mbox_cmd); > > + if (rc < 0) > > + return rc; > > + > > + count = le32_to_cpu(dc_extents.total_extent_cnt); > > + *extent_gen_num = le32_to_cpu(dc_extents.extent_list_num); > > + > > + return count; > > +} > > + > > +static int cxl_dev_get_dc_extents(struct cxl_memdev_state *mds, > > + unsigned int start_gen_num, > > + unsigned int exp_cnt) > > +{ > > + struct cxl_mbox_dc_extents *dc_extents; > > + unsigned int start_index, total_read; > > + struct device *dev = mds->cxlds.dev; > > + struct cxl_mbox_cmd mbox_cmd; > > + int retry = 3; > > Why 3? >
Then shall thou count to 3, no more, no less... 4 shall thou not count... 5 is right out...
;-)
Seriously, it seemed like a decent number to try. I would hope that the extents are not changing much as the host is booting or the device drivers are loading. But since the generation number is there I figured it was fine to try again.
However, its been a while since I focused on this patch and as I look at it now I realize that retrying is going to be a problem anyway. Some of the old extents from the previous generation may have been stored and the new list is likely to have the same extents. Which would result in errors later.
I think it is best to remove the retry and just throw an error.
Thanks for catching, Ira
| |