On Wed, 2011-06-15 at 11:40 +0200, Tejun Heo wrote: > Hello, > > On Wed, Jun 15, 2011 at 11:34:23AM +0200, Kay Sievers wrote: > > Any idea why with the first check GET_EVENT seems, but TUR seems to > > indicate no change? > > That's probably because probing logic already consumed UA. Reset > during probe also causes UA so detection logic consumes them > afterwards. I don't think driver can tell apart different UAs at that > point. If the device determines to report media change after reset > via GET_EVENT, you get a mismatch. So, it's more or less expected. Here is a new patch. Would be great, if one of you guys can check if it fixes the loop we are currently seeing. Thanks, Kay From: Kay Sievers <kay.sievers@xxxxxxxx> Subject: sr: check_events() ignore GET_EVENT when TUR says otherwise Some (broken) devices return GET_EVENT at every open() which lets udev run into a loop when the device is accessed fron an event handler. When GET_EVENT and TUR disagree for a few times in a row, we ignore the GET_EVENT events, and trust only the TUR status. This is the log of a USB stick with a (broken) fake CDROM drive: scsi 5:0:0:0: Direct-Access SanDisk U3 Cruzer Micro 8.02 PQ: 0 ANSI: 0 CCS sd 5:0:0:0: Attached scsi generic sg3 type 0 scsi 5:0:0:1: CD-ROM SanDisk U3 Cruzer Micro 8.02 PQ: 0 ANSI: 0 sd 5:0:0:0: [sdb] Attached SCSI removable disk sr2: scsi3-mmc drive: 48x/48x tray sr 5:0:0:1: Attached scsi CD-ROM sr2 sr 5:0:0:1: Attached scsi generic sg4 type 5 sr2: GET_EVENT and TUR disagree continuously, suppress GET_EVENT events sd 5:0:0:0: [sdb] 31777279 512-byte logical blocks: (16.2 GB/15.1 GiB) sd 5:0:0:0: [sdb] No Caching mode page present sd 5:0:0:0: [sdb] Assuming drive cache: write through sd 5:0:0:0: [sdb] No Caching mode page present sd 5:0:0:0: [sdb] Assuming drive cache: write through sdb: sdb1 Reported-By: Markus Rathgeb maggu2810@xxxxxxxxxxxxxx Cc: Tejun Heo <tj@xxxxxxxxxx> Signed-off-by: Kay Sievers <kay.sievers@xxxxxxxx> --- diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index 4778e27..1b42dbd 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -229,6 +229,15 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi, if (!(clearing & DISK_EVENT_MEDIA_CHANGE)) goto skip_tur; + /* + * Earlier GET_EVENT_STATUS_NOTIFICATION and TUR did not agree + * for a couple of times in a row. We rely on TUR only for this + * likely broken device, to prevent generating incorrect media + * changed events for every open(). + */ + if (cd->ignore_get_event) + events &= ~DISK_EVENT_MEDIA_CHANGE; + /* let's see whether the media is there with TUR */ last_present = cd->media_present; ret = scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr); @@ -241,8 +250,19 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi, cd->media_present = scsi_status_is_good(ret) || (scsi_sense_valid(&sshdr) && sshdr.asc != 0x3a); - if (last_present != cd->media_present) + if (last_present != cd->media_present) { events |= DISK_EVENT_MEDIA_CHANGE; + } else if (events & DISK_EVENT_MEDIA_CHANGE) { + if (cd->tur_mismatch > 8) { + printk("%s: GET_EVENT and TUR disagree continuously, " + "suppress GET_EVENT events\n", cd->cdi.name); + cd->ignore_get_event = true; + } else { + cd->tur_mismatch++; + } + } else if (!cd->ignore_get_event && cd->tur_mismatch > 0) { + cd->tur_mismatch = 0; + } skip_tur: if (cd->device->changed) { events |= DISK_EVENT_MEDIA_CHANGE; diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h index e036f1d..94a3215 100644 --- a/drivers/scsi/sr.h +++ b/drivers/scsi/sr.h @@ -41,6 +41,8 @@ typedef struct scsi_cd { unsigned readcd_known:1; /* drive supports READ_CD (0xbe) */ unsigned readcd_cdda:1; /* reading audio data using READ_CD */ unsigned media_present:1; /* media is present */ + unsigned ignore_get_event:1; /* get_event is unreliable, use TUR */ + int tur_mismatch; /* nr of get_event TUR mismatches */ struct cdrom_device_info cdi; /* We hold gendisk and scsi_device references on probe and use * the refs on this kref to decide when to release them */ -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html