On Thu, Mar 1, 2018 at 9:13 PM, Rodrigo Rivas Costa
[off-list ref] wrote:

On Thu, Mar 01, 2018 at 12:20:37PM +0200, Andy Shevchenko wrote:

quoted

On Thu, Mar 1, 2018 at 12:49 AM, Rodrigo Rivas Costa
[off-list ref] wrote:

quoted

On Wed, Feb 28, 2018 at 09:21:15PM +0200, Andy Shevchenko wrote:

quoted

On Wed, Feb 28, 2018 at 8:43 PM, Rodrigo Rivas Costa
[off-list ref] wrote:

My bad, I didn't mean a literal free() call. More like:

static void steam_unregister(struct steam_device *steam)
{
        struct input_dev *input;
        struct power_supply *battery;

        rcu_read_lock();
        input = rcu_dereference(steam->input);
        battery = rcu_dereference(steam->battery);
        input_gyro = rcu_dereference(steam->input_gyro);
        rcu_read_unlock();

        if (input) {
                RCU_INIT_POINTER(steam->input, NULL);
                synchronize_rcu();
                hid_info(steam->hdev, "Steam Controller '%s' disconnected",
                                steam->serial_no);
                input_unregister_device(input);

Candidate for a helper

static void steam_unregister_input(...)
{
...
}


steam_unregister_input(input);

        }

        if (battery) {
                RCU_INIT_POINTER(steam->battery, NULL);
                synchronize_rcu();
                power_supply_unregister(battery);
        }

Ditto.

        if (input_gyro) {
                RCU_INIT_POINTER(steam->input_gyro, NULL);
                synchronize_rcu();
                input_unregister_device(input_gyro);
        }

Ditto.

}

Also I think input_unregister_device() does not admit a NULL pointer.
Having a special `if (!input_gyro)return;` at the end looks just
wrong to me, it is no different from the other ones.

See above. Hide that detail in a helper function.

-- 
With Best Regards,
Andy Shevchenko