Messages in this thread | | | From | Arnd Bergmann <> | Date | Thu, 12 Dec 2019 12:04:51 +0100 | Subject | Re: [PATCH 24/24] Documentation: document ioctl interfaces better |
| |
On Thu, Dec 12, 2019 at 9:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Wed, Dec 11, 2019 at 9:53 PM Arnd Bergmann <arnd@arndb.de> wrote: > > +``include/uapi/asm-generic/ioctl.h`` provides four macros for defining > > +ioctl commands that follow modern conventions: ``_IOC``, ``_IOR``, > > +``_IOW``, and ``_IORW``. These should be used for all new commands, > > +with the correct parameters: > > + > > +_IO/_IOR/_IOW/_IOWR > > This says _IO.... > > > + The macro name determines whether the argument is used for passing > > + data into kernel (_IOW), from the kernel (_IOR), both (_IOWR) or is > > into the kernel > , or is > > > + not a pointer (_IOC). It is possible but not recommended to pass an > > + integer value instead of a pointer with _IOC. > > ...which is not explained here, but _IOC is?
I mean _IO() everywhere, I would consider _IOC an implementation detail and not document that here.
s/_IOC/_IO/ throughout the document now.
> > +size > > + The name of the data type pointed to by the argument, the command > > + number encodes the ``sizeof(size)`` value in a 13-bit or 14-bit integer, > > + leading to a limit of 8191 bytes for the maximum size of the argument. > > + Note: do not pass sizeof(type) type into _IOR/IOW, as that will lead > > + to encoding sizeof(sizeof(type)), i.e. sizeof(size_t). > > Looks like "size" could be renamed, to make this more obvious?
Changed to data_type now, which is what I found in some other documentation.
> > +space timespec exactly. The get_timespec64() and put_timespec64() helper > > +functions canbe used to ensure that the layout remains compatible with > > can be
done.
> > + > > +``ktime_get_real_ns()`` can be used for CLOCK_REALTIME timestamps that > > +may be required for timestamps that need to be persistent across a reboot > > Drop "may be required for timestamps that"?
done.
> > +* ``long`` and ``unsigned long`` are the size of a register, so > > + they can be either 32 bit or 64 bit wide and cannot be used in portable > > 32-bit or 64-bit (for consistency with the rest of the document)
The convention I was trying to follow is to write
"32-bit userspace" or "32-bit processor"
but
"this type is 32 bit wide"
I wasn't consistent though, changed it now as you suggested.
> > + > > +* On ARM OABI user space, 16-bit member variables have 32-bit > > + alignment, making them incompatible with modern EABI kernels. > > + Conversely, on the m68k architecture, all struct members have at most > > + 16-bit alignment. These rarely cause problems as neither ARM-OABI nor > > "have at most 16-bit alignment" sounds a bit weird to me, as a member > may have a greater alignment. > "struct members are not guaranteed to have an alignment greater than 16-bit"?
done.
> > + > > +As explained for the compat mode, it is best to not avoid any padding in > > best to avoid any implicit padding?
done.
> > + > > +* The complexity of user space access and data structure layout at done > > is done
changed
> > +There are many cases in which ioctl is not the best solution for a > > +problem. Alternatives include > > :
done
> > +* System calls are a better choice for a system-wide feature that > > + is not tied to a physical device or constrained by the file system > > + permissions of a character device node > > + > > +* netlink is the preferred way of configuring any network related > > + objects through sockets. > > + > > +* debugfs is used for ad-hoc interfaces for debugging functionality > > + that does not need to be exposed as a stable interface to applications. > > + > > +* sysfs is a good way to expose the state of an in-kernel object > > + that is not tied to a file descriptor. > > + > > +* configfs can be used for more complex configuration than sysfs > > + > > +* A custom file system can provide extra flexibility with a simple > > + user interface but add a lot of complexity in the implementation. > > adds ... to
done.
Thanks for all the suggestions!
Arnd
| |