On 26/08/05 10:14PM, Darrick J. Wong wrote:
> On Mon, Aug 03, 2026 at 02:29:07AM +0000, John Groves wrote:
> > From: John Groves <[email protected]>
> >
> > This commit introduces fs/famfs/famfs_file.c and the famfs
> > file_operations for read/write.
> >
> > This is not usable yet because:
> >
> > * It calls dax_iomap_rw() with NULL iomap_ops (which will be
> > introduced in a subsequent commit).
> > * famfs_ioctl() is coming in a later commit, and it is necessary
> > to map a file to a memory allocation.
> >
> > Signed-off-by: John Groves <[email protected]>
> > ---
> > fs/famfs/Makefile | 2 +-
> > fs/famfs/famfs_file.c | 138 ++++++++++++++++++++++++++++++++++++++
> > fs/famfs/famfs_inode.c | 2 +-
> > fs/famfs/famfs_internal.h | 2 +
> > 4 files changed, 142 insertions(+), 2 deletions(-)
> > create mode 100644 fs/famfs/famfs_file.c
> >
> > diff --git a/fs/famfs/Makefile b/fs/famfs/Makefile
> > index 62230bcd6793..8cac90c090a4 100644
> > --- a/fs/famfs/Makefile
> > +++ b/fs/famfs/Makefile
> > @@ -2,4 +2,4 @@
> >
> > obj-$(CONFIG_FAMFS) += famfs.o
> >
> > -famfs-y := famfs_inode.o
> > +famfs-y := famfs_inode.o famfs_file.o
> > diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
> > new file mode 100644
> > index 000000000000..e192b573c51f
> > --- /dev/null
> > +++ b/fs/famfs/famfs_file.c
> > @@ -0,0 +1,138 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * famfs - dax file system for shared fabric-attached memory
> > + *
> > + * Copyright 2023-2024 Micron Technology, Inc.
> > + *
> > + * This file system, originally based on ramfs the dax support from xfs,
> > + * is intended to allow multiple host systems to mount a common file system
> > + * view of dax files that map to shared memory.
> > + */
> > +
> > +#include <linux/fs.h>
> > +#include <linux/mm.h>
> > +#include <linux/dax.h>
> > +#include <linux/iomap.h>
> > +
> > +#include "famfs_internal.h"
> > +
> > +/*********************************************************************
> > + * file_operations
> > + */
> > +
> > +/* Reject I/O to files that aren't in a valid state */
> > +static ssize_t
> > +famfs_file_invalid(struct inode *inode)
> > +{
> > + if (!IS_DAX(inode)) {
> > + pr_debug("%s: inode %llx IS_DAX is false\n",
> > + __func__, (u64)inode);
> > + return -ENXIO;
> > + }
> > + return 0;
> > +}
> > +
> > +static ssize_t
> > +famfs_rw_prep(struct kiocb *iocb, struct iov_iter *ubuf)
> > +{
> > + struct inode *inode = iocb->ki_filp->f_mapping->host;
> > + struct super_block *sb = inode->i_sb;
> > + struct famfs_fs_info *fsi = sb->s_fs_info;
> > + size_t i_size = i_size_read(inode);
> > + size_t count = iov_iter_count(ubuf);
> > + size_t max_count;
> > + ssize_t rc;
> > +
> > + if (fsi->deverror)
> > + return -ENODEV;
> > +
> > + rc = famfs_file_invalid(inode);
> > + if (rc)
> > + return rc;
> > +
> > + /* Avoid unsigned underflow if position is past EOF */
> > + if (iocb->ki_pos >= i_size)
> > + max_count = 0;
> > + else
> > + max_count = i_size - iocb->ki_pos;
> > +
> > + if (count > max_count)
> > + iov_iter_truncate(ubuf, max_count);
> > +
> > + if (!iov_iter_count(ubuf))
> > + return 0;
> > +
> > + return rc;
> > +}
> > +
> > +static ssize_t
> > +famfs_dax_read_iter(struct kiocb *iocb, struct iov_iter *to)
> > +{
> > + struct inode *inode = iocb->ki_filp->f_mapping->host;
> > + ssize_t rc;
> > +
> > + /* dax_iomap_rw() requires i_rwsem held (shared for read) */
> > + inode_lock_shared(inode);
> > + rc = famfs_rw_prep(iocb, to);
> > + if (rc || !iov_iter_count(to)) {
> > + inode_unlock_shared(inode);
> > + return rc;
> > + }
> > +
> > + rc = dax_iomap_rw(iocb, to, NULL /*&famfs_iomap_ops */);
> > + inode_unlock_shared(inode);
> > +
> > + file_accessed(iocb->ki_filp);
>
> Is it really accessed if rc != 0?
Good point; looks like only if rc > 0. Will update, thanks.
>
> > + return rc;
> > +}
> > +
> > +/**
> > + * famfs_dax_write_iter()
> > + *
> > + * We need our own write-iter in order to prevent append
> > + *
> > + * @iocb:
> > + * @from: iterator describing the user memory source for the write
> > + */
> > +static ssize_t
> > +famfs_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
> > +{
> > + struct inode *inode = iocb->ki_filp->f_mapping->host;
> > + struct famfs_fs_info *fsi = inode->i_sb->s_fs_info;
> > + ssize_t rc;
> > +
> > + if (!famfs_opt_enabled(fsi, FAMFS_OPT_WRITE))
> > + return -EPERM;
> > +
> > + /* dax_iomap_rw() requires i_rwsem held (exclusive for write) */
> > + inode_lock(inode);
> > + rc = famfs_rw_prep(iocb, from);
> > + if (rc || !iov_iter_count(from)) {
> > + inode_unlock(inode);
> > + return rc;
> > + }
> > +
> > + rc = dax_iomap_rw(iocb, from, NULL /*&famfs_iomap_ops*/);
>
> What happens if you pass a null iomap ops? TBH I was expecting you to
> define the iomap ops with a dummy ->iomap_begin that returns EIO or
> something.
If we actually called dax_iomap_rw() with null iomap ops, it would
hork. However, if we called it with null iomap_ops->iomap_begin it
will also hork. I could introduce the dax_iomap_rw call later, when
sufficient code is in, but that might require (void) declarations
to squelch the compiler about unreferenced stuff.
So dummy iomap_ops won't actually work. My objective was to drop in
bite-sized chunks that were functionally coherent, until it's complete
(in commit 12).
I suspect multiple of these commits would do something bad if you
tried to run them before you had all.
Given all that, I'm inclined to leave it alone.
Hmm, I could keep things where all commits compile, but cause
module_init to fail until all commits are in. Then it couldn't do
any harm to try to try running incomplete famfs.
I think I'll do that...
>
> > + inode_unlock(inode);
> > + return rc;
>
> Do you need to update mtime here?
>
> --D
Yeah, I guess I should - will do.
FYI file times in famfs have limited usefulness, because they don't
propagate in the cluster.
<snip>
