netdev - Re: [PATCH NET-NEXT 01/10] clocksource: allow usage independent of timekeeping.c

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: <1233761074.15940.230.camel@ecld0pohly>
Date:	Wed, 04 Feb 2009 16:24:34 +0100
From:	Patrick Ohly <patrick.ohly@...el.com>
To:	Daniel Walker <dwalker@...o99.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	John Stultz <johnstul@...ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH NET-NEXT 01/10] clocksource: allow usage independent of
	timekeeping.c

On Wed, 2009-02-04 at 15:09 +0000, Daniel Walker wrote:
> On Wed, 2009-02-04 at 15:46 +0100, Patrick Ohly wrote:
> > On Wed, 2009-02-04 at 14:03 +0000, Daniel Walker wrote:
> > > On Wed, 2009-02-04 at 14:01 +0100, Patrick Ohly wrote:
> > >
> > > >  /**
> > > > + * struct cyclecounter - hardware abstraction for a free running counter
> > > > + *       Provides completely state-free accessors to the underlying hardware.
> > > > + *       Depending on which hardware it reads, the cycle counter may wrap
> > > > + *       around quickly. Locking rules (if necessary) have to be defined
> > > > + *       by the implementor and user of specific instances of this API.
> > > > + *
> > > > + * @read:                returns the current cycle value
> > > > + * @mask:                bitmask for two's complement
> > > > + *                       subtraction of non 64 bit counters,
> > > > + *                       see CLOCKSOURCE_MASK() helper macro
> > > > + * @mult:                cycle to nanosecond multiplier
> > > > + * @shift:               cycle to nanosecond divisor (power of two)
> > > > + */
> > > > +struct cyclecounter {
> > > > + cycle_t (*read)(const struct cyclecounter *cc);
> > > > + cycle_t mask;
> > > > + u32 mult;
> > > > + u32 shift;
> > > > +};
> > >
> > > Where are these defined? I don't see any in created in your code.
> >
> > What do you mean with "these"? cycle_t? That type is defined in
> > clocksource.h. This patch intentionally adds these definitions to the
> > existing file because of the large conceptual overlap.
> 
> No, your creating a new structure here that wasn't declared. I was
> referring to "struct cyclecounter".

Sorry, I still don't see the problem. I'm declaring and defining struct
cyclecounter here. Why should it also be declared (= "struct
cyclecounter;") elsewhere?

>  I did look up one of your prior
> submission (Dec. 15) and reviewed that.

Right, I saw that a bit later.

> > In an earlier revision of the patch I had adapted clocksource itself so
> > that it could be used outside of the time keeping code; John wanted me
> > to use these smaller structs instead that you now find in the current
> > patch.
> 
> Well, I think your original idea was better.. I don't think we need the
> duplication of underlying clocksource mechanics.

Please discuss this with John. I'd prefer to not get caught in the
cross-fire of a discussion that I don't know enough about :-/

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

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