diff mbox

ide-cd: Improve "weird block size" error message

Message ID 200906230951.24615.elendil@planet.nl
State Accepted
Delegated to: David Miller
Headers show

Commit Message

Frans Pop June 23, 2009, 7:51 a.m. UTC
On Monday 22 June 2009, Bartlomiej Zolnierkiewicz wrote:
> On Monday 22 June 2009 21:01:37 Frans Pop wrote:
> > There is one thing I should mention here. I have been seeing the
> > following error with this CD drive:
> > ide-cd: hdd: weird block size 2352
> > ide-cd: hdd: default to 2kb block size
>
> This is just a harmless warning coming from enabling of the workaround
> for weird ATAPI devices (the one you have in this sparc machine seems
> to score really high on the weirdness scale ;) introduced by commit
> e8e7b9e.

In that case I'd like to propose the following patch. Currently the error
can get printed much to frequently when there's a disc in the drive.
Example:

Jun 13 18:06:28 gimli kernel: ide-cd: hdd: weird block size 2352
Jun 13 18:06:28 gimli kernel: ide-cd: hdd: default to 2kb block size
Jun 13 18:06:32 gimli kernel: ide-cd: hdd: weird block size 2352
Jun 13 18:06:42 gimli kernel: ide-cd: hdd: default to 2kb block size
Jun 13 18:07:02 gimli kernel: ide-cd: hdd: weird block size 2352
Jun 13 18:07:02 gimli kernel: ide-cd: hdd: default to 2kb block size
Jun 13 18:07:05 gimli kernel: ide-cd: hdd: weird block size 2352
Jun 13 18:07:05 gimli kernel: ide-cd: hdd: default to 2kb block size
Jun 13 18:07:09 gimli kernel: ide-cd: hdd: weird block size 2352
Jun 13 18:07:09 gimli kernel: ide-cd: hdd: default to 2kb block size
Jun 13 18:07:14 gimli kernel: ide-cd: hdd: weird block size 2352
Jun 13 18:07:14 gimli kernel: ide-cd: hdd: default to 2kb block size
Jun 13 18:07:35 gimli kernel: ide-cd: hdd: weird block size 2352
Jun 13 18:07:35 gimli kernel: ide-cd: hdd: default to 2kb block size
Jun 13 18:07:51 gimli kernel: ide-cd: hdd: weird block size 2352
Jun 13 18:07:51 gimli kernel: ide-cd: hdd: default to 2kb block size

I was not using the CD at all here. I suspect HAL's stupid polling to be
the culprit as I first saw it after upgrading X.Org packages to a version
which depends on HAL. I since disabled polling for the device, but I
still feel that warning once should be sufficient as IIUC the value is
device dependent and not medium dependent.

With the patch it only gets printed once, when the driver is initialized.

Cheers,
FJP

---
From: Frans Pop <elendil@planet.nl>
Subject: ide-cd: Improve "weird block size" error message

Currently the error gets repeated too frequently, for example
each time HAL polls the device when a disc is present. Avoid that
by using printk_once instead of printk.
Also join the error and corrective action messages into a single line.

Signed-off-by: Frans Pop <elendil@planet.nl>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Borislav Petkov June 23, 2009, 7:57 a.m. UTC | #1
On Tue, Jun 23, 2009 at 09:51:23AM +0200, Frans Pop wrote:

[..]

