[PATCH v4 05/28] dax: Document dax dev range tuple

Ira Weiny posted 28 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v4 05/28] dax: Document dax dev range tuple
Posted by Ira Weiny 1 month, 2 weeks ago
The device DAX structure is being enhanced to track additional DCD
information.

The current range tuple was not fully documented.  Document it prior to
adding information for DC.

Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes:
[iweiny: move to start of series]
---
 drivers/dax/dax-private.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
index 446617b73aea..ccde98c3d4e2 100644
--- a/drivers/dax/dax-private.h
+++ b/drivers/dax/dax-private.h
@@ -58,7 +58,10 @@ struct dax_mapping {
  * @dev - device core
  * @pgmap - pgmap for memmap setup / lifetime (driver owned)
  * @nr_range: size of @ranges
- * @ranges: resource-span + pgoff tuples for the instance
+ * @ranges: range tuples of memory used
+ * @pgoff: page offset
+ * @range: resource-span
+ * @mapping: device to assist in interrogating the range layout
  */
 struct dev_dax {
 	struct dax_region *region;

-- 
2.46.0
Re: [PATCH v4 05/28] dax: Document dax dev range tuple
Posted by Jonathan Cameron 1 month, 2 weeks ago
On Mon, 07 Oct 2024 18:16:11 -0500
Ira Weiny <ira.weiny@intel.com> wrote:

> The device DAX structure is being enhanced to track additional DCD
> information.
> 
> The current range tuple was not fully documented.  Document it prior to
> adding information for DC.
> 
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
Isn't this a nested struct?
https://docs.kernel.org/doc-guide/kernel-doc.html#nested-structs-unions

I'm not quite sure how we document when it's a nested pointer to a
a structure.  Is it the same as for a 'normal' nested struct?
  
> ---
> Changes:
> [iweiny: move to start of series]
> ---
>  drivers/dax/dax-private.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
> index 446617b73aea..ccde98c3d4e2 100644
> --- a/drivers/dax/dax-private.h
> +++ b/drivers/dax/dax-private.h
> @@ -58,7 +58,10 @@ struct dax_mapping {
>   * @dev - device core
>   * @pgmap - pgmap for memmap setup / lifetime (driver owned)
>   * @nr_range: size of @ranges
> - * @ranges: resource-span + pgoff tuples for the instance
> + * @ranges: range tuples of memory used
> + * @pgoff: page offset
      @ranges.pgoff?
etc

> + * @range: resource-span
> + * @mapping: device to assist in interrogating the range layout
>   */
>  struct dev_dax {
>  	struct dax_region *region;
>
Re: [PATCH v4 05/28] dax: Document dax dev range tuple
Posted by Ira Weiny 1 month, 2 weeks ago
Jonathan Cameron wrote:
> On Mon, 07 Oct 2024 18:16:11 -0500
> Ira Weiny <ira.weiny@intel.com> wrote:
> 
> > The device DAX structure is being enhanced to track additional DCD
> > information.
> > 
> > The current range tuple was not fully documented.  Document it prior to
> > adding information for DC.
> > 
> > Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > 
> Isn't this a nested struct?
> https://docs.kernel.org/doc-guide/kernel-doc.html#nested-structs-unions
> 
> I'm not quite sure how we document when it's a nested pointer to a
> a structure.  Is it the same as for a 'normal' nested struct?

In this case I think it best to document the struct and just document the
reference.  See below.

>   
> > ---
> > Changes:
> > [iweiny: move to start of series]
> > ---
> >  drivers/dax/dax-private.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
> > index 446617b73aea..ccde98c3d4e2 100644
> > --- a/drivers/dax/dax-private.h
> > +++ b/drivers/dax/dax-private.h
> > @@ -58,7 +58,10 @@ struct dax_mapping {
> >   * @dev - device core
> >   * @pgmap - pgmap for memmap setup / lifetime (driver owned)
> >   * @nr_range: size of @ranges
> > - * @ranges: resource-span + pgoff tuples for the instance
> > + * @ranges: range tuples of memory used
> > + * @pgoff: page offset
>       @ranges.pgoff?
> etc

Ok yea.

As for the pointer to a structure.  I think the best thing to do is simply
document that structure.

Something like this building on this patch:


diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
index ccde98c3d4e2..b9816c933575 100644
--- a/drivers/dax/dax-private.h
+++ b/drivers/dax/dax-private.h
@@ -40,6 +40,12 @@ struct dax_region {
        struct device *youngest;
 };
 
+/**
+ * struct dax_mapping - device to display mapping range attributes
+ * @dev: device representing this range
+ * @range_id: index within dev_dax ranges array
+ * @id: ida of this mapping
+ */
 struct dax_mapping {
        struct device dev;
        int range_id;
@@ -59,9 +65,9 @@ struct dax_mapping {
  * @pgmap - pgmap for memmap setup / lifetime (driver owned)
  * @nr_range: size of @ranges
  * @ranges: range tuples of memory used
- * @pgoff: page offset
- * @range: resource-span
- * @mapping: device to assist in interrogating the range layout
+ * @ranges.pgoff: page offset
+ * @ranges.range: resource-span
+ * @ranges.mapping: reference to the dax_mapping for this range
  */
 struct dev_dax {
        struct dax_region *region;
Re: [PATCH v4 05/28] dax: Document dax dev range tuple
Posted by Jonathan Cameron 1 month, 1 week ago
On Fri, 11 Oct 2024 15:40:58 -0500
Ira Weiny <ira.weiny@intel.com> wrote:

> Jonathan Cameron wrote:
> > On Mon, 07 Oct 2024 18:16:11 -0500
> > Ira Weiny <ira.weiny@intel.com> wrote:
> >   
> > > The device DAX structure is being enhanced to track additional DCD
> > > information.
> > > 
> > > The current range tuple was not fully documented.  Document it prior to
> > > adding information for DC.
> > > 
> > > Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > >   
> > Isn't this a nested struct?
> > https://docs.kernel.org/doc-guide/kernel-doc.html#nested-structs-unions
> > 
> > I'm not quite sure how we document when it's a nested pointer to a
> > a structure.  Is it the same as for a 'normal' nested struct?  
> 
> In this case I think it best to document the struct and just document the
> reference.  See below.
> 
> >     
> > > ---
> > > Changes:
> > > [iweiny: move to start of series]
> > > ---
> > >  drivers/dax/dax-private.h | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
> > > index 446617b73aea..ccde98c3d4e2 100644
> > > --- a/drivers/dax/dax-private.h
> > > +++ b/drivers/dax/dax-private.h
> > > @@ -58,7 +58,10 @@ struct dax_mapping {
> > >   * @dev - device core
> > >   * @pgmap - pgmap for memmap setup / lifetime (driver owned)
> > >   * @nr_range: size of @ranges
> > > - * @ranges: resource-span + pgoff tuples for the instance
> > > + * @ranges: range tuples of memory used
> > > + * @pgoff: page offset  
> >       @ranges.pgoff?
> > etc  
> 
> Ok yea.
> 
> As for the pointer to a structure.  I think the best thing to do is simply
> document that structure.
> 
> Something like this building on this patch:
> 
> 
> diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
> index ccde98c3d4e2..b9816c933575 100644
> --- a/drivers/dax/dax-private.h
> +++ b/drivers/dax/dax-private.h
> @@ -40,6 +40,12 @@ struct dax_region {
>         struct device *youngest;
>  };
>  
> +/**
> + * struct dax_mapping - device to display mapping range attributes
> + * @dev: device representing this range
> + * @range_id: index within dev_dax ranges array
> + * @id: ida of this mapping
> + */
>  struct dax_mapping {
>         struct device dev;
>         int range_id;
> @@ -59,9 +65,9 @@ struct dax_mapping {
>   * @pgmap - pgmap for memmap setup / lifetime (driver owned)
>   * @nr_range: size of @ranges
>   * @ranges: range tuples of memory used
> - * @pgoff: page offset
> - * @range: resource-span
> - * @mapping: device to assist in interrogating the range layout
> + * @ranges.pgoff: page offset
> + * @ranges.range: resource-span
> + * @ranges.mapping: reference to the dax_mapping for this range

Maybe just pull out definition of struct dev_dax_range?
Avoids this confusion and no particularly obvious reason why it
is embedded in the definition of dev_dax.

>   */
>  struct dev_dax {
>         struct dax_region *region;
>