2008-06-07 01:10:33

by Chris Wright

[permalink] [raw]
Subject: [patch 21/50] brd: dont show ramdisks in /proc/partitions

-stable review patch. If anyone has any objections, please let us know.
---------------------

From: Marcin Krol <[email protected]>

upstream commit: 53978d0a7a27eb036b9bf33c4caa06257a9dbed7

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.

All kernels prior to 2.6.25 weren't displaying ramdisks in
/proc/partitions. Since there are many userspace tools using information
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.

Signed-off-by: Marcin Krol <[email protected]>
Signed-off-by: Nick Piggin <[email protected]>
Cc: <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
---
drivers/block/brd.c | 1 +
1 file changed, 1 insertion(+)

--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -442,6 +442,7 @@ static struct brd_device *brd_alloc(int
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);


--


2008-06-07 22:19:56

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch 21/50] brd: dont show ramdisks in /proc/partitions

Chris Wright wrote:
> From: Marcin Krol <[email protected]>
>
> 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.

This seems a bit like cargo cult programming to me. If there isn't a
known, good, reason to revert this behaviour change, I would consider it
a bugfix, not a regression.

-hpa

2008-06-09 17:17:51

by Chris Wright

[permalink] [raw]
Subject: Re: [stable] [patch 21/50] brd: dont show ramdisks in /proc/partitions

* H. Peter Anvin ([email protected]) wrote:
> This seems a bit like cargo cult programming to me. If there isn't a
> known, good, reason to revert this behaviour change, I would consider it
> a bugfix, not a regression.

We have one /proc/partitions parser that got broken (never saw details
on how). I don't care for this change either (esp. since nbd is still
left out...it's inconsistent).

This is now the upstream behaviour (obvious since it's a -stable
candidate), and the change was introduced in 2.6.25...I'd actually prefer
to drop this patch, but there's no good reason to deviate if upstream
keeps this one. I can defer while upstream gets revisited, can you
revisit this upstream?

thanks,
-chris

2008-06-09 20:07:44

by Willy Tarreau

[permalink] [raw]
Subject: Re: [stable] [patch 21/50] brd: dont show ramdisks in /proc/partitions

On Mon, Jun 09, 2008 at 10:08:40AM -0700, Chris Wright wrote:
> * H. Peter Anvin ([email protected]) wrote:
> > This seems a bit like cargo cult programming to me. If there isn't a
> > known, good, reason to revert this behaviour change, I would consider it
> > a bugfix, not a regression.
>
> We have one /proc/partitions parser that got broken (never saw details
> on how). I don't care for this change either (esp. since nbd is still
> left out...it's inconsistent).

I also had one of my old install scripts proposing me to choose between
16 ramdisks to install to. Quite bothering in fact. I'm not sure we
really can consider ramdisks as partitions. Also, all of them are listed,
whether they are initialized or not.

In fact, I was glad to see this "fixed" with this patch.

Regards,
willy

2008-06-09 20:55:18

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [stable] [patch 21/50] brd: dont show ramdisks in /proc/partitions

Chris Wright wrote:
> * H. Peter Anvin ([email protected]) wrote:
>> This seems a bit like cargo cult programming to me. If there isn't a
>> known, good, reason to revert this behaviour change, I would consider it
>> a bugfix, not a regression.
>
> We have one /proc/partitions parser that got broken (never saw details
> on how). I don't care for this change either (esp. since nbd is still
> left out...it's inconsistent).
>
> This is now the upstream behaviour (obvious since it's a -stable
> candidate), and the change was introduced in 2.6.25...I'd actually prefer
> to drop this patch, but there's no good reason to deviate if upstream
> keeps this one. I can defer while upstream gets revisited, can you
> revisit this upstream?

I had to ask Chris offline what he meant with this... basically, he and
I both agree that absent hard evidence of malfunction, we should revert
to 2.6.25 behaviour.

-hpa