| Message ID | 20220317234049.69323-2-dave@stgolabs.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl/mbox: Robustify handling of mbox_cmd.return_code | expand |
On Thu, Mar 17, 2022 at 04:40:45PM -0700, Davidlohr Bueso wrote: > ... this is better served in the callback that actually > grabs the lock. > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > --- > drivers/cxl/core/mbox.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index be61a0d8016b..778b04a0fb0a 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -136,7 +136,7 @@ static struct cxl_mem_command *cxl_mem_find_command(u16 opcode) > * @out: Caller allocated buffer for the output. > * @out_size: Expected size of output. > * > - * Context: Any context. Will acquire and release mbox_mutex. > + * Context: Any context. > * Return: > * * %>=0 - Number of bytes returned in @out. > * * %-E2BIG - Payload is too large for hardware. > @@ -165,6 +165,7 @@ int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in, > if (out_size > cxlds->payload_size) > return -E2BIG; > > + /* acquire and releases the mbox_mutex */ > rc = cxlds->mbox_send(cxlds, &mbox_cmd); > if (rc) > return rc; > > -- > 2.26.2 > Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
On Thu, Mar 17, 2022 at 4:41 PM Davidlohr Bueso <dave@stgolabs.net> wrote: > > ... this is better served in the callback that actually > grabs the lock. > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > --- > drivers/cxl/core/mbox.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index be61a0d8016b..778b04a0fb0a 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -136,7 +136,7 @@ static struct cxl_mem_command *cxl_mem_find_command(u16 opcode) > * @out: Caller allocated buffer for the output. > * @out_size: Expected size of output. > * > - * Context: Any context. Will acquire and release mbox_mutex. > + * Context: Any context. > * Return: > * * %>=0 - Number of bytes returned in @out. > * * %-E2BIG - Payload is too large for hardware. > @@ -165,6 +165,7 @@ int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in, > if (out_size > cxlds->payload_size) > return -E2BIG; > > + /* acquire and releases the mbox_mutex */ I'm not even sure this comment is necessary, because that's what lockdep is for, and it's also not true for cxl_mock_mbox_send(). Nothing outside of the ->mbox_send() implementation should be assuming that ->mbox_send() is enforcing serialization. So let's just drop this second hunk.
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index be61a0d8016b..778b04a0fb0a 100644 --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c @@ -136,7 +136,7 @@ static struct cxl_mem_command *cxl_mem_find_command(u16 opcode) * @out: Caller allocated buffer for the output. * @out_size: Expected size of output. * - * Context: Any context. Will acquire and release mbox_mutex. + * Context: Any context. * Return: * * %>=0 - Number of bytes returned in @out. * * %-E2BIG - Payload is too large for hardware. @@ -165,6 +165,7 @@ int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in, if (out_size > cxlds->payload_size) return -E2BIG; + /* acquire and releases the mbox_mutex */ rc = cxlds->mbox_send(cxlds, &mbox_cmd); if (rc) return rc;
... this is better served in the callback that actually grabs the lock. Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> --- drivers/cxl/core/mbox.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)