[lkml]   [2010]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH V7 4/8] posix clocks: hook dynamic clocks into system calls
    On Fri, 17 Dec 2010, Richard Cochran wrote:
    > On Fri, Dec 17, 2010 at 12:20:44AM +0100, Thomas Gleixner wrote:
    > > > +static inline bool clock_is_static(const clockid_t id)
    > > > +{
    > > > + if (0 == (id & ~CLOCKFD_MASK))
    > > > + return true;
    > > > + if (CLOCKFD == (id & CLOCKFD_MASK))
    > > > + return false;
    > >
    > > Please use the usual kernel notation.
    > Sorry, what do you mean?

    if (!id & ~CLOCKFD_MASK)
    if ((id & CLOCKFD_MASK) == CLOCKFD)

    Not a real problem. I just always stumble over that coding style.

    > The code in the patch set is modeled after a USB driver, namely
    > drivers/usb/class/usbtmc.c. Alan Cox had brought up the example of a
    > PTP Hardware Clock appearing as a USB device. So the device might
    > suddenly disappear, and the zombie field is supposed to cover the case
    > where the hardware no longer exists, but the file pointer is still
    > valid.

    Hmm ok, so you use clk->mutex to prevent the underlying device from
    going away while you call a function and clk->zombie indicates that
    it's gone and clk just kept around for an open file descriptor. Now it
    makes sense, but that wants a big fat comment in the code. :)

    Doesn't the same problem exist for the file operations of patch 3? I
    think you want the very same protection there unless you want to do
    the same magic in your USB driver as well.

    So you could do the following:

    static int get_fd_clk(struct posix_clock_descr *cd, struct file *fp)
    cd->fp = fp;
    cd->clk = fp->private_data;
    if (!cd->clk->zombie)
    return 0;
    return -ENODEV;

    static void put_fd_clk(struct posix_clock_descr *cd)

    static int get_posix_clock(const clockid_t id, struct posix_clock_descr *cd)
    struct file *fp = fget(CLOCKID_TO_FD(id));
    int ret;

    if (!fp || fp->f_op->open != posix_clock_open || !fp->private_data)
    return -ENODEV;
    ret = get_fd_clk(cd, fp);
    if (ret)
    return ret;

    static void put_posix_clock(struct posix_clock_descr *cd)

    So your fops would call get_fd_clk() and put_fd_clk() and the pc_timer
    ones get_posix_clock() and put_posix_clock() or whatever sensible
    function names you come up with.

    Thoughts ?

    > I would summarize the discussion like this:
    > Alan Cox was strongly in favor of using a regular file descriptor as
    > the reference to the dynamic clock.
    > John Stultz thought it wouldn't be too bad to cycle though a number of
    > static ids, being careful not to every assign the same static id (in
    > series) to two different clocks.
    > I implemented Alan's idea, since it seemed like maintaining the
    > mapping between clocks and static ids would be extra work, but without
    > any real benefit.

    Yes, he's right. Was too tired to think about the clock ids assingment
    when devices are removed and plugged. The fd mapping makes that all go



     \ /
      Last update: 2010-12-17 11:07    [W:0.053 / U:78.120 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site