Received-SPF: pass (google.com: domain of linux-kernel+bounces-79425-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <8f62b688-6c14-4eab-b039-7d9a112893f8@infradead.org>
Date: Fri, 23 Feb 2024 17:39:22 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH 07/20] famfs: Add include/linux/famfs_ioctl.h
Content-Language: en-US
To: John Groves <John@Groves.net>, John Groves <jgroves@micron.com>,
 Jonathan Corbet <corbet@lwn.net>, Dan Williams <dan.j.williams@intel.com>,
 Vishal Verma <vishal.l.verma@intel.com>, Dave Jiang <dave.jiang@intel.com>,
 Alexander Viro <viro@zeniv.linux.org.uk>,
 Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
 Matthew Wilcox <willy@infradead.org>, linux-cxl@vger.kernel.org,
 linux-fsdevel@vger.kernel.org, linux-doc@vger.kernel.org,
 linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev
Cc: john@jagalactic.com, Dave Chinner <david@fromorbit.com>,
 Christoph Hellwig <hch@infradead.org>, dave.hansen@linux.intel.com,
 gregory.price@memverge.com
References: <cover.1708709155.git.john@groves.net>
 <b40ca30e4bf689249a8c237909d9a7aaca9861e4.1708709155.git.john@groves.net>
From: Randy Dunlap <rdunlap@infradead.org>
In-Reply-To: <b40ca30e4bf689249a8c237909d9a7aaca9861e4.1708709155.git.john@groves.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

Hi--

On 2/23/24 09:41, John Groves wrote:
> Add uapi include file for famfs. The famfs user space uses ioctl on
> individual files to pass in mapping information and file size. This
> would be hard to do via sysfs or other means, since it's
> file-specific.
> 
> Signed-off-by: John Groves <john@groves.net>
> ---
>  include/uapi/linux/famfs_ioctl.h | 56 ++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 include/uapi/linux/famfs_ioctl.h
> 
> diff --git a/include/uapi/linux/famfs_ioctl.h b/include/uapi/linux/famfs_ioctl.h
> new file mode 100644
> index 000000000000..6b3e6452d02f
> --- /dev/null
> +++ b/include/uapi/linux/famfs_ioctl.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * 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,

      This is confusing to me. Is it just me? ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> + * is intended to allow multiple host systems to mount a common file system
> + * view of dax files that map to shared memory.
> + */
> +#ifndef FAMFS_IOCTL_H
> +#define FAMFS_IOCTL_H
> +
> +#include <linux/ioctl.h>
> +#include <linux/uuid.h>
> +
> +#define FAMFS_MAX_EXTENTS 2
> +
> +enum extent_type {
> +	SIMPLE_DAX_EXTENT = 13,
> +	INVALID_EXTENT_TYPE,
> +};
> +
> +struct famfs_extent {
> +	__u64              offset;
> +	__u64              len;
> +};
> +
> +enum famfs_file_type {
> +	FAMFS_REG,
> +	FAMFS_SUPERBLOCK,
> +	FAMFS_LOG,
> +};
> +
> +/**

"/**" is used to begin kernel-doc comments, but this comment block is missing
a few entries to make it be kernel-doc compatible. Please either add them
or just use "/*" to begin the comment.

> + * struct famfs_ioc_map
> + *
> + * This is the metadata that indicates where the memory is for a famfs file
> + */
> +struct famfs_ioc_map {
> +	enum extent_type          extent_type;
> +	enum famfs_file_type      file_type;
> +	__u64                     file_size;
> +	__u64                     ext_list_count;
> +	struct famfs_extent       ext_list[FAMFS_MAX_EXTENTS];
> +};
> +
> +#define FAMFSIOC_MAGIC 'u'

This 'u' value should be documented in
Documentation/userspace-api/ioctl/ioctl-number.rst.

and if possible, you might want to use values like 0x5x or 0x8x
that don't conflict with the ioctl numbers that are already used
in the 'u' space.

> +
> +/* famfs file ioctl opcodes */
> +#define FAMFSIOC_MAP_CREATE    _IOW(FAMFSIOC_MAGIC, 1, struct famfs_ioc_map)
> +#define FAMFSIOC_MAP_GET       _IOR(FAMFSIOC_MAGIC, 2, struct famfs_ioc_map)
> +#define FAMFSIOC_MAP_GETEXT    _IOR(FAMFSIOC_MAGIC, 3, struct famfs_extent)
> +#define FAMFSIOC_NOP           _IO(FAMFSIOC_MAGIC,  4)
> +
> +#endif /* FAMFS_IOCTL_H */

-- 
#Randy