netdev - Re: [PATCH v4 01/10] fwctl: Add basic structure for a class subsystem with a cdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <67a697f02bc0b_2d1e2948a@dwillia2-xfh.jf.intel.com.notmuch>
Date: Fri, 7 Feb 2025 15:32:00 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: Andy Gospodarek <andrew.gospodarek@...adcom.com>, Aron Silverton
	<aron.silverton@...cle.com>, Dan Williams <dan.j.williams@...el.com>, "Daniel
 Vetter" <daniel.vetter@...ll.ch>, Dave Jiang <dave.jiang@...el.com>, "David
 Ahern" <dsahern@...nel.org>, Andy Gospodarek <gospo@...adcom.com>, "Christoph
 Hellwig" <hch@...radead.org>, Itay Avraham <itayavr@...dia.com>, Jiri Pirko
	<jiri@...dia.com>, Jonathan Cameron <Jonathan.Cameron@...wei.com>, "Jakub
 Kicinski" <kuba@...nel.org>, Leonid Bloch <lbloch@...dia.com>, Leon
 Romanovsky <leonro@...dia.com>, <linux-cxl@...r.kernel.org>,
	<linux-rdma@...r.kernel.org>, <netdev@...r.kernel.org>, Saeed Mahameed
	<saeedm@...dia.com>, "Nelson, Shannon" <shannon.nelson@....com>
Subject: Re: [PATCH v4 01/10] fwctl: Add basic structure for a class
 subsystem with a cdev

Jason Gunthorpe wrote:
> Create the class, character device and functions for a fwctl driver to
> un/register to the subsystem.
> 
> A typical fwctl driver has a sysfs presence like:
> 
> $ ls -l /dev/fwctl/fwctl0
> crw------- 1 root root 250, 0 Apr 25 19:16 /dev/fwctl/fwctl0
> 
> $ ls /sys/class/fwctl/fwctl0
> dev  device  power  subsystem  uevent
> 
> $ ls /sys/class/fwctl/fwctl0/device/infiniband/
> ibp0s10f0
> 
> $ ls /sys/class/infiniband/ibp0s10f0/device/fwctl/
> fwctl0/
> 
> $ ls /sys/devices/pci0000:00/0000:00:0a.0/fwctl/fwctl0
> dev  device  power  subsystem  uevent
> 
> Which allows userspace to link all the multi-subsystem driver components
> together and learn the subsystem specific names for the device's
> components.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> Signed-off-by: Jason Gunthorpe <jgg@...dia.com>
[..]
> +struct fwctl_device *_fwctl_alloc_device(struct device *parent,
> +					 const struct fwctl_ops *ops,
> +					 size_t size);
> +/**
> + * fwctl_alloc_device - Allocate a fwctl
> + * @parent: Physical device that provides the FW interface
> + * @ops: Driver ops to register
> + * @drv_struct: 'struct driver_fwctl' that holds the struct fwctl_device
> + * @member: Name of the struct fwctl_device in @drv_struct
> + *
> + * This allocates and initializes the fwctl_device embedded in the drv_struct.
> + * Upon success the pointer must be freed via fwctl_put(). Returns a 'drv_struct
> + * \*' on success, NULL on error.
> + */
> +#define fwctl_alloc_device(parent, ops, drv_struct, member)               \
> +	({                                                                \
> +		static_assert(__same_type(struct fwctl_device,            \
> +					  ((drv_struct *)NULL)->member)); \
> +		static_assert(offsetof(drv_struct, member) == 0);         \
> +		(drv_struct *)_fwctl_alloc_device(parent, ops,            \
> +						  sizeof(drv_struct));    \
> +	})

I have already suggested someone else copy this approach to context
allocation. What do you think of generalizing this in
include/linux/container_of.h as:

#define container_alloc(core_struct, drv_struct, member, alloc_fn, ...)    \
       ({                                                                 \
               static_assert(__same_type(core_struct,                     \
                                         ((drv_struct *)NULL)->member));  \
               static_assert(offsetof(drv_struct, member) == 0);          \
               (drv_struct *)(alloc_fn)(sizeof(drv_struct), __VA_ARGS__); \
       })

...and then fwctl_alloc_device becomes:

#define fwctl_alloc_device(parent, ops, drv_struct, member) \
	container_alloc(struct fwctl_device, drv_struct, member, \
			_fwctl_alloc_device, parent, ops);

Either way, you can add:

Reviewed-by: Dan Williams <dan.j.williams@...el.com>