diff mbox series

[RFC,12/20] famfs: Add inode_operations and file_system_type

Message ID bd2bbdd7523d1c74ca559d8912984e7facabe5c6.1708709155.git.john@groves.net (mailing list archive)
State New
Headers show
Series Introduce the famfs shared-memory file system | expand

Commit Message

John Groves Feb. 23, 2024, 5:41 p.m. UTC
This commit introduces the famfs inode_operations. There is nothing really
unique to famfs here in the inode_operations..

This commit also introduces the famfs_file_system_type struct and the
famfs_kill_sb() function.

Signed-off-by: John Groves <john@groves.net>
---
 fs/famfs/famfs_inode.c | 132 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)

Comments

Jonathan Cameron Feb. 26, 2024, 1:25 p.m. UTC | #1
On Fri, 23 Feb 2024 11:41:56 -0600
John Groves <John@Groves.net> wrote:

> This commit introduces the famfs inode_operations. There is nothing really
> unique to famfs here in the inode_operations..
> 
> This commit also introduces the famfs_file_system_type struct and the
> famfs_kill_sb() function.
> 
> Signed-off-by: John Groves <john@groves.net>

Trivial comments only.

> +
> +/*
> + * File creation. Allocate an inode, and we're done..
> + */
> +/* SMP-safe */
> +static int
> +famfs_mknod(
> +	struct mnt_idmap *idmap,
> +	struct inode     *dir,
> +	struct dentry    *dentry,
> +	umode_t           mode,
> +	dev_t             dev)
> +{
> +	struct inode *inode = famfs_get_inode(dir->i_sb, dir, mode, dev);
> +	int error           = -ENOSPC;
> +
> +	if (inode) {

As below. I would flip it for cleaner code/ shorter indent etc.

> +		struct timespec64       tv;
> +
> +		d_instantiate(dentry, inode);
> +		dget(dentry);	/* Extra count - pin the dentry in core */
> +		error = 0;
> +		tv = inode_set_ctime_current(inode);
> +		inode_set_mtime_to_ts(inode, tv);
> +		inode_set_atime_to_ts(inode, tv);
> +	}
> +	return error;
> +}
> +
> +static int famfs_mkdir(
> +	struct mnt_idmap *idmap,
> +	struct inode     *dir,
> +	struct dentry    *dentry,
> +	umode_t           mode)
> +{
> +	int retval = famfs_mknod(&nop_mnt_idmap, dir, dentry, mode | S_IFDIR, 0);
> +
> +	if (!retval)
> +		inc_nlink(dir);

Copy local style, so fine if this is common pattern, otherwise I'd go for
consistent error cases out of line as easier for us sleepy caffeine 
deprived reviewers.


	if (retval)
		return retval;

	inc_nlink(dir);

	return 0;
> +
> +	return retval;
> +}
> +
> +static int famfs_create(
> +	struct mnt_idmap *idmap,
> +	struct inode     *dir,
> +	struct dentry    *dentry,
> +	umode_t           mode,
> +	bool              excl)
> +{
> +	return famfs_mknod(&nop_mnt_idmap, dir, dentry, mode | S_IFREG, 0);
> +}
> +
> +static int famfs_symlink(
> +	struct mnt_idmap *idmap,
> +	struct inode     *dir,
> +	struct dentry    *dentry,
> +	const char       *symname)
> +{
> +	struct inode *inode;
> +	int error = -ENOSPC;
> +
> +	inode = famfs_get_inode(dir->i_sb, dir, S_IFLNK | 0777, 0);
	if (!inode)
		return -ENOSPC;

> +	if (inode) {
> +		int l = strlen(symname)+1;
> +
> +		error = page_symlink(inode, symname, l);
	if (error) {
		iput(inode);
		return error;
	}
	
	...

> +		if (!error) {
> +			struct timespec64       tv;
> +
> +			d_instantiate(dentry, inode);
> +			dget(dentry);
> +			tv = inode_set_ctime_current(inode);
> +			inode_set_mtime_to_ts(inode, tv);
> +			inode_set_atime_to_ts(inode, tv);
> +		} else
> +			iput(inode);
> +	}
> +	return error;
> +}
John Groves Feb. 26, 2024, 10:53 p.m. UTC | #2
On 24/02/26 01:25PM, Jonathan Cameron wrote:
> On Fri, 23 Feb 2024 11:41:56 -0600
> John Groves <John@Groves.net> wrote:
> 
> > This commit introduces the famfs inode_operations. There is nothing really
> > unique to famfs here in the inode_operations..
> > 
> > This commit also introduces the famfs_file_system_type struct and the
> > famfs_kill_sb() function.
> > 
> > Signed-off-by: John Groves <john@groves.net>
> 
> Trivial comments only.
> 
> > +
> > +/*
> > + * File creation. Allocate an inode, and we're done..
> > + */
> > +/* SMP-safe */
> > +static int
> > +famfs_mknod(
> > +	struct mnt_idmap *idmap,
> > +	struct inode     *dir,
> > +	struct dentry    *dentry,
> > +	umode_t           mode,
> > +	dev_t             dev)
> > +{
> > +	struct inode *inode = famfs_get_inode(dir->i_sb, dir, mode, dev);
> > +	int error           = -ENOSPC;
> > +
> > +	if (inode) {
> 
> As below. I would flip it for cleaner code/ shorter indent etc.

Nice - done.

> 
> > +		struct timespec64       tv;
> > +
> > +		d_instantiate(dentry, inode);
> > +		dget(dentry);	/* Extra count - pin the dentry in core */
> > +		error = 0;
> > +		tv = inode_set_ctime_current(inode);
> > +		inode_set_mtime_to_ts(inode, tv);
> > +		inode_set_atime_to_ts(inode, tv);
> > +	}
> > +	return error;
> > +}
> > +
> > +static int famfs_mkdir(
> > +	struct mnt_idmap *idmap,
> > +	struct inode     *dir,
> > +	struct dentry    *dentry,
> > +	umode_t           mode)
> > +{
> > +	int retval = famfs_mknod(&nop_mnt_idmap, dir, dentry, mode | S_IFDIR, 0);
> > +
> > +	if (!retval)
> > +		inc_nlink(dir);
> 
> Copy local style, so fine if this is common pattern, otherwise I'd go for
> consistent error cases out of line as easier for us sleepy caffeine 
> deprived reviewers.
> 
> 
> 	if (retval)
> 		return retval;
> 
> 	inc_nlink(dir);
> 
> 	return 0;

Agree, done.

> > +
> > +	return retval;
> > +}
> > +
> > +static int famfs_create(
> > +	struct mnt_idmap *idmap,
> > +	struct inode     *dir,
> > +	struct dentry    *dentry,
> > +	umode_t           mode,
> > +	bool              excl)
> > +{
> > +	return famfs_mknod(&nop_mnt_idmap, dir, dentry, mode | S_IFREG, 0);
> > +}
> > +
> > +static int famfs_symlink(
> > +	struct mnt_idmap *idmap,
> > +	struct inode     *dir,
> > +	struct dentry    *dentry,
> > +	const char       *symname)
> > +{
> > +	struct inode *inode;
> > +	int error = -ENOSPC;
> > +
> > +	inode = famfs_get_inode(dir->i_sb, dir, S_IFLNK | 0777, 0);
> 	if (!inode)
> 		return -ENOSPC;
> 
> > +	if (inode) {
> > +		int l = strlen(symname)+1;
> > +
> > +		error = page_symlink(inode, symname, l);
> 	if (error) {
> 		iput(inode);
> 		return error;
> 	}
> 	
> 	...
> 

Right, I like it. This was some tortured conditioning, which came from
somewhere. I deny responsibility :D

Thanks,
John
diff mbox series

Patch

diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c
index f98f82962d7b..ab46ec50b70d 100644
--- a/fs/famfs/famfs_inode.c
+++ b/fs/famfs/famfs_inode.c
@@ -85,6 +85,109 @@  static struct inode *famfs_get_inode(
 	return inode;
 }
 
+/***************************************************************************
+ * famfs inode_operations: these are currently pretty much boilerplate
+ */
+
+static const struct inode_operations famfs_file_inode_operations = {
+	/* All generic */
+	.setattr	   = simple_setattr,
+	.getattr	   = simple_getattr,
+};
+
+
+/*
+ * File creation. Allocate an inode, and we're done..
+ */
+/* SMP-safe */
+static int
+famfs_mknod(
+	struct mnt_idmap *idmap,
+	struct inode     *dir,
+	struct dentry    *dentry,
+	umode_t           mode,
+	dev_t             dev)
+{
+	struct inode *inode = famfs_get_inode(dir->i_sb, dir, mode, dev);
+	int error           = -ENOSPC;
+
+	if (inode) {
+		struct timespec64       tv;
+
+		d_instantiate(dentry, inode);
+		dget(dentry);	/* Extra count - pin the dentry in core */
+		error = 0;
+		tv = inode_set_ctime_current(inode);
+		inode_set_mtime_to_ts(inode, tv);
+		inode_set_atime_to_ts(inode, tv);
+	}
+	return error;
+}
+
+static int famfs_mkdir(
+	struct mnt_idmap *idmap,
+	struct inode     *dir,
+	struct dentry    *dentry,
+	umode_t           mode)
+{
+	int retval = famfs_mknod(&nop_mnt_idmap, dir, dentry, mode | S_IFDIR, 0);
+
+	if (!retval)
+		inc_nlink(dir);
+
+	return retval;
+}
+
+static int famfs_create(
+	struct mnt_idmap *idmap,
+	struct inode     *dir,
+	struct dentry    *dentry,
+	umode_t           mode,
+	bool              excl)
+{
+	return famfs_mknod(&nop_mnt_idmap, dir, dentry, mode | S_IFREG, 0);
+}
+
+static int famfs_symlink(
+	struct mnt_idmap *idmap,
+	struct inode     *dir,
+	struct dentry    *dentry,
+	const char       *symname)
+{
+	struct inode *inode;
+	int error = -ENOSPC;
+
+	inode = famfs_get_inode(dir->i_sb, dir, S_IFLNK | 0777, 0);
+	if (inode) {
+		int l = strlen(symname)+1;
+
+		error = page_symlink(inode, symname, l);
+		if (!error) {
+			struct timespec64       tv;
+
+			d_instantiate(dentry, inode);
+			dget(dentry);
+			tv = inode_set_ctime_current(inode);
+			inode_set_mtime_to_ts(inode, tv);
+			inode_set_atime_to_ts(inode, tv);
+		} else
+			iput(inode);
+	}
+	return error;
+}
+
+static const struct inode_operations famfs_dir_inode_operations = {
+	.create		= famfs_create,
+	.lookup		= simple_lookup,
+	.link		= simple_link,
+	.unlink		= simple_unlink,
+	.symlink	= famfs_symlink,
+	.mkdir		= famfs_mkdir,
+	.rmdir		= simple_rmdir,
+	.mknod		= famfs_mknod,
+	.rename		= simple_rename,
+};
+
 /**********************************************************************************
  * famfs super_operations
  *
@@ -329,5 +432,34 @@  static int famfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
+static void famfs_kill_sb(struct super_block *sb)
+{
+	struct famfs_fs_info *fsi = sb->s_fs_info;
+
+	mutex_lock(&famfs_context_mutex);
+	list_del(&fsi->fsi_list);
+	mutex_unlock(&famfs_context_mutex);
+
+	if (fsi->bdev_handle)
+		bdev_release(fsi->bdev_handle);
+	if (fsi->dax_devp)
+		fs_put_dax(fsi->dax_devp, fsi);
+	if (fsi->dax_filp) /* This only happens if it's char dax */
+		filp_close(fsi->dax_filp, NULL);
+
+	if (fsi && fsi->rootdev)
+		kfree(fsi->rootdev);
+	kfree(fsi);
+	kill_litter_super(sb);
+}
+
+#define MODULE_NAME "famfs"
+static struct file_system_type famfs_fs_type = {
+	.name		  = MODULE_NAME,
+	.init_fs_context  = famfs_init_fs_context,
+	.parameters	  = famfs_fs_parameters,
+	.kill_sb	  = famfs_kill_sb,
+	.fs_flags	  = FS_USERNS_MOUNT,
+};
 
 MODULE_LICENSE("GPL");