On Fri, 23 Feb 2024 11:41:56 -0600
John Groves <[email protected]> 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 <[email protected]>
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;
> +}
On 24/02/26 01:25PM, Jonathan Cameron wrote:
> On Fri, 23 Feb 2024 11:41:56 -0600
> John Groves <[email protected]> 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 <[email protected]>
>
> 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