Introduce simple_open()

I noticed that if I want to tweak debugfs file ops I need to copy the implementation of default_open() in fs/debugfs/file.c. I went to look for an exported function that would be common across all debugfs users but I couldn't find any.

This patchset moves default_open() to libfs so that other filesystems can use it (although almost 99% of users are debugfs clients). I wrote a semantic patch to find all the kernel users and ended up with a nice diff:

 58 files changed, 150 insertions(+), 560 deletions(-)

I will send those patches as a follow up to these two once we settle on naming and merging strategies for the patches.

Stephen Boyd (2):
libfs: Add simple_open()
coccinelle: semantic patch for simple_open()

 fs/libfs.c                               |    8 +++++++
 include/linux/fs.h                       |    1 +
 scripts/coccinelle/api/simple_open.cocci |   34 ++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+)
 create mode 100644 scripts/coccinelle/api/simple_open.cocci

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

debugfs and a few other drivers use a version of simple_open() to pass a pointer from the file to the read/write file ops. Add support for this simple case to libfs so that we can remove the many duplicate copies of this simple function.

---
 fs/libfs.c         |    8 ++++++++
 include/linux/fs.h |    1 +
 2 files changed, 9 insertions(+)

index 7c895a7..f5af1d6 100644
@@ -264,6 +264,13 @@ Enomem:
 	return ERR_PTR(-ENOMEM);
 }
 
+int simple_open(struct inode *inode, struct file *file)
+{
+	if (inode->i_private)
+		file->private_data = inode->i_private;
+	return 0;
+}
+
 int simple_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = old_dentry->d_inode;

@@ -984,6 +991,7 @@ EXPORT_SYMBOL(simple_dir_operations);
 EXPORT_SYMBOL(simple_empty);
 EXPORT_SYMBOL(simple_fill_super);
 EXPORT_SYMBOL(simple_getattr);
+EXPORT_SYMBOL(simple_open);
 EXPORT_SYMBOL(simple_link);
 EXPORT_SYMBOL(simple_lookup);
 EXPORT_SYMBOL(simple_pin_fs);

index f42a557..7871f90 100644
@@ -2517,6 +2517,7 @@ extern int dcache_readdir(struct file *, void *, filldir_t);
 extern int simple_setattr(struct dentry *, struct iattr *);
 extern int simple_getattr(struct vfsmount *, struct dentry *, struct kstat *);
 extern int simple_statfs(struct dentry *, struct kstatfs *);
+extern int simple_open(struct inode *inode, struct file *file);
 extern int simple_link(struct dentry *, struct inode *, struct dentry *);
 extern int simple_unlink(struct inode *, struct dentry *);
 extern int simple_rmdir(struct inode *, struct dentry *);

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Find instances of an open-coded simple_open() and replace them with calls to simple_open().

---

This seems to eat the definition of simple_open() as well. Is there a way to say only remove both the function and the reference to the function so that libfs survives?

 scripts/coccinelle/api/simple_open.cocci |   34 ++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 scripts/coccinelle/api/simple_open.cocci

new file mode 100644
index 0000000..956d04d
@@ -0,0 +1,34 @@
+// This removes an open coded simple_open() function
+// and replaces file operations references to the function
+// with simple_open() instead.
+//
+
+virtual patch
+virtual context
+
+@ open @
+identifier open_f;
+identifier i, f;
+@@
+-int open_f(struct inode *i, struct file *f)
+-{
+(
+-if (i->i_private)
+-f->private_data = i->i_private;
+|
+-f->private_data = i->i_private;
+)
+-return 0;
+-}
+
+@ has_open depends on open @
+identifier fops;
+identifier open.open_f;
+@@
+struct file_operations fops = {
+...
+-.open = open_f,
++.open = simple_open,
+...
+};
+

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Try identifier open_f != simple_open;

Is there a header file that should be present?

Your transformation rules should include depends on patch in the very first line, after the rule name.

Can you make a report rule? If not, I will do it. Someone asked for that to always be present.

thanks,
julia

Ah thanks. That works.

It would be <include/fs.h> which should already be there if there is a file_operations structure.

I tried but failed. I get:

nit_defs_builtins: /usr/share/coccinelle/standard.h
between: p start 15 p end 15 m1 start 15 m1 end 15 m2 start 15 m2 end 15

-int -open:open_f-struct -inode -*-open:i-, -struct -file -*-open:f
Fatal error: exception Failure("The semantic patch is structured in a way that may give bad results with isomorphisms. Please try to rewrite it by moving + code out from -/context terms.")

When I add this:

@r1 depends on org || report @
identifier open_f != simple_open;
identifier i, f;
position p1;
@@
int open_f@p1(struct inode *i, struct file *f)
{
(
if (i->i_private)
f->private_data = i->i_private;
|
f->private_data = i->i_private;
)
return 0;
}

@script:python depends on org@
p1 << r1.p1;
@@

cocci.print_main("extra function",p1)

Any ideas what's wrong?

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.