Re: [PATCH] habanalabs: use correct variable to show fd open counter

Greg KH
2019-07-08T11:21:41Z

On Mon, Jul 08, 2019 at 01:43:55PM +0300, Oded Gabbay wrote:
> The current code checks if the user context pointer is NULL or not to
> display the number of open file descriptors of a device. However, that
> variable (user_ctx) will eventually go away as the driver will support
> multiple processes. Instead, the driver can use the atomic counter of
> the open file descriptors which the driver already maintains.
>
> Signed-off-by: Oded Gabbay <oded.gabbay@gmail.com>
> ---
> drivers/misc/habanalabs/sysfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/habanalabs/sysfs.c b/drivers/misc/habanalabs/sysfs.c
> index 25eb46d29d88..881be19b5fad 100644
> --- a/drivers/misc/habanalabs/sysfs.c
> +++ b/drivers/misc/habanalabs/sysfs.c
> @@ -356,7 +356,7 @@ static ssize_t write_open_cnt_show(struct device *dev,
> {
> struct hl_device *hdev = dev_get_drvdata(dev);
>
> - return sprintf(buf, "%d\n", hdev->user_ctx ? 1 : 0);
> + return sprintf(buf, "%d\n", atomic_read(&hdev->fd_open_cnt));
> }

Odds are, this means nothing, as it doesn't get touched if the file
descriptor is duped or sent to another process.

Why do you care about the number of open files? Whenever someone tries
to do this type of logic, it is almost always wrong, just let userspace
do what it wants to do, and if wants to open something twice, then it
gets to keep the pieces when things break.

thanks,

greg k-h