On 25/04/20 08:43PM, Randy Dunlap wrote:
> Hi,
Hi Randy - thanks for the review!
>
> On 4/20/25 6:33 PM, John Groves wrote:
> > * The new GET_DAXDEV message/response is enabled
> > * The command it triggered by the update_daxdev_table() call, if there
> > are any daxdevs in the subject fmap that are not represented in the
> > daxdev_dable yet.
> >
> > Signed-off-by: John Groves <j...@groves.net>
> > ---
> > fs/fuse/famfs.c | 281 ++++++++++++++++++++++++++++++++++++--
> > fs/fuse/famfs_kfmap.h | 23 ++++
> > fs/fuse/fuse_i.h | 4 +
> > fs/fuse/inode.c | 2 +
> > fs/namei.c | 1 +
> > include/uapi/linux/fuse.h | 15 ++
> > 6 files changed, 316 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/fuse/famfs.c b/fs/fuse/famfs.c
> > index e62c047d0950..2e182cb7d7c9 100644
> > --- a/fs/fuse/famfs.c
> > +++ b/fs/fuse/famfs.c
> > @@ -20,6 +20,250 @@
> > #include "famfs_kfmap.h"
> > #include "fuse_i.h"
> >
> > +/*
> > + * famfs_teardown()
> > + *
> > + * Deallocate famfs metadata for a fuse_conn
> > + */
> > +void
> > +famfs_teardown(struct fuse_conn *fc)
>
> Is this function formatting prevalent in fuse?
> It's a bit different from most Linux.
> (many locations throughout the patch set)
I'll check and clean it up if not; function names beginning in column 1 is a
"thing", but I'll normalize to nearby standards.
>
> > +{
> > + struct famfs_dax_devlist *devlist = fc->dax_devlist;
> > + int i;
> > +
> > + fc->dax_devlist = NULL;
> > +
> > + if (!devlist)
> > + return;
> > +
> > + if (!devlist->devlist)
> > + goto out;
> > +
> > + /* Close & release all the daxdevs in our table */
> > + for (i = 0; i < devlist->nslots; i++) {
> > + if (devlist->devlist[i].valid && devlist->devlist[i].devp)
> > + fs_put_dax(devlist->devlist[i].devp, fc);
> > + }
> > + kfree(devlist->devlist);
> > +
> > +out:
> > + kfree(devlist);
> > +}
> > +
> > +static int
> > +famfs_verify_daxdev(const char *pathname, dev_t *devno)
> > +{
> > + struct inode *inode;
> > + struct path path;
> > + int err;
> > +
> > + if (!pathname || !*pathname)
> > + return -EINVAL;
> > +
> > + err = kern_path(pathname, LOOKUP_FOLLOW, &path);
> > + if (err)
> > + return err;
> > +
> > + inode = d_backing_inode(path.dentry);
> > + if (!S_ISCHR(inode->i_mode)) {
> > + err = -EINVAL;
> > + goto out_path_put;
> > + }
> > +
> > + if (!may_open_dev(&path)) { /* had to export this */
> > + err = -EACCES;
> > + goto out_path_put;
> > + }
> > +
> > + *devno = inode->i_rdev;
> > +
> > +out_path_put:
> > + path_put(&path);
> > + return err;
> > +}
> > +
> > +/**
> > + * famfs_fuse_get_daxdev()
>
> Missing " - <short function description>"
> but then it's a static function, so kernel-doc is not required.
> It's up to you, but please use full kernel-doc notation if using kernel-doc.
Thank you - and sorry for being a bit sloppy on this stuff. I'm caching fixes
for all your comments along these lines into a branch for the next version of
the series.
Snipping the rest, but will address it all.
Thanks,
John
