All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kay Sievers <kay.sievers@vrfy.org>
To: Florian Mickler <florian@mickler.org>
Cc: Maxim Levitsky <maximlevitsky@gmail.com>,
	Tejun Heo <tj@kernel.org>,
	Henrique de Moraes Holschuh <hmh@hmh.eng.br>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Jens Axboe <axboe@kernel.dk>
Subject: Re: [REGRESSION] cdrom drive doesn't detect removal
Date: Thu, 30 Sep 2010 21:27:52 +0200	[thread overview]
Message-ID: <AANLkTim_ZW6D6TKNh9W5Qn2biC0_BQcPuMKKKS6_X-bp@mail.gmail.com> (raw)
In-Reply-To: <20100930164918.22f40a83@schatten.dmk.lab>

On Thu, Sep 30, 2010 at 16:49, Florian Mickler <florian@mickler.org> wrote:
> On Thu, 30 Sep 2010 16:17:32 +0200
> Maxim Levitsky <maximlevitsky@gmail.com> wrote:
>> On Thu, 2010-09-30 at 13:38 +0200, Florian Mickler wrote:
>> > On Thu, 30 Sep 2010 09:48:50 +0200
>> > Kay Sievers <kay.sievers@vrfy.org> wrote:
>> > > >
>> > > > So, is the $subject problem solved now? Normally, we shouldn't break
>> > > > stuff with new kernels... If this is only a temporary breakage on
>> > > > the other hand, we should keep track of it...
>> > > > I ask, because this is listed as https://bugzilla.kernel.org/show_bug.cgi?id=18522.
>> > > > If it should stay listed, we may need an ETA for the fix...
>> > >
>> > > Hmm, I still don't think that's a bug or regression.
>> > >
>> > > Optical drives are not supposed to report media changes without
>> > > constantly being polled. Why Tejun's seems to have an influence in
>> > > Maxim's setup, is likely more a timing-related issue, or some other
>> > > thing, we never really got an idea why it could change anything.
>> > >
>> > > The current behavior is the expected and correct behavior, and for me
>> > > also the older kernels behave like this.
>> >
>> > So I'm gonna close this as invalid then. Please shout, if that's
>> > not ok.
>> Its not invalid at all.
>>
>> Let me explain again the problem:
>>
>> Indeed cdrom drivers need to be polled.
>> And yes, both udisks and hal _do_ poll the drive.
>>
>> However, udisks uses O_EXCL, when it opens the drive, and that fails s
>> soon as filesystem is mounted, therefore effectively, as soon as disk is
>> inserted, polling stops.
>>
>> Without the bisected commit, O_EXCL would still fail, however due to the
>> bug that commit fixed, the drive will still be polled.
>>
>> Hal also polls the drive, but it has a workaround, that is if open with
>> O_EXCL fails, it retries the open without it.
>>
>> This is pretty much all, there are no unknowns left.
>>
>>
>> However the reason behind O_EXCL is that we don't want polling while a
>> disk is being written (burned), but we do want polling while disk is
>> mounted.
>>
>> So just 'fixing' udisk to not take O_EXCL will bring us to square one,
>> as well as reverting the commit.
>>
>> Therefore this problem doesn't have a simple solution.
>> I think that the best solution is to move polling to the kernel, and
>> introduce a way for burning application to take absolute exclusive
>> access to the device and as a part of that disable that in-kernel
>> polling.
>> A new ioctl will suffice, or the meaning of O_EXCL can be changed for
>> cdrom driver only (if somebody has it open with O_EXCL, fail all opens,
>> including the ones that don't have O_EXCL).

> Ok. I will reopen then. Is someone working on fixing this issue?
>
> If we leave it open, we wanna have an ETA for the fix so that people
> trolling the bugzilla know for how long they have to let this report
> simmer.

I don't think anything needs to be or can really be fixed in the
kernel. We need a working O_EXCL, and before Tejun's patch it was
broken. That the broken behavior seemed to have some wanted effects on
some boxes can't the reason to leave O_EXCL broken.

The current state is the expected behavior. It all seems more like an
issue with current udisks which should be discussed at the freedesktop
bugzilla. Udisk's behavior is intended, but there might be no specific
reason not to change it to what HAL did.

Kay

  reply	other threads:[~2010-09-30 19:28 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-12  9:49 [REGRESSION] cdrom drive doesn't detect removal Maxim Levitsky
2010-09-14  1:27 ` Maxim Levitsky
2010-09-14  7:39   ` Tejun Heo
2010-09-14  8:07     ` Kay Sievers
2010-09-14 23:38       ` Maxim Levitsky
2010-09-14 23:49         ` Kay Sievers
2010-09-15  0:37           ` Maxim Levitsky
2010-09-15  1:01             ` Kay Sievers
2010-09-15 13:27               ` Henrique de Moraes Holschuh
2010-09-15 13:44                 ` Kay Sievers
2010-09-15 22:20                   ` Maxim Levitsky
2010-09-16  6:51                     ` Kay Sievers
2010-09-21 11:42                       ` Maxim Levitsky
2010-09-21 23:09                         ` Maxim Levitsky
2010-09-22  7:38                           ` Tejun Heo
2010-09-22 13:41                             ` Maxim Levitsky
2010-09-22 13:58                               ` Maxim Levitsky
2010-09-23  8:47                                 ` Tejun Heo
2010-09-23  9:21                                   ` Kay Sievers
2010-09-30  6:30                                     ` Florian Mickler
2010-09-30  7:48                                       ` Kay Sievers
2010-09-30 11:38                                         ` Florian Mickler
2010-09-30 14:17                                           ` Maxim Levitsky
2010-09-30 14:49                                             ` Florian Mickler
2010-09-30 19:27                                               ` Kay Sievers [this message]
2010-09-30 20:14                                                 ` Florian Mickler
2010-09-30 20:32                                                   ` Kay Sievers
2010-09-30 20:47                                                     ` Florian Mickler
2010-09-30 20:57                                                       ` Kay Sievers
2010-10-01  5:55                                               ` Tejun Heo
2010-10-01  7:54                                                 ` Florian Mickler

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=AANLkTim_ZW6D6TKNh9W5Qn2biC0_BQcPuMKKKS6_X-bp@mail.gmail.com \
    --to=kay.sievers@vrfy.org \
    --cc=axboe@kernel.dk \
    --cc=florian@mickler.org \
    --cc=hmh@hmh.eng.br \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maximlevitsky@gmail.com \
    --cc=tj@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.