linux-kernel - Re: [PATCH 1/3] drm: Widen vblank UAPI to 64 bits. Change vblank time to ktime

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: <87wp6geh14.fsf@keithp.com>
Date:   Sun, 06 Aug 2017 13:35:03 -0400
From:   "Keith Packard" <keithp@...thp.com>
To:     Daniel Vetter <daniel@...ll.ch>
Cc:     linux-kernel@...r.kernel.org, Dave Airlie <airlied@...hat.com>,
        Daniel Vetter <daniel@...ll.ch>,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH 1/3] drm: Widen vblank UAPI to 64 bits. Change vblank time to ktime_t [v2]

Daniel Vetter <daniel@...ll.ch> writes:

> Subject is a bit confusing since you say uapi, but this is just the
> internal prep work. Dropping UAPI fixes that. With that fixed:

Yeah, thanks.

> Reviewed-by: Daniel Vetter <daniel.vetter@...ll.ch>

Added.

> Two more optional comments below, feel free to adapt or ignore. I'll wait
> for Michel's r-b before merging either way.
>
>>  static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
>> +				  u64 req_seq,
>>  				  union drm_wait_vblank *vblwait,
>
> Minor bikeshed: Since you pass the requested vblank number explicit, mabye
> also pass the user_data explicit and remove the vblwait struct from the
> parameter list? Restricts the old uapi cruft a bit.

I also need to re-write the reply.sequence value in the queue
function; seems like passing in the vblwait is a simpler plan.

>> +static u64 widen_32_to_64(u32 narrow, u64 near)
>> +{
>> +	u64 wide = narrow | (near & 0xffffffff00000000ULL);
>> +	if ((int64_t) (wide - near) > 0x80000000LL)
>> +		wide -= 0x100000000ULL;
>> +	else if ((int64_t) (near - wide) > 0x80000000LL)
>> +		wide += 0x100000000ULL;
>> +	return wide;
>
> return near + (int32_s) ((uint32_t)wide - near) ?

Oh, yes, that makes perfect sense -- an int32_t will obviously hold the
shortest distance between the two, whether negative or positive. Of
course, '(uint32_t) wide' is just 'narrow'.

> But then it took me way too long to think about this one, so maybe leave
> it at that.

Your version is a lot shorter, and I think it's actually clearer. How
about

static inline uint64_t widen_32_to_64(uint32_t narrow, uint64_t near)
{
	return near + (int32_t) (narrow - (uint32_t) near);
}

Here's a test program which validates the widen function.


View attachment "near.c" of type "text/x-csrc" (1989 bytes)


-- 
-keith


Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)