Date: Tue, 29 Dec 2015 21:00:42 +0100
From: Wim Van Sebroeck <wim@iguana.be>
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-watchdog@vger.kernel.org, Pratyush Anand <panand@redhat.com>,
        Hans de Goede <hdegoede@redhat.com>,
        Damien Riegel <damien.riegel@savoirfairelinux.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/6] watchdog: Replace driver based refcounting
Message-ID: <20151229200042.GA12170@spo001.leaseweb.nl>
References: <1451088105-1722-1-git-send-email-linux@roeck-us.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1451088105-1722-1-git-send-email-linux@roeck-us.net>
User-Agent: Mutt/1.4.1i
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3516
Lines: 73

Hi Guenter,

> All variables required by the watchdog core to manage a watchdog are
> currently stored in struct watchdog_device. The lifetime of those
> variables is determined by the watchdog driver. However, the lifetime
> of variables used by the watchdog core differs from the lifetime of
> struct watchdog_device. To remedy this situation, watchdog drivers
> can implement ref and unref callbacks, to be used by the watchdog
> core to lock struct watchdog_device in memory. This mechanism was
> introduced with commit e907df327252 ("watchdog: Add support for
> dynamically allocated watchdog_device structs").
> 
> While this solves the immediate problem, it depends on watchdog drivers
> to actually implement the ref/unref callbacks. This is error prone,
> often not implemented in the first place, or not implemented correctly.
> 
> To solve the problem without requiring driver support, split the variables
> in struct watchdog_device into two data structures - one for variables
> associated with the watchdog driver, one for variables associated with
> the watchdog core. With this approach, the watchdog core can keep track
> of the variables it uses and no longer depends on ref/unref callbacks
> in the driver. As a side effect, some of the variables originally in struct
> watchdog_driver are now private to the watchdog core and no longer visible
> in watchdog drivers.
> 
> The 'ref' and 'unref' callbacks in struct watchdog_driver are no longer
> used and marked as deprecated for the time being.
> 
> Patch 1/6 moves watchdog device creation from watchdog_core.c to
> watchdog_dev.c to simplify watchdog device handling.
> 
> Patch 2/6 modifies the diag288 watchdog to no longer use and overload
> watchdog core internal flags.
> 
> Patch 3/6 separates variables in watchdog_device based on variable
> lifetime.
> 
> Patch 4/6 to 6/6 remove existing ref/unref functions from the drivers
> implementing it. Two of those patches (for da9052 and da9055) fix a
> previously introduced bug as side effect.
> 
> The series applies on top of the current watchdog-next as well as the
> pending patches introducing sysfs support ("watchdog: Use static struct
> class watchdog_class in stead of pointer" and "watchdog: Read device
> status through sysfs attributes") by Pratyush Anand.
> 
> ---
> v2: Add patch 2/6.
>     Add more detailed description to patch 3/6.
>     Rename internal data structure to watchdog_core_data, and the variable
>     pointing to it to wd_data. Move its declaration into watchdog_dev.c.
>     Reorder include files in watchdog_dev.c to alphabetic order.
>     Remove _watchdog_ping helper function and call watchdog_ping()
>     directly. Handle locks in the calling code.
>     Drop locking from watchdog_open(). It is already protected against
>     parallel device removal in the driver core code, and file access
>     functions won't be called before the function returns.
>     Remove previously added message "watchdog still running". It wasn't
>     there before, and we should avoid user visible changes.
>     Add missing kfree() to the sch56xx patch.
>     Add comments to da9052 and da9055 patches indicating that the
>     patches fix a bug.

This patchset has been added to linux-watchdog-next.

Kind regards,
Wim.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/