Re: [PATCH] drivers/cdrom: improved ioctl for media change detection

From: Lukas Prediger
Date: Sat Aug 28 2021 - 21:25:40 EST


Thanks for the reply and sorry for the spam :/. I am not sure what
happened there.

>> 2. As the last_media_change field will be in ms now, is it safe to
>> convert those back to jiffies for comparison or is there a risk of
>> information loss (due to rounding or whatever) in either conversion?
>> More technically, can I make the assumption that for any jiffies value
>> x it holds that
> The granularity of jiffies depends on the HZ setting, generally just
> consider it somewhere in between 100 and 1000. That's where my initial
> granularity numbers came from.
>
>> time_before(msecs_to_jiffies(jiffies_to_msecs(x)), x) is always false ?
> I don't think that matters. Internally, always keep things in jiffies.
> That's what you use to compare with, and to check if it's changed since
> last time. The only time you convert to ms is to pass it back to
> userspace. And that's going to be a delta of jiffies always, just ensure
> you assign last_checked = jiffies when it's setup initially.
>
The issue I have is that the value I am comparing to is provided by the code
calling the ioctl so that I don't have to maintain state for every potential
calling process in the kernel. Therefore, if we want the API to work with ms,
I have to convert the user provided value back to jiffies for comparison.

I now ran a brief test that suggests that the above condition does not hold
and therefore the value returned in has_changed may be 1 for subsequent calls
when the disc was not in fact changed.

Workarounds I see would be to either expose the jiffies value through the
API (which is maybe not really clean), or making the comparison on the ms value
(but how to deal with potential wraparounds then?). Of those, I would currently
tend to the first and treat the nature of the returned timestamp as an opaque
value from the user perspective - it is probably not really of any use to them
outside of passing it back into the ioctl for subsequent calls. Do you
see other ways to resolve this I may not have thought of?

Kind regards,
Lukas