Re: [PATCH 3/7] Functions to fetch POSIX dynamic clock object

[Thomas Gleixner]

On Thu, Oct 01 2020 at 22:51, Erez Geva wrote:
> Add kernel functions to fetch a pointer to a POSIX dynamic clock
> using a user file description dynamic clock ID.

And how is that supposed to work. What are the lifetime rules?
  
> +struct posix_clock *posix_clock_get_clock(clockid_t id)
> +{
> +     int err;
> +     struct posix_clock_desc cd;

The core code uses reverse fir tree ordering of variable declaration
based on the length:

        struct posix_clock_desc cd;
        int err;

> +     /* Verify we use posix clock ID */
> +     if (!is_clockid_fd_clock(id))
> +             return ERR_PTR(-EINVAL);
> +
> +     err = get_clock_desc(id, &cd);

So this is a kernel interface and get_clock_desc() does:

        struct file *fp = fget(clockid_to_fd(id));

How is that file descriptor valid in random kernel context?

> +     if (err)
> +             return ERR_PTR(err);
> +
> +     get_device(cd.clk->dev);

The purpose of this is? Comments are overrated...

> +     put_clock_desc(&cd);
> +
> +     return cd.clk;
> +}
> +EXPORT_SYMBOL_GPL(posix_clock_get_clock);
> +
> +int posix_clock_put_clock(struct posix_clock *clk)
> +{
> +     if (IS_ERR_OR_NULL(clk))
> +             return -EINVAL;
> +     put_device(clk->dev);
> +     return 0;
> +}
> +EXPORT_SYMBOL_GPL(posix_clock_put_clock);
> +
> +int posix_clock_gettime(struct posix_clock *clk, struct timespec64 *ts)
> +{
> +     int err;
> +
> +     if (IS_ERR_OR_NULL(clk))
> +             return -EINVAL;
> +
> +     down_read(&clk->rwsem);

Open coding the logic of get_posix_clock() and having a copy here and
in the next function is really useful.

Thanks,

        tglx