LKML Archive on
help / color / mirror / Atom feed
From: Lukas Prediger <>
To: Jens Axboe <>
Subject: Re: [PATCH] drivers/cdrom: improved ioctl for media change detection
Date: Fri, 27 Aug 2021 20:30:20 +0300	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 27.08.21 01:38, Jens Axboe wrote:

> Thanks for sending in the patch, appologies it's taken so long to get a
> response. CDROM isn't really actively maintained much these days,
> unfortunately. Should get a new maintainer.
> Anyway, for this patch, few comments inline.

Thanks for the feedback, I'm currently editing the patch to improve it based on your suggestions.
Some questions before resubmitting below.

> I'd just use jiffies for this, it's not really a case of something that
> needs a fine grained clock source. That'll give you 1-10ms resolution,
> which should be more than adequate for this. Then use jiffies_to_msecs()
> and make the API be in miliseconds.

>> @@ -295,6 +297,19 @@ struct cdrom_generic_command
>>  	};
>>  };
>> +/* This struct is used by CDROM_TIMED_MEDIA_CHANGE */
>> +struct cdrom_timed_media_change_info
>> +{
>> +	__u64   last_media_change;	/* Timestamp of the last detected media
>> +					 * change. May be set by caller, updated
>> +					 * upon successful return of ioctl.
>> +					 */
>> +	__u8    has_changed;		/* Set to 1 by ioctl if last detected media
>> +					 * change was more recent than
>> +					 * last_media_change set by caller.
>> +					 */
>> +};
> The struct layout should be modified such that there are no holes or
> padding in it. Probably just make the has_changed a flags thing, and
> make it u64 as well. Then you can define bit 0 to be HAS_CHANGED, and
> that leaves you room to add more flags in the future. Though the latter
> probably isn't much of a concern here, but...

1. jiffies_to_msecs returns unsigned int. Should I reflect that in the struct (i.e., make the
last_media_change and has_changed fields also of type unsigned int or should I keep them at
a fixed bit width?

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

time_before(msecs_to_jiffies(jiffies_to_msecs(x)), x) is always false ?

I tried to determine that from the code, but I haven't reached a conclusion that I'm confident in..

Kind regards,

  reply	other threads:[~2021-08-27 17:30 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 [this message]
2021-08-28  3:18     ` Jens Axboe
2021-08-28 10:27       ` Lukas Prediger
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:

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

  git send-email \ \ \ \ \
    --subject='Re: [PATCH] drivers/cdrom: improved ioctl for media change detection' \

* 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).