This patch converts sysfs.txt to sysfs.rst, and adds a corresponding
entry in index.rst.
Most of the whitespacing and indentation is kept similar to the
original document.
Changes to the original document include:
- Adding an authors statement in the header.
- Replacing the underscores in the title with asterisks. This is so
that the "The" in the title appears in italics in HTML.
- Replacing the tilde (~) headings with equal signs, for reST section
headings.
- List out the helper macros with backquotes and corresponding description
on the next line.
- Placing C code and shell code in reST code blocks, with an indentation
of an 8 length tab.
Signed-off-by: Jaskaran Singh <[email protected]>
---
Documentation/filesystems/index.rst | 1 +
.../filesystems/{sysfs.txt => sysfs.rst} | 323 ++++++++++--------
2 files changed, 189 insertions(+), 135 deletions(-)
rename Documentation/filesystems/{sysfs.txt => sysfs.rst} (60%)
diff --git a/Documentation/filesystems/index.rst b/Documentation/filesystems/index.rst
index 2c3a9f761205..18b5ea780b9b 100644
--- a/Documentation/filesystems/index.rst
+++ b/Documentation/filesystems/index.rst
@@ -46,4 +46,5 @@ Documentation for filesystem implementations.
.. toctree::
:maxdepth: 2
+ sysfs
virtiofs
diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.rst
similarity index 60%
rename from Documentation/filesystems/sysfs.txt
rename to Documentation/filesystems/sysfs.rst
index ddf15b1b0d5a..de0de5869323 100644
--- a/Documentation/filesystems/sysfs.txt
+++ b/Documentation/filesystems/sysfs.rst
@@ -1,15 +1,18 @@
+======================================================
+sysfs - *The* filesystem for exporting kernel objects.
+======================================================
-sysfs - _The_ filesystem for exporting kernel objects.
+Authors:
-Patrick Mochel <[email protected]>
-Mike Murphy <[email protected]>
+- Patrick Mochel <[email protected]>
+- Mike Murphy <[email protected]>
-Revised: 16 August 2011
-Original: 10 January 2003
+| Revised: 16 August 2011
+| Original: 10 January 2003
What it is:
-~~~~~~~~~~~
+===========
sysfs is a ram-based filesystem initially based on ramfs. It provides
a means to export kernel data structures, their attributes, and the
@@ -21,16 +24,18 @@ interface.
Using sysfs
-~~~~~~~~~~~
+===========
sysfs is always compiled in if CONFIG_SYSFS is defined. You can access
it by doing:
- mount -t sysfs sysfs /sys
+.. code-block:: sh
+
+ mount -t sysfs sysfs /sys
Directory Creation
-~~~~~~~~~~~~~~~~~~
+==================
For every kobject that is registered with the system, a directory is
created for it in sysfs. That directory is created as a subdirectory
@@ -48,7 +53,7 @@ only modified directly by the function sysfs_schedule_callback().
Attributes
-~~~~~~~~~~
+==========
Attributes can be exported for kobjects in the form of regular files in
the filesystem. Sysfs forwards file I/O operations to methods defined
@@ -67,15 +72,16 @@ you publicly humiliated and your code rewritten without notice.
An attribute definition is simply:
-struct attribute {
- char * name;
- struct module *owner;
- umode_t mode;
-};
+.. code-block:: c
+ struct attribute {
+ char * name;
+ struct module *owner;
+ umode_t mode;
+ };
-int sysfs_create_file(struct kobject * kobj, const struct attribute * attr);
-void sysfs_remove_file(struct kobject * kobj, const struct attribute * attr);
+ int sysfs_create_file(struct kobject * kobj, const struct attribute * attr);
+ void sysfs_remove_file(struct kobject * kobj, const struct attribute * attr);
A bare attribute contains no means to read or write the value of the
@@ -85,36 +91,44 @@ a specific object type.
For example, the driver model defines struct device_attribute like:
-struct device_attribute {
- struct attribute attr;
- ssize_t (*show)(struct device *dev, struct device_attribute *attr,
- char *buf);
- ssize_t (*store)(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count);
-};
+.. code-block:: c
+
+ struct device_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct device *dev, struct device_attribute *attr,
+ char *buf);
+ ssize_t (*store)(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count);
+ };
-int device_create_file(struct device *, const struct device_attribute *);
-void device_remove_file(struct device *, const struct device_attribute *);
+ int device_create_file(struct device *, const struct device_attribute *);
+ void device_remove_file(struct device *, const struct device_attribute *);
It also defines this helper for defining device attributes:
-#define DEVICE_ATTR(_name, _mode, _show, _store) \
-struct device_attribute dev_attr_##_name = __ATTR(_name, _mode, _show, _store)
+.. code-block:: c
+
+ #define DEVICE_ATTR(_name, _mode, _show, _store) \
+ struct device_attribute dev_attr_##_name = __ATTR(_name, _mode, _show, _store)
For example, declaring
-static DEVICE_ATTR(foo, S_IWUSR | S_IRUGO, show_foo, store_foo);
+.. code-block:: c
+
+ static DEVICE_ATTR(foo, S_IWUSR | S_IRUGO, show_foo, store_foo);
is equivalent to doing:
-static struct device_attribute dev_attr_foo = {
- .attr = {
- .name = "foo",
- .mode = S_IWUSR | S_IRUGO,
- },
- .show = show_foo,
- .store = store_foo,
-};
+.. code-block:: c
+
+ static struct device_attribute dev_attr_foo = {
+ .attr = {
+ .name = "foo",
+ .mode = S_IWUSR | S_IRUGO,
+ },
+ .show = show_foo,
+ .store = store_foo,
+ };
Note as stated in include/linux/kernel.h "OTHER_WRITABLE? Generally
considered a bad idea." so trying to set a sysfs file writable for
@@ -124,31 +138,45 @@ For the common cases sysfs.h provides convenience macros to make
defining attributes easier as well as making code more concise and
readable. The above case could be shortened to:
-static struct device_attribute dev_attr_foo = __ATTR_RW(foo);
+.. code-block:: c
+
+ static struct device_attribute dev_attr_foo = __ATTR_RW(foo);
the list of helpers available to define your wrapper function is:
-__ATTR_RO(name): assumes default name_show and mode 0444
-__ATTR_WO(name): assumes a name_store only and is restricted to mode
- 0200 that is root write access only.
-__ATTR_RO_MODE(name, mode): fore more restrictive RO access currently
- only use case is the EFI System Resource Table
- (see drivers/firmware/efi/esrt.c)
-__ATTR_RW(name): assumes default name_show, name_store and setting
- mode to 0644.
-__ATTR_NULL: which sets the name to NULL and is used as end of list
- indicator (see: kernel/workqueue.c)
+
+``__ATTR_RO(name)``
+ assumes default name_show and mode 0444
+
+``__ATTR_WO(name)``
+ assumes a name_store only and is restricted to mode
+ 0200 that is root write access only.
+
+``__ATTR_RO_MODE(name, mode)``
+ for more restrictive RO access currently
+ only use case is the EFI System Resource Table
+ (see drivers/firmware/efi/esrt.c)
+
+``__ATTR_RW(name)``
+ assumes default name_show, name_store and setting
+ mode to 0644.
+
+``__ATTR_NULL``
+ which sets the name to NULL and is used as end of list
+ indicator (see: kernel/workqueue.c)
Subsystem-Specific Callbacks
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+============================
When a subsystem defines a new attribute type, it must implement a
set of sysfs operations for forwarding read and write calls to the
show and store methods of the attribute owners.
-struct sysfs_ops {
- ssize_t (*show)(struct kobject *, struct attribute *, char *);
- ssize_t (*store)(struct kobject *, struct attribute *, const char *, size_t);
-};
+.. code-block:: c
+
+ struct sysfs_ops {
+ ssize_t (*show)(struct kobject *, struct attribute *, char *);
+ ssize_t (*store)(struct kobject *, struct attribute *, const char *, size_t);
+ };
[ Subsystems should have already defined a struct kobj_type as a
descriptor for this type, which is where the sysfs_ops pointer is
@@ -162,37 +190,41 @@ calls the associated methods.
To illustrate:
-#define to_dev(obj) container_of(obj, struct device, kobj)
-#define to_dev_attr(_attr) container_of(_attr, struct device_attribute, attr)
+.. code-block:: c
-static ssize_t dev_attr_show(struct kobject *kobj, struct attribute *attr,
- char *buf)
-{
- struct device_attribute *dev_attr = to_dev_attr(attr);
- struct device *dev = to_dev(kobj);
- ssize_t ret = -EIO;
+ #define to_dev(obj) container_of(obj, struct device, kobj)
+ #define to_dev_attr(_attr) container_of(_attr, struct device_attribute, attr)
- if (dev_attr->show)
- ret = dev_attr->show(dev, dev_attr, buf);
- if (ret >= (ssize_t)PAGE_SIZE) {
- printk("dev_attr_show: %pS returned bad count\n",
- dev_attr->show);
- }
- return ret;
-}
+ static ssize_t dev_attr_show(struct kobject *kobj, struct attribute *attr,
+ char *buf)
+ {
+ struct device_attribute *dev_attr = to_dev_attr(attr);
+ struct device *dev = to_dev(kobj);
+ ssize_t ret = -EIO;
+
+ if (dev_attr->show)
+ ret = dev_attr->show(dev, dev_attr, buf);
+ if (ret >= (ssize_t)PAGE_SIZE) {
+ printk("dev_attr_show: %pS returned bad count\n",
+ dev_attr->show);
+ }
+ return ret;
+ }
Reading/Writing Attribute Data
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+==============================
To read or write attributes, show() or store() methods must be
specified when declaring the attribute. The method types should be as
simple as those defined for device attributes:
-ssize_t (*show)(struct device *dev, struct device_attribute *attr, char *buf);
-ssize_t (*store)(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count);
+.. code-block:: c
+
+ ssize_t (*show)(struct device *dev, struct device_attribute *attr, char *buf);
+ ssize_t (*store)(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count);
IOW, they should take only an object, an attribute, and a buffer as parameters.
@@ -253,21 +285,23 @@ Other notes:
A very simple (and naive) implementation of a device attribute is:
-static ssize_t show_name(struct device *dev, struct device_attribute *attr,
- char *buf)
-{
- return scnprintf(buf, PAGE_SIZE, "%s\n", dev->name);
-}
+.. code-block:: c
-static ssize_t store_name(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count)
-{
- snprintf(dev->name, sizeof(dev->name), "%.*s",
- (int)min(count, sizeof(dev->name) - 1), buf);
- return count;
-}
+ static ssize_t show_name(struct device *dev, struct device_attribute *attr,
+ char *buf)
+ {
+ return scnprintf(buf, PAGE_SIZE, "%s\n", dev->name);
+ }
-static DEVICE_ATTR(name, S_IRUGO, show_name, store_name);
+ static ssize_t store_name(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+ {
+ snprintf(dev->name, sizeof(dev->name), "%.*s",
+ (int)min(count, sizeof(dev->name) - 1), buf);
+ return count;
+ }
+
+ static DEVICE_ATTR(name, S_IRUGO, show_name, store_name);
(Note that the real implementation doesn't allow userspace to set the
@@ -275,28 +309,28 @@ name for a device.)
Top Level Directory Layout
-~~~~~~~~~~~~~~~~~~~~~~~~~~
+==========================
The sysfs directory arrangement exposes the relationship of kernel
data structures.
-The top level sysfs directory looks like:
+The top level sysfs directory looks like: ::
-block/
-bus/
-class/
-dev/
-devices/
-firmware/
-net/
-fs/
+ block/
+ bus/
+ class/
+ dev/
+ devices/
+ firmware/
+ net/
+ fs/
devices/ contains a filesystem representation of the device tree. It maps
directly to the internal kernel device tree, which is a hierarchy of
struct device.
bus/ contains flat directory layout of the various bus types in the
-kernel. Each bus's directory contains two subdirectories:
+kernel. Each bus's directory contains two subdirectories: ::
devices/
drivers/
@@ -326,80 +360,99 @@ TODO: Finish this section.
Current Interfaces
-~~~~~~~~~~~~~~~~~~
+==================
The following interface layers currently exist in sysfs:
-- devices (include/linux/device.h)
-----------------------------------
+devices (include/linux/device.h)
+--------------------------------
Structure:
-struct device_attribute {
- struct attribute attr;
- ssize_t (*show)(struct device *dev, struct device_attribute *attr,
- char *buf);
- ssize_t (*store)(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count);
-};
+.. code-block:: c
+
+ struct device_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct device *dev, struct device_attribute *attr,
+ char *buf);
+ ssize_t (*store)(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count);
+ };
Declaring:
-DEVICE_ATTR(_name, _mode, _show, _store);
+.. code-block:: c
+
+ DEVICE_ATTR(_name, _mode, _show, _store);
Creation/Removal:
-int device_create_file(struct device *dev, const struct device_attribute * attr);
-void device_remove_file(struct device *dev, const struct device_attribute * attr);
+.. code-block:: c
+
+ int device_create_file(struct device *dev, const struct device_attribute * attr);
+ void device_remove_file(struct device *dev, const struct device_attribute * attr);
-- bus drivers (include/linux/device.h)
---------------------------------------
+bus drivers (include/linux/device.h)
+------------------------------------
+
Structure:
-struct bus_attribute {
- struct attribute attr;
- ssize_t (*show)(struct bus_type *, char * buf);
- ssize_t (*store)(struct bus_type *, const char * buf, size_t count);
-};
+.. code-block:: c
+
+ struct bus_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct bus_type *, char * buf);
+ ssize_t (*store)(struct bus_type *, const char * buf, size_t count);
+ };
Declaring:
-static BUS_ATTR_RW(name);
-static BUS_ATTR_RO(name);
-static BUS_ATTR_WO(name);
+.. code-block:: c
+
+ static BUS_ATTR_RW(name);
+ static BUS_ATTR_RO(name);
+ static BUS_ATTR_WO(name);
Creation/Removal:
-int bus_create_file(struct bus_type *, struct bus_attribute *);
-void bus_remove_file(struct bus_type *, struct bus_attribute *);
+.. code-block:: c
+
+ int bus_create_file(struct bus_type *, struct bus_attribute *);
+ void bus_remove_file(struct bus_type *, struct bus_attribute *);
-- device drivers (include/linux/device.h)
------------------------------------------
+device drivers (include/linux/device.h)
+---------------------------------------
Structure:
-struct driver_attribute {
- struct attribute attr;
- ssize_t (*show)(struct device_driver *, char * buf);
- ssize_t (*store)(struct device_driver *, const char * buf,
- size_t count);
-};
+.. code-block:: c
+
+ struct driver_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct device_driver *, char * buf);
+ ssize_t (*store)(struct device_driver *, const char * buf,
+ size_t count);
+ };
Declaring:
-DRIVER_ATTR_RO(_name)
-DRIVER_ATTR_RW(_name)
+.. code-block:: c
+
+ DRIVER_ATTR_RO(_name)
+ DRIVER_ATTR_RW(_name)
Creation/Removal:
-int driver_create_file(struct device_driver *, const struct driver_attribute *);
-void driver_remove_file(struct device_driver *, const struct driver_attribute *);
+.. code-block:: c
+
+ int driver_create_file(struct device_driver *, const struct driver_attribute *);
+ void driver_remove_file(struct device_driver *, const struct driver_attribute *);
Documentation
-~~~~~~~~~~~~~
+=============
The sysfs directory structure and the attributes in each directory define an
ABI between the kernel and user space. As for any ABI, it is important that
--
2.21.0
On Tue, 5 Nov 2019 12:48:46 +0530
Jaskaran Singh <[email protected]> wrote:
> This patch converts sysfs.txt to sysfs.rst, and adds a corresponding
> entry in index.rst.
>
> Most of the whitespacing and indentation is kept similar to the
> original document.
>
> Changes to the original document include:
>
> - Adding an authors statement in the header.
> - Replacing the underscores in the title with asterisks. This is so
> that the "The" in the title appears in italics in HTML.
> - Replacing the tilde (~) headings with equal signs, for reST section
> headings.
> - List out the helper macros with backquotes and corresponding description
> on the next line.
> - Placing C code and shell code in reST code blocks, with an indentation
> of an 8 length tab.
>
> Signed-off-by: Jaskaran Singh <[email protected]>
Thanks for working to improve the documentation. There are some problems
here, none of which are your creation, but I would sure like to resolve
them while working with this document.
The first of these is that Documentation/filesystems is really the wrong
place for this file - it's covering the internal API for subsystems that
want to create entries in sysfs. IMO, it belongs in either the driver-API
manual or the core-API manual - probably the latter.
> Documentation/filesystems/index.rst | 1 +
> .../filesystems/{sysfs.txt => sysfs.rst} | 323 ++++++++++--------
> 2 files changed, 189 insertions(+), 135 deletions(-)
> rename Documentation/filesystems/{sysfs.txt => sysfs.rst} (60%)
>
> diff --git a/Documentation/filesystems/index.rst b/Documentation/filesystems/index.rst
> index 2c3a9f761205..18b5ea780b9b 100644
> --- a/Documentation/filesystems/index.rst
> +++ b/Documentation/filesystems/index.rst
> @@ -46,4 +46,5 @@ Documentation for filesystem implementations.
> .. toctree::
> :maxdepth: 2
>
> + sysfs
> virtiofs
> diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.rst
> similarity index 60%
> rename from Documentation/filesystems/sysfs.txt
> rename to Documentation/filesystems/sysfs.rst
> index ddf15b1b0d5a..de0de5869323 100644
> --- a/Documentation/filesystems/sysfs.txt
> +++ b/Documentation/filesystems/sysfs.rst
> @@ -1,15 +1,18 @@
> +======================================================
> +sysfs - *The* filesystem for exporting kernel objects.
> +======================================================
>
> -sysfs - _The_ filesystem for exporting kernel objects.
Nits: We can really just drop emphasis like that, it doesn't really help
anybody. Also the period can go on section headers.
> +Authors:
>
> -Patrick Mochel <[email protected]>
> -Mike Murphy <[email protected]>
> +- Patrick Mochel <[email protected]>
> +- Mike Murphy <[email protected]>
I would be absolutely amazed if either of those email addresses works at
this point. I'd take them out.
> -Revised: 16 August 2011
> -Original: 10 January 2003
> +| Revised: 16 August 2011
> +| Original: 10 January 2003
Dates like that are a red flag. See below.
> What it is:
> -~~~~~~~~~~~
> +===========
>
> sysfs is a ram-based filesystem initially based on ramfs. It provides
> a means to export kernel data structures, their attributes, and the
> @@ -21,16 +24,18 @@ interface.
>
>
> Using sysfs
> -~~~~~~~~~~~
> +===========
>
> sysfs is always compiled in if CONFIG_SYSFS is defined. You can access
> it by doing:
>
> - mount -t sysfs sysfs /sys
> +.. code-block:: sh
> +
> + mount -t sysfs sysfs /sys
In the spirit of minimal markup, I'd do the above as:
it by doing::
mount -t sysfs sysfs /sys
But then I know that others are much more fond of .. code-block and syntax
highlighting than I am.
> Directory Creation
> -~~~~~~~~~~~~~~~~~~
> +==================
>
> For every kobject that is registered with the system, a directory is
> created for it in sysfs. That directory is created as a subdirectory
> @@ -48,7 +53,7 @@ only modified directly by the function sysfs_schedule_callback().
>
>
> Attributes
> -~~~~~~~~~~
> +==========
>
> Attributes can be exported for kobjects in the form of regular files in
> the filesystem. Sysfs forwards file I/O operations to methods defined
> @@ -67,15 +72,16 @@ you publicly humiliated and your code rewritten without notice.
>
> An attribute definition is simply:
>
> -struct attribute {
> - char * name;
> - struct module *owner;
> - umode_t mode;
> -};
> +.. code-block:: c
>
> + struct attribute {
> + char * name;
> + struct module *owner;
> + umode_t mode;
> + };
Here is where we go pretty far off the rails. If you go looking in
include/linux/sysfs.h, the actual definition of this structure is:
struct attribute {
const char *name;
umode_t mode;
#ifdef CONFIG_DEBUG_LOCK_ALLOC
bool ignore_lockdep:1;
struct lock_class_key *key;
struct lock_class_key skey;
#endif
};
Most notably, the owner field went away quite some time ago.
Documentation like this is not really useful to anybody; once a reader
realizes it doesn't describe current reality, they will justifiably
disregard it. This isn't your fault, of course, but converting something
like this to RST gives the illusion that it has been updated, when that is
very much not the case.
At a bare minimum, an effort like this needs to put a big flashing warning
at the top of the file. But it would be soooooo much better to actually
update the content as well.
The best way to do that would be to annotate the source with proper
kerneldoc comments, then pull them into the documentation rather than
repeating the information here. Is there any chance you might be up for
taking on a task like this?
Thanks,
jon
On Thu, 2019-11-07 at 12:04 -0700, Jonathan Corbet wrote:
> On Tue, 5 Nov 2019 12:48:46 +0530
> Jaskaran Singh <[email protected]> wrote:
>
> > This patch converts sysfs.txt to sysfs.rst, and adds a
> > corresponding
> > entry in index.rst.
> >
> > Most of the whitespacing and indentation is kept similar to the
> > original document.
> >
> > Changes to the original document include:
> >
> > - Adding an authors statement in the header.
> > - Replacing the underscores in the title with asterisks. This is
> > so
> > that the "The" in the title appears in italics in HTML.
> > - Replacing the tilde (~) headings with equal signs, for reST
> > section
> > headings.
> > - List out the helper macros with backquotes and corresponding
> > description
> > on the next line.
> > - Placing C code and shell code in reST code blocks, with an
> > indentation
> > of an 8 length tab.
> >
> > Signed-off-by: Jaskaran Singh <[email protected]>
>
> Thanks for working to improve the documentation. There are some
> problems
> here, none of which are your creation, but I would sure like to
> resolve
> them while working with this document.
>
> The first of these is that Documentation/filesystems is really the
> wrong
> place for this file - it's covering the internal API for subsystems
> that
> want to create entries in sysfs. IMO, it belongs in either the
> driver-API
> manual or the core-API manual - probably the latter.
>
> > Documentation/filesystems/index.rst | 1 +
> > .../filesystems/{sysfs.txt => sysfs.rst} | 323 ++++++++++--
> > ------
> > 2 files changed, 189 insertions(+), 135 deletions(-)
> > rename Documentation/filesystems/{sysfs.txt => sysfs.rst} (60%)
> >
> > diff --git a/Documentation/filesystems/index.rst
> > b/Documentation/filesystems/index.rst
> > index 2c3a9f761205..18b5ea780b9b 100644
> > --- a/Documentation/filesystems/index.rst
> > +++ b/Documentation/filesystems/index.rst
> > @@ -46,4 +46,5 @@ Documentation for filesystem implementations.
> > .. toctree::
> > :maxdepth: 2
> >
> > + sysfs
> > virtiofs
> > diff --git a/Documentation/filesystems/sysfs.txt
> > b/Documentation/filesystems/sysfs.rst
> > similarity index 60%
> > rename from Documentation/filesystems/sysfs.txt
> > rename to Documentation/filesystems/sysfs.rst
> > index ddf15b1b0d5a..de0de5869323 100644
> > --- a/Documentation/filesystems/sysfs.txt
> > +++ b/Documentation/filesystems/sysfs.rst
> > @@ -1,15 +1,18 @@
> > +======================================================
> > +sysfs - *The* filesystem for exporting kernel objects.
> > +======================================================
> >
> > -sysfs - _The_ filesystem for exporting kernel objects.
>
> Nits: We can really just drop emphasis like that, it doesn't really
> help
> anybody. Also the period can go on section headers.
>
> > +Authors:
> >
> > -Patrick Mochel <[email protected]>
> > -Mike Murphy <[email protected]>
> > +- Patrick Mochel <[email protected]>
> > +- Mike Murphy <[email protected]>
>
> I would be absolutely amazed if either of those email addresses works
> at
> this point. I'd take them out.
>
> > -Revised: 16 August 2011
> > -Original: 10 January 2003
> > +| Revised: 16 August 2011
> > +| Original: 10 January 2003
>
> Dates like that are a red flag. See below.
>
> > What it is:
> > -~~~~~~~~~~~
> > +===========
> >
> > sysfs is a ram-based filesystem initially based on ramfs. It
> > provides
> > a means to export kernel data structures, their attributes, and
> > the
> > @@ -21,16 +24,18 @@ interface.
> >
> >
> > Using sysfs
> > -~~~~~~~~~~~
> > +===========
> >
> > sysfs is always compiled in if CONFIG_SYSFS is defined. You can
> > access
> > it by doing:
> >
> > - mount -t sysfs sysfs /sys
> > +.. code-block:: sh
> > +
> > + mount -t sysfs sysfs /sys
>
> In the spirit of minimal markup, I'd do the above as:
>
> it by doing::
>
> mount -t sysfs sysfs /sys
>
> But then I know that others are much more fond of .. code-block and
> syntax
> highlighting than I am.
>
> > Directory Creation
> > -~~~~~~~~~~~~~~~~~~
> > +==================
> >
> > For every kobject that is registered with the system, a directory
> > is
> > created for it in sysfs. That directory is created as a
> > subdirectory
> > @@ -48,7 +53,7 @@ only modified directly by the function
> > sysfs_schedule_callback().
> >
> >
> > Attributes
> > -~~~~~~~~~~
> > +==========
> >
> > Attributes can be exported for kobjects in the form of regular
> > files in
> > the filesystem. Sysfs forwards file I/O operations to methods
> > defined
> > @@ -67,15 +72,16 @@ you publicly humiliated and your code rewritten
> > without notice.
> >
> > An attribute definition is simply:
> >
> > -struct attribute {
> > - char * name;
> > - struct module *owner;
> > - umode_t mode;
> > -};
> > +.. code-block:: c
> >
> > + struct attribute {
> > + char * name;
> > + struct module *owner;
> > + umode_t mode;
> > + };
>
> Here is where we go pretty far off the rails. If you go looking in
> include/linux/sysfs.h, the actual definition of this structure is:
>
> struct attribute {
> const char *name;
> umode_t mode;
> #ifdef CONFIG_DEBUG_LOCK_ALLOC
> bool ignore_lockdep:1;
> struct lock_class_key *key;
> struct lock_class_key skey;
> #endif
> };
>
> Most notably, the owner field went away quite some time ago.
>
> Documentation like this is not really useful to anybody; once a
> reader
> realizes it doesn't describe current reality, they will justifiably
> disregard it. This isn't your fault, of course, but converting
> something
> like this to RST gives the illusion that it has been updated, when
> that is
> very much not the case.
>
> At a bare minimum, an effort like this needs to put a big flashing
> warning
> at the top of the file. But it would be soooooo much better to
> actually
> update the content as well.
>
> The best way to do that would be to annotate the source with proper
> kerneldoc comments, then pull them into the documentation rather than
> repeating the information here. Is there any chance you might be up
> for
> taking on a task like this?
>
Sure! I'll send the documentation patch(es) followed by a v2 for this
patch.
Cheers,
Jaskaran.
> Thanks,
>
> jon
>
On Sat, 09 Nov 2019 18:36:16 +0530
Jaskaran Singh <[email protected]> wrote:
> > At a bare minimum, an effort like this needs to put a big flashing
> > warning
> > at the top of the file. But it would be soooooo much better to
> > actually
> > update the content as well.
> >
> > The best way to do that would be to annotate the source with proper
> > kerneldoc comments, then pull them into the documentation rather than
> > repeating the information here. Is there any chance you might be up
> > for
> > taking on a task like this?
> >
>
>
> Sure! I'll send the documentation patch(es) followed by a v2 for this
> patch.
Great. As an alternative, if you like, redo the current patch with the
other suggested fixes and a "this is obsolete" warning at the top. Then,
after the subsequent improvements land, that warning can eventually be
removed again.
Thanks,
jon
On Tue, 2019-11-12 at 10:04 -0700, Jonathan Corbet wrote:
> On Sat, 09 Nov 2019 18:36:16 +0530
> Jaskaran Singh <[email protected]> wrote:
>
> > > At a bare minimum, an effort like this needs to put a big
> > > flashing
> > > warning
> > > at the top of the file. But it would be soooooo much better to
> > > actually
> > > update the content as well.
> > >
> > > The best way to do that would be to annotate the source with
> > > proper
> > > kerneldoc comments, then pull them into the documentation rather
> > > than
> > > repeating the information here. Is there any chance you might be
> > > up
> > > for
> > > taking on a task like this?
> > >
> >
> > Sure! I'll send the documentation patch(es) followed by a v2 for
> > this
> > patch.
>
> Great. As an alternative, if you like, redo the current patch with
> the
> other suggested fixes and a "this is obsolete" warning at the
> top. Then,
> after the subsequent improvements land, that warning can eventually
> be
> removed again.
>
> Thanks,
>
> jon
Hopefully I can get the kernel-doc comments right. If not, I'll go with
the alternative :)
Cheers,
Jaskaran.