Re: ldconfig hwcap rewrite

[Andreas Jaeger <aj at suse dot de>]

>>>>> Ulrich Drepper writes:

Uli> It looks OK but there are a few problems.
>> +#define HWCAP_CHECK								\
>> +      if (*hwcap && (cache_new->libs[middle].hwcap & *hwcap) > _dl_hwcap)	\
>> +	continue
>> +      SEARCH_CACHE (cache_new);
>> }
>> +  else

Uli> The first test certainly must be "if (hwcap", i.e., no dereferencing.

Thanks, I've fixed this.

Uli> Second, related to this, if you are using binary sort for the extended
Uli> table, how do you make sure you see all the entries with a given name
Uli> but different hwcaps?  It seems you are stumbling accross one and then
Uli> us the existence of the hwcap field to single step to the next entry.
Uli> But what if the first entry with a matching name is not the one with
Uli> the most specific hwcap?
I've sorted them - but this sorting is not optimal.

Uli> What I think is necessary is that either

Uli> - there is exactly one of the hwcap entries actually in the table and
Uli>   by some the different hwcap entries can be by some other mean

Uli> - add an explicit index to the first hwcap entry with the matching hwcap.
Uli>   The algorithm would look like this:

Uli>      find idx with matching file name

Uli>      if hwcap in entry[idx] is zero
Uli>        found entry idx
Uli>      else
Uli>        idx = entry[idx].first_hwcap

Uli>        while ((entry[idx].hwcap & _dl_hwcap) > _dl_hwcap)
Uli>         // Probably need to handle case here where no lib with hwcap
Uli>         // zero is available 
Uli>         ++idx;

Uli>        found entry idx

Uli> If you have already done something like this I missed it in my brief
Uli> look over the code.  It should be carefully documented in any case.

I'll tomorrow again over the code - and either fix or document it.

What's the best way to check for hwcaps?  The current way which isn't
either clever nor very good is to use:
if ((entry[idx].hwcap & _dl_hwcap) > _dl_hwcap)
for stepping over libraries and using the first one that fits.  But
there might be a number of libraries which fit.  Is there a way to
order them?  Currently thw hwcaps are ordered to their numerical value.
I'll also try to use HWCAP_IMPORTANT.

Currently I sort two entries e1, e2 in ldconfig with:
  res = _dl_cache_libcmp (e2->lib, e1->lib);
  if (res == 0)
    {
      if (e1->flags < e2->flags)
	return 1;
      else if (e1->flags > e2->flags)
	return -1;
      else if (e2->hwcap > e1->hwcap)
	return 1;
      else if (e2->hwcap < e1->hwcap)
	return -1;
    }
  return res;

Better seems to be (using HWCAP_IMPORTANT):
  res = _dl_cache_libcmp (e2->lib, e1->lib);
  if (res == 0)
    {
      if (e1->flags < e2->flags)
	return 1;
      else if (e1->flags > e2->flags)
	return -1;
      /* First use the important bits.  */
      else if ((e2->hwcap & HWCAP_IMPORTANT) > (e1->hwcap & HWCAP_IMPORTANT))
        return 1;
      else if ((e2->hwcap & HWCAP_IMPORTANT) < (e1->hwcap & HWCAP_IMPORTANT))
        return -1;
      else if (e2->hwcap > e1->hwcap)
	return 1;
      else if (e2->hwcap < e1->hwcap)
	return -1;
    }
  return res;

Even better seems to be to count the number of set bits in hwcap (like
it's done in sysdeps/generic/dl-sysdeps.c) - and order by this one
after looking at HWCAP_IMPORTANT.  This way the most specific ones are
first.

Should I look at hwcaps beside HWCAP_IMPORTANT at all (I do
currently)?  Does it make sense to handle others?  I don't think and
will change the functions accordingly.

What do you think?  I'll call it a day now and will look into these
issues tomorrow again when time permits.

Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de