[PATCH i-g-t 2/3] lib/xe: replace div64_u64_round_up() with DIV_ROUND_UP

[Kamil Konieczny]

Hi Lin,,
On 2025-04-25 at 03:35:08 +0000, Lin, Shuicheng wrote:
> On Thu, April 24, 2025 10:40 AM Kamil Konieczny wrote:
> > Hi Shuicheng,
> > On 2025-04-16 at 16:30:52 +0000, Shuicheng Lin wrote:
> > > DIV_ROUND_UP is defined in igt_aux.h with the same function as
> > > div64_u64_round_up().
> > 
> > Version in igt_aux.h is different then one below.
> > 
> > >
> > > Cc: Matt Roper <matthew.d.roper at intel.com>
> > > Signed-off-by: Shuicheng Lin <shuicheng.lin at intel.com>
> > > ---
> > >  lib/xe/xe_util.c | 10 +---------
> > >  1 file changed, 1 insertion(+), 9 deletions(-)
> > >
> > > diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c index
> > > 06b378ce0..9907c8b16 100644
> > > --- a/lib/xe/xe_util.c
> > > +++ b/lib/xe/xe_util.c
> > > @@ -253,14 +253,6 @@ static uint32_t reference_clock(int fd, int gt_id)
> > >  	return refclock;
> > >  }
> > >
> > > -static uint64_t div64_u64_round_up(const uint64_t x, const uint64_t
> > > y) -{
> > > -	igt_assert(y > 0);
> > > -	igt_assert_lte_u64(x, UINT64_MAX - (y - 1));
> > 
> > I like those two asserts here, why not moving this function to lib igt_aux.c and
> > creating new macro for it?
> > 
> > #define DIV64_ROUND_UP(n, d) div64_u64_round_up((uint64_t)(n),
> > (uint64_t)(d))
> > 
> > or make it a multi-line macro with do { ... } while(0)
> > 
> > Regards,
> > Kamil
> 
> Since there is DIV_ROUND_UP already, I don't want to add DIV64_ROUND_UP.
> DIV_ROUND_UP doesn't require the data type. Although there is no harm
> to do the data conversion, I just think it is not necessary here.
> I agree the assert is more secure.
> Since my change is just a cleanup, no real functional changes.

If you want real cleanup without functional changes move this function
to igt_aux.c and use it instead of using a macro.

Btw all macros are error prone, why not just a function with a little
more safety like:

	r = x % y;
	x /= y;
	if (r)
		++x;

so second assert becomes unnecessery except a new check:

	igt_assert(y > 1);

> Let me drop this patch. Thanks.
> 
> BTW, I just found there is _DIV_ROUND_UP definition in the code also. Maybe we
> could do more cleanup later.

Another almost identical macro will not solve a problem.

Regards,
Kamil

> 
> Shuicheng
> 
> > 
> > > -
> > > -	return (x + y - 1) / y;
> > > -}
> > > -
> > >  /**
> > >   * xe_nsec_to_ticks: convert time in nanoseconds to timestamp ticks
> > >   * @fd: opened device
> > > @@ -273,5 +265,5 @@ uint32_t xe_nsec_to_ticks(int fd, int gt_id,
> > > uint64_t nsec)  {
> > >  	uint32_t refclock = reference_clock(fd, gt_id);
> > >
> > > -	return div64_u64_round_up(nsec * refclock, NSEC_PER_SEC);
> > > +	return DIV_ROUND_UP(nsec * refclock, NSEC_PER_SEC);
> > >  }
> > > --
> > > 2.25.1
> > >