linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][RESEND] docs: filesystems: sysfs: convert sysfs.txt to reST
@ 2019-11-05  7:18 Jaskaran Singh
  2019-11-07 19:04 ` Jonathan Corbet
  0 siblings, 1 reply; 5+ messages in thread
From: Jaskaran Singh @ 2019-11-05  7:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: corbet, mchehab+samsung, christian, neilb, willy,
	jaskaransingh7654321, tobin, stefanha, hofrat, gregkh,
	jeffrey.t.kirsher, linux-doc, skhan, linux-kernel-mentees

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 <jaskaransingh7654321@gmail.com>
---
 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	<mochel@osdl.org>
-Mike Murphy <mamurph@cs.clemson.edu>
+- Patrick Mochel	<mochel@osdl.org>
+- Mike Murphy   	<mamurph@cs.clemson.edu>
 
-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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH][RESEND] docs: filesystems: sysfs: convert sysfs.txt to reST
  2019-11-05  7:18 [PATCH][RESEND] docs: filesystems: sysfs: convert sysfs.txt to reST Jaskaran Singh
@ 2019-11-07 19:04 ` Jonathan Corbet
  2019-11-09 13:06   ` Jaskaran Singh
  0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Corbet @ 2019-11-07 19:04 UTC (permalink / raw)
  To: Jaskaran Singh
  Cc: linux-kernel, mchehab+samsung, christian, neilb, willy, tobin,
	stefanha, hofrat, gregkh, jeffrey.t.kirsher, linux-doc, skhan,
	linux-kernel-mentees

On Tue, 5 Nov 2019 12:48:46 +0530
Jaskaran Singh <jaskaransingh7654321@gmail.com> 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 <jaskaransingh7654321@gmail.com>

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	<mochel@osdl.org>
> -Mike Murphy <mamurph@cs.clemson.edu>
> +- Patrick Mochel	<mochel@osdl.org>
> +- Mike Murphy   	<mamurph@cs.clemson.edu>

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][RESEND] docs: filesystems: sysfs: convert sysfs.txt to reST
  2019-11-07 19:04 ` Jonathan Corbet
@ 2019-11-09 13:06   ` Jaskaran Singh
  2019-11-12 17:04     ` Jonathan Corbet
  0 siblings, 1 reply; 5+ messages in thread
From: Jaskaran Singh @ 2019-11-09 13:06 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: linux-kernel, mchehab+samsung, christian, neilb, willy, tobin,
	stefanha, hofrat, gregkh, jeffrey.t.kirsher, linux-doc, skhan,
	linux-kernel-mentees

On Thu, 2019-11-07 at 12:04 -0700, Jonathan Corbet wrote:
> On Tue, 5 Nov 2019 12:48:46 +0530
> Jaskaran Singh <jaskaransingh7654321@gmail.com> 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 <jaskaransingh7654321@gmail.com>
> 
> 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	<mochel@osdl.org>
> > -Mike Murphy <mamurph@cs.clemson.edu>
> > +- Patrick Mochel	<mochel@osdl.org>
> > +- Mike Murphy   	<mamurph@cs.clemson.edu>
> 
> 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
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][RESEND] docs: filesystems: sysfs: convert sysfs.txt to reST
  2019-11-09 13:06   ` Jaskaran Singh
@ 2019-11-12 17:04     ` Jonathan Corbet
  2019-11-13  7:30       ` Jaskaran Singh
  0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Corbet @ 2019-11-12 17:04 UTC (permalink / raw)
  To: Jaskaran Singh
  Cc: linux-kernel, mchehab+samsung, christian, neilb, willy, tobin,
	stefanha, hofrat, gregkh, jeffrey.t.kirsher, linux-doc, skhan,
	linux-kernel-mentees

On Sat, 09 Nov 2019 18:36:16 +0530
Jaskaran Singh <jaskaransingh7654321@gmail.com> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][RESEND] docs: filesystems: sysfs: convert sysfs.txt to reST
  2019-11-12 17:04     ` Jonathan Corbet
@ 2019-11-13  7:30       ` Jaskaran Singh
  0 siblings, 0 replies; 5+ messages in thread
From: Jaskaran Singh @ 2019-11-13  7:30 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: linux-kernel, mchehab+samsung, christian, neilb, willy, tobin,
	stefanha, hofrat, gregkh, jeffrey.t.kirsher, linux-doc, skhan,
	linux-kernel-mentees

On Tue, 2019-11-12 at 10:04 -0700, Jonathan Corbet wrote:
> On Sat, 09 Nov 2019 18:36:16 +0530
> Jaskaran Singh <jaskaransingh7654321@gmail.com> 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.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-11-13  7:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05  7:18 [PATCH][RESEND] docs: filesystems: sysfs: convert sysfs.txt to reST Jaskaran Singh
2019-11-07 19:04 ` Jonathan Corbet
2019-11-09 13:06   ` Jaskaran Singh
2019-11-12 17:04     ` Jonathan Corbet
2019-11-13  7:30       ` Jaskaran Singh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).