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