[patch] brd: fix ramdisk regression

[patch] brd: fix ramdisk regression

[Nick Piggin]

From: Marcin Krol <hawk@pld-linux.org>

In 2.6.25, ramdisk devices show up in /proc/partitions, which is a
behaviour change from the old rd.c. Add GENHD_FL_SUPPRESS_PARTITION_INFO,
which was present in rd.c.

Signed-off-by: Marcin Krol <hawk@pld-linux.org>
--
--- linux-2.6.25/drivers/block/brd.c.orig	2008-04-17 04:49:44.000000000 +0200
+++ linux-2.6.25/drivers/block/brd.c	2008-05-18 01:18:28.381903343 +0200
@@ -442,6 +442,7 @@
 	disk->fops		= &brd_fops;
 	disk->private_data	= brd;
 	disk->queue		= brd->brd_queue;
+	disk->flags |= GENHD_FL_SUPPRESS_PARTITION_INFO;
 	sprintf(disk->disk_name, "ram%d", i);
 	set_capacity(disk, rd_size * 2);
 

Re: [patch] brd: fix ramdisk regression

[Andrew Morton]

On Tue, 20 May 2008 15:41:25 +0200
Nick Piggin <npiggin@suse.de> wrote:

> From: Marcin Krol <hawk@pld-linux.org>
> 
> In 2.6.25, ramdisk devices show up in /proc/partitions, which is a
> behaviour change from the old rd.c. Add GENHD_FL_SUPPRESS_PARTITION_INFO,
> which was present in rd.c.
> 
> Signed-off-by: Marcin Krol <hawk@pld-linux.org>

I added your signed-off-by: to this patch.

I renamed it to "brd: don't show ramdisks in /proc/partitions"

> --
> --- linux-2.6.25/drivers/block/brd.c.orig	2008-04-17 04:49:44.000000000 +0200
> +++ linux-2.6.25/drivers/block/brd.c	2008-05-18 01:18:28.381903343 +0200
> @@ -442,6 +442,7 @@
>  	disk->fops		= &brd_fops;
>  	disk->private_data	= brd;
>  	disk->queue		= brd->brd_queue;
> +	disk->flags |= GENHD_FL_SUPPRESS_PARTITION_INFO;
>  	sprintf(disk->disk_name, "ram%d", i);
>  	set_capacity(disk, rd_size * 2);
>  

Why is it a "regression"?

The change in 2.6.25 was a back-compatible one.

This change is not a back-compatible one and if we're going to now
withdraw the newly-added 2.6.25 feature then we should also withdraw it
from 2.6.26.x and 2.6.25.x (if that is still under maintenance).  To
reduce the incidence of "hey where did my feature go" problems.

Really, life would be simpler if we just left the accidentally-added
feature in place.  What problems does it cause?

Re: [patch] brd: fix ramdisk regression

[Marcin Krol]

>> --- linux-2.6.25/drivers/block/brd.c.orig	2008-04-17 04:49:44.000000000 +0200
>> +++ linux-2.6.25/drivers/block/brd.c	2008-05-18 01:18:28.381903343 +0200
>> @@ -442,6 +442,7 @@
>>  	disk->fops		= &brd_fops;
>>  	disk->private_data	= brd;
>>  	disk->queue		= brd->brd_queue;
>> +	disk->flags |= GENHD_FL_SUPPRESS_PARTITION_INFO;
>>  	sprintf(disk->disk_name, "ram%d", i);
>>  	set_capacity(disk, rd_size * 2);
>>  
> 
> Why is it a "regression"?
> 
> The change in 2.6.25 was a back-compatible one.
> 
> This change is not a back-compatible one and if we're going to now
> withdraw the newly-added 2.6.25 feature then we should also withdraw it
> from 2.6.26.x and 2.6.25.x (if that is still under maintenance).  To
> reduce the incidence of "hey where did my feature go" problems.
> 
> Really, life would be simpler if we just left the accidentally-added
> feature in place.  What problems does it cause?

All kernels prior to 2.6.25 weren't displaying ramdisks in 
/proc/partitions. Since there are many userspace tools using infromation 
from /proc/partitions some of them may now behave incorrectly (I didn't 
tested any though). For example before 2.6.25 /proc/partitions was empty 
if no block devices like hard disks and such were detected by kernel. 
Now all 16 ramdisks are always visible there. Some software may rely on 
such information (I mean, on empty /proc/partitions).

