On 24/02/26 01:25PM, Jonathan Cameron wrote:
> On Fri, 23 Feb 2024 11:41:56 -0600
> John Groves <John@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> 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
