Re: [PATCH 1/3] cxl/mem: Fix command comment

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Tue, 3 Jan 2023 19:02:38 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> Ira Weiny wrote:
> > The command comment had some minor grammatical errors.
> > 
> > Fix them.
> > 
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > ---
> >  include/uapi/linux/cxl_mem.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
> > index c71021a2a9ed..555f9140e2bc 100644
> > --- a/include/uapi/linux/cxl_mem.h
> > +++ b/include/uapi/linux/cxl_mem.h
> > @@ -11,9 +11,9 @@
> >  /**
> >   * DOC: UAPI
> >   *
> > - * Not all of all commands that the driver supports are always available for use
> > - * by userspace. Userspace must check the results from the QUERY command in
> > - * order to determine the live set of commands.
> > + * Not all of the commands that the driver supports are available for use by
> > + * userspace at all times.  Userspace must check the results from the QUERY
> > + * command in order to determine the live set of commands.
> >   */  
> 
> It's interesting that these grammatical fixups further highlight that
> the existing description was a lie. This new wording makes it seem like
> QUERY informs about temporarily disabled commands (like those in the
> cxlds->exclusive_cmds set), in addition to the device enabled commands
> (those in the cxlds->enabled_cmds set).
> 
> It turns out this has always been a lie and the cxl tool checks if a
> command is supported and enabled by trying to execute it if it exists in
> the cxl_query_cmd() payload.
> 
> Now we could either go fix that, or change this comment to reflect the
> current reality that cxl_command_info.flags is always zero and the
> command info array is just telling you if the driver knows how to
> attempt the given command.

I'd really like to see a query mechanism that reflects whether the hardware
supports the command (plus all the other reasons why we might not be able to use
it - ultimately the question is 'can I issue this'). Some commands may
'take a while' so it's not nice to have to issue them to find out if they
exist given the discoverable nature hardware side.

Possible that mechanism is different from this one though...

Jonathan

> 
> >  
> >  #define CXL_MEM_QUERY_COMMANDS _IOR(0xCE, 1, struct cxl_mem_query_commands)
> > 
> > -- 
> > 2.38.1  
> 
>