Alan Jenkins wrote:
> Kay Sievers wrote:
>
>> On Tue, Sep 2, 2008 at 13:09, Alan Jenkins <alan-jenkins@xxxxxxxxxxxxxx> wrote:
>>
>>
>>> udev_sysfs.c device and attribute value caches are only appropriate for
>>> short-lived processes, like udev-event and usbid. In long lived processes
>>> they would fill up with stale data.
>>>
>>>
>> Sure, the cache was never coded to be used in threaded programs.
>>
>>
>>
>>> Fortunately libudev doesn't use these caches.
>>>
>>>
>> For good reason, yeah. And it will not, at least not in its current state.
>>
>>
>>
>>> But this should be made
>>> explicit to guard against future changes. Let's move the caches into a
>>> separate file, with its own initialization and cleanup functions.
>>>
>>>
>> Wouldn't it be nicer to create a sysfs_cache object, which is passed
>> to all these functions? The "cheap" tools can just use NULL there, but
>> any possible user could have more than one instance of the cache. The
>> cache is really needed if you do thousand of rule compares against a
>> sysfs value, over and over. I guess we should start changing the stuff
>> to work with local vars, instead of moving the current global stuff to
>> a different file?
>>
>>
>
> Good point. That helps make the cache usage more explicit, and it's
> much nicer for my purposes as well. (I had blindly converted the global
> cache variables to per-thread variables, without touching the callers).
>
> Ok. To keep the existing behaviour where udev_rules_get_run() reuses
> the cache from udev_rules_get_name(), they both need a sysfs_cache
> parameter. Then I have to change test-udev and udevtest as well as
> udevd - no point having test programs if they use different code paths.
> I'll see what that looks like then and resend.
>
Oops. sysfs_cache can't be made optional ("just use NULL") without a
bit more effort. At the moment we rely on sysfs_cleanup() freeing
everything in the cache. If the cache isn't used, we need a new API to
free individual results.
So we also need sysfs_device_cleanup(), to free the device if it's not
on a cache list. And sysfs_attr_get_value() can copy the result to a
buffer instead of returning a string. I think that's reasonable.
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
