Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1243752

Re: [PATCH] sched_clock: add data pointer argument to read callback

From Måns Rullgård <mans@mansr.com>
Newsgroups linux.kernel
Subject Re: [PATCH] sched_clock: add data pointer argument to read callback
Date 2015-10-10 02:50 +0200
Message-ID <qhQwh-1ED-3@gated-at.bofh.it> (permalink)
References <qhPgS-8oj-7@gated-at.bofh.it> <qhPgS-8oj-5@gated-at.bofh.it> <qhPAd-jk-3@gated-at.bofh.it> <qhPJU-v0-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Russell King - ARM Linux <linux@arm.linux.org.uk> writes:

> On Sat, Oct 10, 2015 at 12:48:22AM +0100, Måns Rullgård wrote:
>> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
>> 
>> > On Fri, Oct 09, 2015 at 10:57:35PM +0100, Mans Rullgard wrote:
>> >> This passes a data pointer specified in the sched_clock_register()
>> >> call to the read callback allowing simpler implementations thereof.
>> >> 
>> >> In this patch, existing uses of this interface are simply updated
>> >> with a null pointer.
>> >
>> > This is a bad description.  It tells us what the patch is doing,
>> > (which we can see by reading the patch) but not _why_.  Please include
>> > information on why the change is necessary - describe what you are
>> > trying to achieve.
>> 
>> Currently most of the callbacks use a global variable to store the
>> address of a counter register.  This has several downsides:
>> 
>> - Loading the address of a global variable can be more expensive than
>>   keeping a pointer next to the function pointer.
>> 
>> - It makes it impossible to have multiple instances of a driver call
>>   sched_clock_register() since the caller can't know which clock will
>>   win in the end.
>> 
>> - Many of the existing callbacks are practically identical and could be
>>   replaced with a common generic function if it had a pointer argument.
>> 
>> If I've missed something that makes this a stupid idea, please tell.
>
> So my next question is whether you intend to pass an iomem pointer
> through this, or a some kind of structure, or both.  It matters,
> because iomem pointers have a __iomem attribute to keep sparse
> happy.  Having to force that attribute on and off pointers is frowned
> upon, as it defeats the purpose of the sparse static checker.

So this is an instance where tools like sparse get in the way of doing
the simplest, most efficient, and obviously correct thing.  Who wins in
such cases?

-- 
Måns Rullgård
mans@mansr.com
--
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/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH] sched_clock: add data pointer argument to read callback Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-10 01:30 +0200
  Re: [PATCH] sched_clock: add data pointer argument to read callback Måns Rullgård <mans@mansr.com> - 2015-10-10 01:50 +0200
    Re: [PATCH] sched_clock: add data pointer argument to read callback Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-10 02:00 +0200
      Re: [PATCH] sched_clock: add data pointer argument to read callback Måns Rullgård <mans@mansr.com> - 2015-10-10 02:50 +0200
        Re: [PATCH] sched_clock: add data pointer argument to read callback Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-10 17:40 +0200

csiph-web