There was quite similar situation back in 2004, and ramdisks were 
excluded back from displaying. Thats why I called this a regression 
(maybe a bit unfortunate). See this patch for info: 
http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.3-rc2/2.6.3-rc2-mm1/broken-out/nbd-proc-partitions-fix.patch

I also think that someone somewhere (long time ago) excluded ramdisks 
from /proc/partitions for good reasons. It is possible that now such new 
"feature" is harmless, but I think there are more chances that someone 
will say "hey, /proc/partitions has changed, now my software doesn't 
work" then "hey where did my new 2.6.25 feature go". nbd devices are 
also excluded, maybe for very same (unknown to me) reasons.

For me this change isn't problematic. Yes, it did broken my software and 
yes, I could fix my software but since its working only on specific 
kernels (precompiled by me) I've chosen to revert kernel change instead 
to get old behaviour.

Marcin

Re: [patch] brd: fix ramdisk regression

[Andrew Morton]

On Tue, 20 May 2008 22:24:01 +0200
Marcin Krol <hawk@pld-linux.org> wrote:

> >> --- linux-2.6.25/drivers/block/brd.c.orig	2008-04-17 04:49:44.000000000 +0200
> >> +++ linux-2.6.25/drivers/block/brd.c	2008-05-18 01:18:28.381903343 +0200
> >> @@ -442,6 +442,7 @@
> >>  	disk->fops		= &brd_fops;
> >>  	disk->private_data	= brd;
> >>  	disk->queue		= brd->brd_queue;
> >> +	disk->flags |= GENHD_FL_SUPPRESS_PARTITION_INFO;
> >>  	sprintf(disk->disk_name, "ram%d", i);
> >>  	set_capacity(disk, rd_size * 2);
> >>  
> > 
> > Why is it a "regression"?
> > 
> > The change in 2.6.25 was a back-compatible one.
> > 
> > This change is not a back-compatible one and if we're going to now
> > withdraw the newly-added 2.6.25 feature then we should also withdraw it
> > from 2.6.26.x and 2.6.25.x (if that is still under maintenance).  To
> > reduce the incidence of "hey where did my feature go" problems.
> > 
> > Really, life would be simpler if we just left the accidentally-added
> > feature in place.  What problems does it cause?
> 
> All kernels prior to 2.6.25 weren't displaying ramdisks in 
> /proc/partitions. Since there are many userspace tools using infromation 
> from /proc/partitions some of them may now behave incorrectly (I didn't 
> tested any though). For example before 2.6.25 /proc/partitions was empty 
> if no block devices like hard disks and such were detected by kernel. 
> Now all 16 ramdisks are always visible there. Some software may rely on 
> such information (I mean, on empty /proc/partitions).
> 
> There was quite similar situation back in 2004, and ramdisks were 
> excluded back from displaying. Thats why I called this a regression 
> (maybe a bit unfortunate). See this patch for info: 
> http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.3-rc2/2.6.3-rc2-mm1/broken-out/nbd-proc-partitions-fix.patch
> 
> I also think that someone somewhere (long time ago) excluded ramdisks 
> from /proc/partitions for good reasons. It is possible that now such new 
> "feature" is harmless, but I think there are more chances that someone 
> will say "hey, /proc/partitions has changed, now my software doesn't 
> work" then "hey where did my new 2.6.25 feature go". nbd devices are 
> also excluded, maybe for very same (unknown to me) reasons.

I added the above info to the changelog.  But it's still a bit vague,
and as far as I know we don't actually know about any breakage (do we?)

> For me this change isn't problematic. Yes, it did broken my software and 
> yes, I could fix my software but since its working only on specific 
> kernels (precompiled by me) I've chosen to revert kernel change instead 
> to get old behaviour.

Ah, it only affects 2.6.25.x and no earlier -stable kernels.  Not so bad.

Greg, Chris?  Thoughts?

Re: [stable] [patch] brd: fix ramdisk regression

[Chris Wright]

* Andrew Morton (akpm@linux-foundation.org) wrote:
> > For me this change isn't problematic. Yes, it did broken my software and 
> > yes, I could fix my software but since its working only on specific 
> > kernels (precompiled by me) I've chosen to revert kernel change instead 
> > to get old behaviour.
> 
> Ah, it only affects 2.6.25.x and no earlier -stable kernels.  Not so bad.
> 
> Greg, Chris?  Thoughts?

I don't know why it was suppressed to begin with.  Seems /proc/partitions
parsers already need to deal w/ arbitrary number and type of block
devices, so is it really a regression (it was also just removed from nbd)?

thanks,
-chris