> ---
> From: Frans Pop <elendil@planet.nl>
> Subject: ide-cd: Improve "weird block size" error message
> 
> Currently the error gets repeated too frequently, for example
> each time HAL polls the device when a disc is present. Avoid that
> by using printk_once instead of printk.
> Also join the error and corrective action messages into a single line.
> 
> Signed-off-by: Frans Pop <elendil@planet.nl>
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> 
> diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
> index 4a19686..7ec6996 100644
> --- a/drivers/ide/ide-cd.c
> +++ b/drivers/ide/ide-cd.c
> @@ -886,10 +886,9 @@ static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity,
>  	case 4096:
>  		break;
>  	default:
> -		printk(KERN_ERR PFX "%s: weird block size %u\n",
> +		printk_once(KERN_ERR PFX "%s: weird block size %u; "
> +				"setting default block size to 2048\n",
>  				drive->name, blocklen);
> -		printk(KERN_ERR PFX "%s: default to 2kb block size\n",
> -				drive->name);

Please leave the weird block size in the printk since it sometimes might
give insights on what is going on.

Thanks.
Borislav Petkov June 23, 2009, 8:02 a.m. UTC | #2
On Tue, Jun 23, 2009 at 09:57:33AM +0200, Borislav Petkov wrote:
> On Tue, Jun 23, 2009 at 09:51:23AM +0200, Frans Pop wrote:
> 
> [..]
> 
> > ---
> > From: Frans Pop <elendil@planet.nl>
> > Subject: ide-cd: Improve "weird block size" error message
> > 
> > Currently the error gets repeated too frequently, for example
> > each time HAL polls the device when a disc is present. Avoid that
> > by using printk_once instead of printk.
> > Also join the error and corrective action messages into a single line.
> > 
> > Signed-off-by: Frans Pop <elendil@planet.nl>
> > Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > 
> > diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
> > index 4a19686..7ec6996 100644
> > --- a/drivers/ide/ide-cd.c
> > +++ b/drivers/ide/ide-cd.c
> > @@ -886,10 +886,9 @@ static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity,
> >  	case 4096:
> >  		break;
> >  	default:
> > -		printk(KERN_ERR PFX "%s: weird block size %u\n",
> > +		printk_once(KERN_ERR PFX "%s: weird block size %u; "
> > +				"setting default block size to 2048\n",
> >  				drive->name, blocklen);
> > -		printk(KERN_ERR PFX "%s: default to 2kb block size\n",
> > -				drive->name);
>

Ah, nevermind! Hadn't had a coffee yet, sorry :).

Acked-by: Borislav Petkov <petkovbb@gmail.com>
Frans Pop June 23, 2009, 8:20 a.m. UTC | #3
On Tuesday 23 June 2009, Borislav Petkov wrote:
> > diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
> > index 4a19686..7ec6996 100644
> > --- a/drivers/ide/ide-cd.c
> > +++ b/drivers/ide/ide-cd.c
> > @@ -886,10 +886,9 @@ static int cdrom_read_capacity(ide_drive_t
> > *drive, unsigned long *capacity, case 4096:
> >  		break;
> >  	default:
> > -		printk(KERN_ERR PFX "%s: weird block size %u\n",
> > +		printk_once(KERN_ERR PFX "%s: weird block size %u; "
                                                          ^^^^^^^
> > +				"setting default block size to 2048\n",
> >  				drive->name, blocklen);
> > -		printk(KERN_ERR PFX "%s: default to 2kb block size\n",
> > -				drive->name);
>
> Please leave the weird block size in the printk since it sometimes
> might give insights on what is going on.

I did :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 23, 2009, 11:03 p.m. UTC | #4
From: Borislav Petkov <petkovbb@googlemail.com>
Date: Tue, 23 Jun 2009 10:02:23 +0200

> On Tue, Jun 23, 2009 at 09:57:33AM +0200, Borislav Petkov wrote:
>> On Tue, Jun 23, 2009 at 09:51:23AM +0200, Frans Pop wrote:
>> 
>> [..]
>> 
>> > ---
>> > From: Frans Pop <elendil@planet.nl>
>> > Subject: ide-cd: Improve "weird block size" error message
>> > 
>> > Currently the error gets repeated too frequently, for example
>> > each time HAL polls the device when a disc is present. Avoid that
>> > by using printk_once instead of printk.
>> > Also join the error and corrective action messages into a single line.
>> > 
>> > Signed-off-by: Frans Pop <elendil@planet.nl>
>> > Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
 ...
> Acked-by: Borislav Petkov <petkovbb@gmail.com>

Applied, thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 4a19686..7ec6996 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -886,10 +886,9 @@  static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity,
 	case 4096:
 		break;
 	default:
-		printk(KERN_ERR PFX "%s: weird block size %u\n",
+		printk_once(KERN_ERR PFX "%s: weird block size %u; "
+				"setting default block size to 2048\n",
 				drive->name, blocklen);
-		printk(KERN_ERR PFX "%s: default to 2kb block size\n",
-				drive->name);
 		blocklen = 2048;
 		break;
 	}