| Message ID | 167364543268.908947.8539778012050364814.stgit@djiang5-mobl3.local |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cxl: fix cdat_available state post error | expand |
On Fri, Jan 13, 2023 at 02:30:34PM -0700, Jiang, Dave wrote: > Given that the sysfs attributre for CDAT_read() checks port->cdat_avilable, > it indicates that the binary backing cdat must be present. > > port->cdat_available is set to true in read_cdat_data() before the data is > actually read. When error happens during read, this state is never set to > false. Move the setting of the attribute to after everything is done to > ensure it's always true. Actually port->cdat_available does not indicate that the data was read. Only if the sysfs entry should be there: ... * @cdat_available: Should a CDAT attribute be available in sysfs ... The table pointer should be checked to see if the data is actually available. If data is not available the sysfs will return no data. Ira > > Fixes: c97006046c79 ("cxl/port: Read CDAT table") > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/cxl/core/pci.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 1d1492440287..91cdd6a00964 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -601,8 +601,6 @@ void read_cdat_data(struct cxl_port *port) > return; > } > > - port->cdat_available = true; > - > if (cxl_cdat_get_length(dev, cdat_doe, &cdat_length)) { > dev_dbg(dev, "No CDAT length\n"); > return; > @@ -621,6 +619,8 @@ void read_cdat_data(struct cxl_port *port) > port->cdat.length = 0; > dev_err(dev, "CDAT data read error\n"); > } > + > + port->cdat_available = true; > } > EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL); > > >
On 1/13/23 4:37 PM, Ira Weiny wrote: > On Fri, Jan 13, 2023 at 02:30:34PM -0700, Jiang, Dave wrote: >> Given that the sysfs attributre for CDAT_read() checks port->cdat_avilable, >> it indicates that the binary backing cdat must be present. >> >> port->cdat_available is set to true in read_cdat_data() before the data is >> actually read. When error happens during read, this state is never set to >> false. Move the setting of the attribute to after everything is done to >> ensure it's always true. > > Actually port->cdat_available does not indicate that the data was read. Only > if the sysfs entry should be there: > > ... > * @cdat_available: Should a CDAT attribute be available in sysfs > ... > > The table pointer should be checked to see if the data is actually available. > If data is not available the sysfs will return no data. Ok that makes sense. I only verified to see if cdat_available was checked in the sysfs function but didn't read further. Patch dropped. > > Ira > >> >> Fixes: c97006046c79 ("cxl/port: Read CDAT table") >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> --- >> drivers/cxl/core/pci.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c >> index 1d1492440287..91cdd6a00964 100644 >> --- a/drivers/cxl/core/pci.c >> +++ b/drivers/cxl/core/pci.c >> @@ -601,8 +601,6 @@ void read_cdat_data(struct cxl_port *port) >> return; >> } >> >> - port->cdat_available = true; >> - >> if (cxl_cdat_get_length(dev, cdat_doe, &cdat_length)) { >> dev_dbg(dev, "No CDAT length\n"); >> return; >> @@ -621,6 +619,8 @@ void read_cdat_data(struct cxl_port *port) >> port->cdat.length = 0; >> dev_err(dev, "CDAT data read error\n"); >> } >> + >> + port->cdat_available = true; >> } >> EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL); >> >> >>
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 1d1492440287..91cdd6a00964 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -601,8 +601,6 @@ void read_cdat_data(struct cxl_port *port) return; } - port->cdat_available = true; - if (cxl_cdat_get_length(dev, cdat_doe, &cdat_length)) { dev_dbg(dev, "No CDAT length\n"); return; @@ -621,6 +619,8 @@ void read_cdat_data(struct cxl_port *port) port->cdat.length = 0; dev_err(dev, "CDAT data read error\n"); } + + port->cdat_available = true; } EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
Given that the sysfs attributre for CDAT_read() checks port->cdat_avilable, it indicates that the binary backing cdat must be present. port->cdat_available is set to true in read_cdat_data() before the data is actually read. When error happens during read, this state is never set to false. Move the setting of the attribute to after everything is done to ensure it's always true. Fixes: c97006046c79 ("cxl/port: Read CDAT table") Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- drivers/cxl/core/pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)