From bbb0a4247aaf1eabbd6d87750eafe99c577920f7 Mon Sep 17 00:00:00 2001 From: Jonathan Corbet Date: Fri, 16 Jan 2009 09:49:50 -0700 Subject: [PATCH 1/5] Document Reported-by in SubmittingPatches Randy pointed out that the Reported-By tag should be documented with the others in SubmittingPatches. Reported-by: Randy Dunlap Signed-off-by: Jonathan Corbet --- Documentation/SubmittingPatches | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index f309d3c6221c..6f29e29555c0 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -405,7 +405,14 @@ person it names. This tag documents that potentially interested parties have been included in the discussion -14) Using Tested-by: and Reviewed-by: +14) Using Reported-by:, Tested-by: and Reviewed-by: + +If this patch fixes a problem reported by somebody else, consider adding a +Reported-by: tag to credit the reporter for their contribution. Please +note that this tag should not be added without the reporter's permission, +especially if the problem was not reported in a public forum. That said, +if we diligently credit our bug reporters, they will, hopefully, be +inspired to help us again in the future. A Tested-by: tag indicates that the patch has been successfully tested (in some environment) by the person named. This tag informs maintainers that From 5d98932ab0acb699dc56d9e252f056b9b2cdab25 Mon Sep 17 00:00:00 2001 From: Jonathan Corbet Date: Tue, 21 Apr 2009 13:33:06 -0600 Subject: [PATCH 2/5] docs: Encourage better changelogs in the development process document Add a couple of paragraphs to the "patch formatting" section on how patches should be described. This text is shamelessly cribbed from suggestions posted by Rusty Russell. Signed-off-by: Jonathan Corbet --- Documentation/development-process/5.Posting | 31 +++++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/Documentation/development-process/5.Posting b/Documentation/development-process/5.Posting index dd48132a74dd..f622c1e9f0f9 100644 --- a/Documentation/development-process/5.Posting +++ b/Documentation/development-process/5.Posting @@ -119,7 +119,7 @@ which takes quite a bit of time and thought after the "real work" has been done. When done properly, though, it is time well spent. -5.4: PATCH FORMATTING +5.4: PATCH FORMATTING AND CHANGELOGS So now you have a perfect series of patches for posting, but the work is not done quite yet. Each patch needs to be formatted into a message which @@ -146,8 +146,33 @@ that end, each patch will be composed of the following: - One or more tag lines, with, at a minimum, one Signed-off-by: line from the author of the patch. Tags will be described in more detail below. -The above three items should, normally, be the text used when committing -the change to a revision control system. They are followed by: +The items above, together, form the changelog for the patch. Writing good +changelogs is a crucial but often-neglected art; it's worth spending +another moment discussing this issue. When writing a changelog, you should +bear in mind that a number of different people will be reading your words. +These include subsystem maintainers and reviewers who need to decide +whether the patch should be included, distributors and other maintainers +trying to decide whether a patch should be backported to other kernels, bug +hunters wondering whether the patch is responsible for a problem they are +chasing, users who want to know how the kernel has changed, and more. A +good changelog conveys the needed information to all of these people in the +most direct and concise way possible. + +To that end, the summary line should describe the effects of and motivation +for the change as well as possible given the one-line constraint. The +detailed description can then amplify on those topics and provide any +needed additional information. If the patch fixes a bug, cite the commit +which introduced the bug if possible. If a problem is associated with +specific log or compiler output, include that output to help others +searching for a solution to the same problem. If the change is meant to +support other changes coming in later patch, say so. If internal APIs are +changed, detail those changes and how other developers should respond. In +general, the more you can put yourself into the shoes of everybody who will +be reading your changelog, the better that changelog (and the kernel as a +whole) will be. + +Needless to say, the changelog should be the text used when committing the +change to a revision control system. It will be followed by: - The patch itself, in the unified ("-u") patch format. Using the "-p" option to diff will associate function names with changes, making the From 5801da1b2f1207da21271ffd6768cd40a6c7f1c4 Mon Sep 17 00:00:00 2001 From: Pavel Machek Date: Thu, 4 Jun 2009 16:26:50 +0200 Subject: [PATCH 3/5] SubmittingPatches: fix typo Fix typo. Signed-off-by: Pavel Machek Signed-off-by: Jonathan Corbet --- Documentation/SubmittingPatches | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 6f29e29555c0..71b6da2b7175 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -451,7 +451,7 @@ offer a Reviewed-by tag for a patch. This tag serves to give credit to reviewers and to inform maintainers of the degree of review which has been done on the patch. Reviewed-by: tags, when supplied by reviewers known to understand the subject area and to perform thorough reviews, will normally -increase the liklihood of your patch getting into the kernel. +increase the likelihood of your patch getting into the kernel. 15) The canonical patch format From 2ae19acaa50a09c1099956efb895c0aca74ab050 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Thu, 16 Apr 2009 07:44:45 -0400 Subject: [PATCH 4/5] Documentation: Add "how to write a good patch summary" to SubmittingPatches Unfortunately many patch submissions are arriving with painfully poor patch descriptions. As a result of the discussion on LKML: http://lkml.org/lkml/2009/4/15/296 explain how to submit a better patch description, in the (perhaps vain) hope that maintainers won't end up having to rewrite the git commit logs as often as they do today. Signed-off-by: "Theodore Ts'o" Cc: Ingo Molnar Signed-off-by: Jonathan Corbet --- Documentation/SubmittingPatches | 65 ++++++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 14 deletions(-) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 71b6da2b7175..6c456835c1fd 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -91,6 +91,10 @@ Be as specific as possible. The WORST descriptions possible include things like "update driver X", "bug fix for driver X", or "this patch includes updates for subsystem X. Please apply." +The maintainer will thank you if you write your patch description in a +form which can be easily pulled into Linux's source code management +system, git, as a "commit log". See #15, below. + If your description starts to get long, that's a sign that you probably need to split up your patch. See #3, next. @@ -492,12 +496,33 @@ phrase" should not be a filename. Do not use the same "summary phrase" for every patch in a whole patch series (where a "patch series" is an ordered sequence of multiple, related patches). -Bear in mind that the "summary phrase" of your email becomes -a globally-unique identifier for that patch. It propagates -all the way into the git changelog. The "summary phrase" may -later be used in developer discussions which refer to the patch. -People will want to google for the "summary phrase" to read -discussion regarding that patch. +Bear in mind that the "summary phrase" of your email becomes a +globally-unique identifier for that patch. It propagates all the way +into the git changelog. The "summary phrase" may later be used in +developer discussions which refer to the patch. People will want to +google for the "summary phrase" to read discussion regarding that +patch. It will also be the only thing that people may quickly see +when, two or three months later, they are going through perhaps +thousands of patches using tools such as "gitk" or "git log +--oneline". + +For these reasons, the "summary" must be no more than 70-75 +characters, and it must describe both what the patch changes, as well +as why the patch might be necessary. It is challenging to be both +succinct and descriptive, but that is what a well-written summary +should do. + +The "summary phrase" may be prefixed by tags enclosed in square +brackets: "Subject: [PATCH tag] ". The tags are not +considered part of the summary phrase, but describe how the patch +should be treated. Common tags might include a version descriptor if +the multiple versions of the patch have been sent out in response to +comments (i.e., "v1, v2, v3"), or "RFC" to indicate a request for +comments. If there are four patches in a patch series the individual +patches may be numbered like this: 1/4, 2/4, 3/4, 4/4. This assures +that developers understand the order in which the patches should be +applied and that they have reviewed or applied all of the patches in +the patch series. A couple of example Subjects: @@ -517,19 +542,31 @@ the patch author in the changelog. The explanation body will be committed to the permanent source changelog, so should make sense to a competent reader who has long since forgotten the immediate details of the discussion that might -have led to this patch. +have led to this patch. Including symptoms of the failure which the +patch addresses (kernel log messages, oops messages, etc.) is +especially useful for people who might be searching the commit logs +looking for the applicable patch. If a patch fixes a compile failure, +it may not be necessary to include _all_ of the compile failures; just +enough that it is likely that someone searching for the patch can find +it. As in the "summary phrase", it is important to be both succinct as +well as descriptive. The "---" marker line serves the essential purpose of marking for patch handling tools where the changelog message ends. One good use for the additional comments after the "---" marker is for -a diffstat, to show what files have changed, and the number of inserted -and deleted lines per file. A diffstat is especially useful on bigger -patches. Other comments relevant only to the moment or the maintainer, -not suitable for the permanent changelog, should also go here. -Use diffstat options "-p 1 -w 70" so that filenames are listed from the -top of the kernel source tree and don't use too much horizontal space -(easily fit in 80 columns, maybe with some indentation). +a diffstat, to show what files have changed, and the number of +inserted and deleted lines per file. A diffstat is especially useful +on bigger patches. Other comments relevant only to the moment or the +maintainer, not suitable for the permanent changelog, should also go +here. A good example of such comments might be "patch changelogs" +which describe what has changed between the v1 and v2 version of the +patch. + +If you are going to include a diffstat after the "---" marker, please +use diffstat options "-p 1 -w 70" so that filenames are listed from +the top of the kernel source tree and don't use too much horizontal +space (easily fit in 80 columns, maybe with some indentation). See more details on the proper patch format in the following references. From f89d7eaf6c34828070f407d0e04b73127f176ec5 Mon Sep 17 00:00:00 2001 From: Jonathan Corbet Date: Thu, 4 Jun 2009 16:35:25 -0600 Subject: [PATCH 5/5] Document the debugfs API This is an updated document covering the internal API for the debugfs filesystem. Thanks to Shen Feng for suggesting that I put this text here and noting that the old LWN version was rather out of date. Acked-by: Greg Kroah-Hartman Reported-by: Shen Feng Signed-off-by: Jonathan Corbet --- Documentation/filesystems/debugfs.txt | 158 ++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 Documentation/filesystems/debugfs.txt diff --git a/Documentation/filesystems/debugfs.txt b/Documentation/filesystems/debugfs.txt new file mode 100644 index 000000000000..ed52af60c2d8 --- /dev/null +++ b/Documentation/filesystems/debugfs.txt @@ -0,0 +1,158 @@ +Copyright 2009 Jonathan Corbet + +Debugfs exists as a simple way for kernel developers to make information +available to user space. Unlike /proc, which is only meant for information +about a process, or sysfs, which has strict one-value-per-file rules, +debugfs has no rules at all. Developers can put any information they want +there. The debugfs filesystem is also intended to not serve as a stable +ABI to user space; in theory, there are no stability constraints placed on +files exported there. The real world is not always so simple, though [1]; +even debugfs interfaces are best designed with the idea that they will need +to be maintained forever. + +Debugfs is typically mounted with a command like: + + mount -t debugfs none /sys/kernel/debug + +(Or an equivalent /etc/fstab line). + +Note that the debugfs API is exported GPL-only to modules. + +Code using debugfs should include . Then, the first order +of business will be to create at least one directory to hold a set of +debugfs files: + + struct dentry *debugfs_create_dir(const char *name, struct dentry *parent); + +This call, if successful, will make a directory called name underneath the +indicated parent directory. If parent is NULL, the directory will be +created in the debugfs root. On success, the return value is a struct +dentry pointer which can be used to create files in the directory (and to +clean it up at the end). A NULL return value indicates that something went +wrong. If ERR_PTR(-ENODEV) is returned, that is an indication that the +kernel has been built without debugfs support and none of the functions +described below will work. + +The most general way to create a file within a debugfs directory is with: + + struct dentry *debugfs_create_file(const char *name, mode_t mode, + struct dentry *parent, void *data, + const struct file_operations *fops); + +Here, name is the name of the file to create, mode describes the access +permissions the file should have, parent indicates the directory which +should hold the file, data will be stored in the i_private field of the +resulting inode structure, and fops is a set of file operations which +implement the file's behavior. At a minimum, the read() and/or write() +operations should be provided; others can be included as needed. Again, +the return value will be a dentry pointer to the created file, NULL for +error, or ERR_PTR(-ENODEV) if debugfs support is missing. + +In a number of cases, the creation of a set of file operations is not +actually necessary; the debugfs code provides a number of helper functions +for simple situations. Files containing a single integer value can be +created with any of: + + struct dentry *debugfs_create_u8(const char *name, mode_t mode, + struct dentry *parent, u8 *value); + struct dentry *debugfs_create_u16(const char *name, mode_t mode, + struct dentry *parent, u16 *value); + struct dentry *debugfs_create_u32(const char *name, mode_t mode, + struct dentry *parent, u32 *value); + struct dentry *debugfs_create_u64(const char *name, mode_t mode, + struct dentry *parent, u64 *value); + +These files support both reading and writing the given value; if a specific +file should not be written to, simply set the mode bits accordingly. The +values in these files are in decimal; if hexadecimal is more appropriate, +the following functions can be used instead: + + struct dentry *debugfs_create_x8(const char *name, mode_t mode, + struct dentry *parent, u8 *value); + struct dentry *debugfs_create_x16(const char *name, mode_t mode, + struct dentry *parent, u16 *value); + struct dentry *debugfs_create_x32(const char *name, mode_t mode, + struct dentry *parent, u32 *value); + +Note that there is no debugfs_create_x64(). + +These functions are useful as long as the developer knows the size of the +value to be exported. Some types can have different widths on different +architectures, though, complicating the situation somewhat. There is a +function meant to help out in one special case: + + struct dentry *debugfs_create_size_t(const char *name, mode_t mode, + struct dentry *parent, + size_t *value); + +As might be expected, this function will create a debugfs file to represent +a variable of type size_t. + +Boolean values can be placed in debugfs with: + + struct dentry *debugfs_create_bool(const char *name, mode_t mode, + struct dentry *parent, u32 *value); + +A read on the resulting file will yield either Y (for non-zero values) or +N, followed by a newline. If written to, it will accept either upper- or +lower-case values, or 1 or 0. Any other input will be silently ignored. + +Finally, a block of arbitrary binary data can be exported with: + + struct debugfs_blob_wrapper { + void *data; + unsigned long size; + }; + + struct dentry *debugfs_create_blob(const char *name, mode_t mode, + struct dentry *parent, + struct debugfs_blob_wrapper *blob); + +A read of this file will return the data pointed to by the +debugfs_blob_wrapper structure. Some drivers use "blobs" as a simple way +to return several lines of (static) formatted text output. This function +can be used to export binary information, but there does not appear to be +any code which does so in the mainline. Note that all files created with +debugfs_create_blob() are read-only. + +There are a couple of other directory-oriented helper functions: + + struct dentry *debugfs_rename(struct dentry *old_dir, + struct dentry *old_dentry, + struct dentry *new_dir, + const char *new_name); + + struct dentry *debugfs_create_symlink(const char *name, + struct dentry *parent, + const char *target); + +A call to debugfs_rename() will give a new name to an existing debugfs +file, possibly in a different directory. The new_name must not exist prior +to the call; the return value is old_dentry with updated information. +Symbolic links can be created with debugfs_create_symlink(). + +There is one important thing that all debugfs users must take into account: +there is no automatic cleanup of any directories created in debugfs. If a +module is unloaded without explicitly removing debugfs entries, the result +will be a lot of stale pointers and no end of highly antisocial behavior. +So all debugfs users - at least those which can be built as modules - must +be prepared to remove all files and directories they create there. A file +can be removed with: + + void debugfs_remove(struct dentry *dentry); + +The dentry value can be NULL, in which case nothing will be removed. + +Once upon a time, debugfs users were required to remember the dentry +pointer for every debugfs file they created so that all files could be +cleaned up. We live in more civilized times now, though, and debugfs users +can call: + + void debugfs_remove_recursive(struct dentry *dentry); + +If this function is passed a pointer for the dentry corresponding to the +top-level directory, the entire hierarchy below that directory will be +removed. + +Notes: + [1] http://lwn.net/Articles/309298/