Re: [PATCH V3 2/5] misc: mlx5ctl: Add mlx5ctl misc driver

[Greg Kroah-Hartman]

On Mon, Nov 20, 2023 at 11:06:16PM -0800, Saeed Mahameed wrote:
> +struct mlx5ctl_dev {
> +	struct mlx5_core_dev *mdev;
> +	struct miscdevice miscdev;
> +	struct auxiliary_device *adev;
> +	struct list_head fd_list;
> +	spinlock_t fd_list_lock; /* protect list add/del */
> +	struct rw_semaphore rw_lock;
> +	struct kref refcount;

You now have 2 different things that control the lifespan of this
structure.  We really need some way to automatically check this so that
people don't keep making this same mistake, it happens all the time :(

Please pick one structure (miscdevice) or the other (kref) to control
the lifespan, having 2 will just not work.

Also, why a rw_semaphore?  Only use those if you can prove with a
benchmark that it is actually faster, otherwise it's simpler to just use
a normal mutex (hint, you are changing the fields in the structure with
the read lock held, which feels very wrong, and so needs a LOT of
documentation, or just use a normal mutex...)

thanks,

greg k-h