From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal =?UTF-8?B?U3VjaMOhbmVr?= Subject: Re: [PATCH resend 3/6] cdrom: wait for tray to close Date: Wed, 31 Jan 2018 19:20:27 +0100 Message-ID: <20180131192027.070196c1@kitsune.suse.cz> References: <03915a2e64f50ec04ea6d8e6f80e36ecf16e4f0f.1516985620.git.msuchanek@suse.de> <1517245546.2687.17.camel@wdc.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1517245546.2687.17.camel@wdc.com> Sender: linux-kernel-owner@vger.kernel.org To: Bart Van Assche Cc: "corbet@lwn.net" , "linux-kernel@vger.kernel.org" , "linux-ide@vger.kernel.org" , "keescook@chromium.org" , "tglx@linutronix.de" , "martin.petersen@oracle.com" , "axboe@kernel.dk" , "linux-scsi@vger.kernel.org" , "kstewart@linuxfoundation.org" , "pombredanne@nexb.com" , "bp@alien8.de" , "gregkh@linuxfoundation.org" , "jejb@linux.vnet.ibm.com" , "christophe.jaillet@wanadoo.fr" , "davem@davemloft.net" , linux-doc@ List-Id: linux-ide@vger.kernel.org On Mon, 29 Jan 2018 17:05:47 +0000 Bart Van Assche wrote: > On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote: > > +static int cdrom_tray_close(struct cdrom_device_info *cdi) > > +{ > > + int ret; > > + > > + ret = cdi->ops->tray_move(cdi, 0); > > + if (ret || !cdi->ops->drive_status) > > + return ret; > > + > > + return poll_event_interruptible(CDS_TRAY_OPEN != > > + cdi->ops->drive_status(cdi, CDSL_CURRENT), > > 500); +} > > + > > static > > int open_for_common(struct cdrom_device_info *cdi, tracktype > > *tracks) { > > @@ -1048,7 +1062,9 @@ int open_for_common(struct cdrom_device_info > > *cdi, tracktype *tracks) if (CDROM_CAN(CDC_CLOSE_TRAY) && > > cdi->options & CDO_AUTO_CLOSE) { > > cd_dbg(CD_OPEN, "trying to close > > the tray\n"); > > - ret = cdo->tray_move(cdi, 0); > > + ret = cdrom_tray_close(cdi); > > + if (ret == -ERESTARTSYS) > > + return ret; > > if (ret) { > > cd_dbg(CD_OPEN, "bummer. > > tried to close the tray but failed.\n"); /* Ignore the error from > > the low @@ -2312,7 +2328,8 @@ static int > > cdrom_ioctl_closetray(struct cdrom_device_info *cdi) > > if (!CDROM_CAN(CDC_CLOSE_TRAY)) > > return -ENOSYS; > > - return cdi->ops->tray_move(cdi, 0); > > + > > + return cdrom_tray_close(cdi); > > } > > So this patch changes code that does not wait into code that > potentially waits forever? Sorry but I don't think that's ideal. > Please make sure that after a certain time (a few seconds?) the loop > finishes. The problem is that few seconds does not cut it. We are waiting for a mechanical tray or CD changer to move. On non-broken hardware the tray either closes or an error is reported within a few seconds. For the timeout to not race with the event we are waiting for it must be much longer, though. Also note that this code is only invoked when the user specifically requested that the tray gets closed automatically which is off by default. Thanks Michal From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AH8x224beNGrX5O+jxjVUijY6pHfLjNA/qOXEX9d2sNeDN3uNooR2PIxcDOcHv4uJvPlnhCOtJnz ARC-Seal: i=1; a=rsa-sha256; t=1517422833; cv=none; d=google.com; s=arc-20160816; b=kfjakylnZlhEmaDuDm/9CSdBSyZafIymXPcGBWxV5NOj+9Rxe/7Uh6uyY32IEobST1 5ZyNaWzOhjUpw4h3+zBgxVnph4aSXi9Dy88ThhV7vwc+URaKArQ102levNrsGeQOYHz2 Ogzsbdwwt8o2jn/hTZpRt9bVEnTS5ra+aBbrtX0Ny+ppXLXO2gWwpVOJeqpqUMVZJhzS qIrlXGGGW+nnqQri4Mjhs5Amv6mlSum+5O05bonLvw7E1QjIWd+P5APST73U/3NyNYf/ c0R9VFxRmZ+tileDqlOy3YGyTIxat2VQkhXQqeU2lYV1cONxcwcnBF+wYfwGdjF+6eu3 K7jA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date :arc-authentication-results; bh=jcUQELfA0vnR6/xBm/duqHGXduNIpBVc/1eAqzeZEYs=; b=O97iKEX1NVzNReWV5AvwHvUruJinr12EZpdjVNJVhU/vKdObUXxqenfg8oI5vJoqZ3 sQqUYNv43Yeb6el/LyeIilC1VLaaxhxlt+MHqXXRYY7up2fbH6iarxLOuqA1RjCRIIqe fdQCTutdTBv6ISeCc+5o0X3lA2ig3pqNRJxE0/uA5vkYm83rtq/x37aaMStVbkrqsC63 NYSRt2x+t5m/FdmRZRd81GlAufPbun14qcJ0QIBcf9Pdk/Vp+pC8m6IIDQ0OsUaS2KiT M/rmGHwkbehiCKvagLmx3kbP2NeKUonjBa5HTkqsf19KRDNl6ZMRkGIEHXZIFx5tt+Dr XgSA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of msuchanek@suse.de designates 195.135.220.15 as permitted sender) smtp.mailfrom=msuchanek@suse.de Authentication-Results: mx.google.com; spf=pass (google.com: domain of msuchanek@suse.de designates 195.135.220.15 as permitted sender) smtp.mailfrom=msuchanek@suse.de Date: Wed, 31 Jan 2018 19:20:27 +0100 From: Michal =?UTF-8?B?U3VjaMOhbmVr?= To: Bart Van Assche Cc: "corbet@lwn.net" , "linux-kernel@vger.kernel.org" , "linux-ide@vger.kernel.org" , "keescook@chromium.org" , "tglx@linutronix.de" , "martin.petersen@oracle.com" , "axboe@kernel.dk" , "linux-scsi@vger.kernel.org" , "kstewart@linuxfoundation.org" , "pombredanne@nexb.com" , "bp@alien8.de" , "gregkh@linuxfoundation.org" , "jejb@linux.vnet.ibm.com" , "christophe.jaillet@wanadoo.fr" , "davem@davemloft.net" , "linux-doc@vger.kernel.org" , "tim@cyberelk.net" Subject: Re: [PATCH resend 3/6] cdrom: wait for tray to close Message-ID: <20180131192027.070196c1@kitsune.suse.cz> In-Reply-To: <1517245546.2687.17.camel@wdc.com> References: <03915a2e64f50ec04ea6d8e6f80e36ecf16e4f0f.1516985620.git.msuchanek@suse.de> <1517245546.2687.17.camel@wdc.com> Organization: SUSE Linux X-Mailer: Claws Mail 3.15.1-dirty (GTK+ 2.24.31; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1590761212542102638?= X-GMAIL-MSGID: =?utf-8?q?1591133164787649518?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Mon, 29 Jan 2018 17:05:47 +0000 Bart Van Assche wrote: > On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote: > > +static int cdrom_tray_close(struct cdrom_device_info *cdi) > > +{ > > + int ret; > > + > > + ret = cdi->ops->tray_move(cdi, 0); > > + if (ret || !cdi->ops->drive_status) > > + return ret; > > + > > + return poll_event_interruptible(CDS_TRAY_OPEN != > > + cdi->ops->drive_status(cdi, CDSL_CURRENT), > > 500); +} > > + > > static > > int open_for_common(struct cdrom_device_info *cdi, tracktype > > *tracks) { > > @@ -1048,7 +1062,9 @@ int open_for_common(struct cdrom_device_info > > *cdi, tracktype *tracks) if (CDROM_CAN(CDC_CLOSE_TRAY) && > > cdi->options & CDO_AUTO_CLOSE) { > > cd_dbg(CD_OPEN, "trying to close > > the tray\n"); > > - ret = cdo->tray_move(cdi, 0); > > + ret = cdrom_tray_close(cdi); > > + if (ret == -ERESTARTSYS) > > + return ret; > > if (ret) { > > cd_dbg(CD_OPEN, "bummer. > > tried to close the tray but failed.\n"); /* Ignore the error from > > the low @@ -2312,7 +2328,8 @@ static int > > cdrom_ioctl_closetray(struct cdrom_device_info *cdi) > > if (!CDROM_CAN(CDC_CLOSE_TRAY)) > > return -ENOSYS; > > - return cdi->ops->tray_move(cdi, 0); > > + > > + return cdrom_tray_close(cdi); > > } > > So this patch changes code that does not wait into code that > potentially waits forever? Sorry but I don't think that's ideal. > Please make sure that after a certain time (a few seconds?) the loop > finishes. The problem is that few seconds does not cut it. We are waiting for a mechanical tray or CD changer to move. On non-broken hardware the tray either closes or an error is reported within a few seconds. For the timeout to not race with the event we are waiting for it must be much longer, though. Also note that this code is only invoked when the user specifically requested that the tray gets closed automatically which is off by default. Thanks Michal