linux-kernel - Re: [PATCH] clockevents: Introduce mode specific callbacks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKohpomhTO=ZaOAm8Q4dyX0=1vYOktm_y=u-++dNtgGm8j172A@mail.gmail.com>
Date:	Wed, 11 Feb 2015 11:24:53 +0800
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Linaro Kernel Mailman List <linaro-kernel@...ts.linaro.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Kevin Hilman <khilman@...aro.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Preeti U Murthy <preeti@...ux.vnet.ibm.com>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Linaro Networking <linaro-networking@...aro.org>
Subject: Re: [PATCH] clockevents: Introduce mode specific callbacks

On 10 February 2015 at 22:15, Peter Zijlstra <peterz@...radead.org> wrote:
> On Wed, Feb 04, 2015 at 01:06:23PM +0530, Viresh Kumar wrote:
>> +     /*
>> +      * Mode transition callback(s): Only one of the two groups should be
>> +      * defined:
>> +      * - set_mode(), only for modes <= CLOCK_EVT_MODE_RESUME.
>> +      * - set_mode_{shutdown|periodic|oneshot|resume}().
>> +      */
>>       void                    (*set_mode)(enum clock_event_mode mode,
>>                                           struct clock_event_device *);
>> +     int                     (*set_mode_periodic)(struct clock_event_device *);
>> +     int                     (*set_mode_oneshot)(struct clock_event_device *);
>> +     int                     (*set_mode_shutdown)(struct clock_event_device *);
>> +     int                     (*set_mode_resume)(struct clock_event_device *);
>
>
>
>
>> +/* Sanity check of mode transition callbacks */
>> +static int clockevents_sanity_check(struct clock_event_device *dev)
>> +{
>> +     /* Legacy set_mode() callback */
>> +     if (dev->set_mode)
>> +             return 0;
>> +
>> +     if (dev->features & CLOCK_EVT_FEAT_DUMMY)
>> +             return 0;
>> +
>> +     /* New mode-specific callbacks */
>> +     if (!dev->set_mode_shutdown)
>> +             return -EINVAL;
>> +
>> +     if ((dev->features & CLOCK_EVT_FEAT_PERIODIC) &&
>> +         !dev->set_mode_periodic)
>> +             return -EINVAL;
>> +
>> +     if ((dev->features & CLOCK_EVT_FEAT_ONESHOT) &&
>> +         !dev->set_mode_oneshot)
>> +             return -EINVAL;
>> +
>> +     return 0;
>> +}
>
> It appears to me you've not actually checked that condition outlined
> above, a driver could set both the legacy and the new callbacks.

Exactly for this reason I mentioned this in the logs:

>> If the legacy ->set_mode() callback is provided, all mode specific
>> callbacks would be ignored.

So, either we can mention that in the code as well OR add code to
check and WARN about that. Will do whatever looks better to you
guys.

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