tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: RFC: New userspace fetch/store API




> On Feb 24, 2019, at 1:42 PM, Mindaugas Rasiukevicius <rmind%netbsd.org@localhost> wrote:
> 
>> Outstanding question before I go too far down the rabbit hole: should I
>> bother with the “intrsafe” variants?  The only application for it in the
>> tree right now is in the profiling code, as an optimization to avoid
>> taking an AST when it’s time to bump the counters.
> 
> Please do not over-engineer it.  IIRC, there are not that many use cases
> of this API, the need for more variants didn't really increase over the
> decade(s) and there is no need to provide a vast spectrum of functions.
> They can be added later, if there will be a need.

Again, I’m happy to eliminate the _intrsafe variants.  The only use is in the profiling code, and it’s just to avoid taking an AST.

> Also, are there any uses of byte and short operations?  If not, then I
> think these variants should be eliminated.  Nowadays, I would argue that
> generally there should be no good reason to choose sub-word operations;
> stick with the word size (it helps with the alignment too) or more.

There are lots of uses in MD code, and a few in MI code (ureadc() and mincore(), for example).  I figured having the same API when it’s just MD code would be worthwhile.

>> These functions may block (to service a page fault), and are NOT safe to
>> call from any interrupt context.
>> 
> 
> Please make sure you add the asserts.

I’ll make C wrappers for all of these, and have the MD guts prefixed with an underscore so I can centralize the assertion logic.

>> The implementation is entirely in architecture-dependent code.  The
>> following fetch primitives must be supplied:
>> 
>> int     ufetch_8(const uint8_t *uaddr, uint8_t *valp);
>> int     ufetch_16(const uint16_t *uaddr, uint16_t *valp);
>> int     ufetch_32(const uint32_t *uaddr, uint32_t *valp);
>> #ifdef _LP64
>> int     ufetch_64(const uint64_t *uaddr, uint64_t *valp);
>> #endif
> 
> Please prefix the size with 'u' if you operate on unsigned types:
> 
> ufetch_u{8,16,32,64} and the same for ustore_*

Even at the expense of being different from atomic_ops(3)?

-- thorpej



Home | Main Index | Thread Index | Old Index