LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Lukas Prediger <lukas.prediger@lumip.de>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drivers/cdrom: improved ioctl for media change detection
Date: Sat, 28 Aug 2021 13:27:41 +0300	[thread overview]
Message-ID: <6bbfc86d-8e3b-db5e-0bf5-8bce63d2049f@lumip.de> (raw)
In-Reply-To: <5f3b7d8b-9e97-094b-efd1-cad6cab793b6@kernel.dk>

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


  reply	other threads:[~2021-08-29  1:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-05 19:44 Lukas Prediger
2021-08-26 18:01 ` Lukas Prediger
2021-08-26 22:38 ` Jens Axboe
2021-08-27 17:30   ` Lukas Prediger
2021-08-28  3:18     ` Jens Axboe
2021-08-28 10:27       ` Lukas Prediger [this message]
2021-08-28 13:22         ` Jens Axboe
2021-09-06 16:53           ` Lukas Prediger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6bbfc86d-8e3b-db5e-0bf5-8bce63d2049f@lumip.de \
    --to=lukas.prediger@lumip.de \
    --cc=axboe@kernel.dk \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH] drivers/cdrom: improved ioctl for media change detection' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).