Re: [watchdog-next] watchdog: kill unref/ref ops

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/04/2016 03:38 PM, Winkler, Tomas wrote:


-----Original Message-----
From: Guenter Roeck [mailto:linux@xxxxxxxxxxxx]
Sent: Sunday, January 03, 2016 17:48
To: Winkler, Tomas; Wim Van Sebroeck
Cc: linux-watchdog@xxxxxxxxxxxxxxx
Subject: Re: [watchdog-next] watchdog: kill unref/ref ops

On 01/03/2016 03:32 AM, Tomas Winkler wrote:
ref/unref ops are not called at all so even marked them as deprecated
is misleading, we need to just drop the API.

One the other side, the functions are now no-ops, so they don't hurt either.
Only reason for not dropping the functions as part of my series was that
the mei watchdog code was starting to implement them, and I didn't want
to create dependencies for that code. Maybe my thinking was too conservative.

Signed-off-by: Tomas Winkler <tomas.winkler@xxxxxxxxx>

... of course that is no concern anymore since this patch is from its
maintainer.

Acked-by: Guenter Roeck <linux@xxxxxxxxxxxx>

Now I'm looking into the current code and one thing I do not understand , if the watchdog_core_data is holding
a pointer to wdd, who is responsible for wdd and who is holding the reference counter on that pointer.
If the underlying driver will release the wdd (for any reason) your code will read junk.
I think wdd should not be a pointer here.

The pointer is cleared when the watchdog device is removed (in watchdog_cdev_unregister,
which is called from watchdog_dev_unregister), and the cleared pointer is used by
the watchdog core as indication that the driver is no longer accessible.
This replaces the WDOG_UNREGISTERED flag.

Guenter

/*
  * struct watchdog_core_data - watchdog core internal data
  * @kref:       Reference count.
  * @cdev:       The watchdog's Character device.
  * @wdd:        Pointer to watchdog device.
  * @lock:       Lock for watchdog core.
  * @status:     Watchdog core internal status bits.
  */
struct watchdog_core_data {
         struct kref kref;
         struct cdev cdev;
         struct watchdog_device *wdd;
         struct mutex lock;
         unsigned long status;           /* Internal status bits */
#define _WDOG_DEV_OPEN          0       /* Opened ? */
#define _WDOG_ALLOW_RELEASE     1       /* Did we receive the magic char ? */
};

Thanks
Tomas


--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux