[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)

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.045 / U:19.604 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site