* Re: [PATCH] fix units/partition count in sd.c (2.4.x)
@ 2005-02-16 22:42 soohoon.lee
0 siblings, 0 replies; 4+ messages in thread
From: soohoon.lee @ 2005-02-16 22:42 UTC (permalink / raw)
To: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 478 bytes --]
Conincidentally I've found the same problem but fixed it differently.
Because nr_real is not real # of devices but max # of devices of a major #,
it doesn't need to be changed on disk add/remove.
1223c1223
< sd_gendisks[i].nr_real = 0;
---
> sd_gendisks[i].nr_real = SCSI_DISKS_PER_MAJOR;
1336d1335
< SD_GENDISK(i).nr_real++;
1450d1448
< SD_GENDISK(i).nr_real--;
2.6 has little different structure but it does like this
sd.c:sd_probe()
gd->minors = 16;
Soohoon.
[-- Attachment #2: diff.patch --]
[-- Type: application/octet-stream, Size: 957 bytes --]
--- drivers/scsi/sd.c.org 2005-02-16 09:45:20.000000000 -0500
+++ drivers/scsi/sd.c 2005-02-16 09:45:38.000000000 -0500
@@ -1220,7 +1220,7 @@
goto cleanup_gendisks_part;
memset(sd_gendisks[i].part, 0, (SCSI_DISKS_PER_MAJOR << 4) * sizeof(struct hd_struct));
sd_gendisks[i].sizes = sd_sizes + (i * SCSI_DISKS_PER_MAJOR << 4);
- sd_gendisks[i].nr_real = 0;
+ sd_gendisks[i].nr_real = SCSI_DISKS_PER_MAJOR;
sd_gendisks[i].real_devices =
(void *) (rscsi_disks + i * SCSI_DISKS_PER_MAJOR);
}
@@ -1333,7 +1333,6 @@
rscsi_disks[i].device = SDp;
rscsi_disks[i].has_part_table = 0;
sd_template.nr_dev++;
- SD_GENDISK(i).nr_real++;
devnum = i % SCSI_DISKS_PER_MAJOR;
SD_GENDISK(i).de_arr[devnum] = SDp->de;
if (SDp->removable)
@@ -1447,7 +1446,6 @@
SDp->attached--;
sd_template.dev_noticed--;
sd_template.nr_dev--;
- SD_GENDISK(i).nr_real--;
return;
}
return;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fix units/partition count in sd.c (2.4.x)
@ 2005-02-26 19:29 Soo Lee
0 siblings, 0 replies; 4+ messages in thread
From: Soo Lee @ 2005-02-26 19:29 UTC (permalink / raw)
To: Luben Tuikov, Marcelo Tosatti; +Cc: Linux Kernel Mailing List, James Bottomley
Thanks for your attention.
I think It only matters when there're many scsi controllers but less disks like one disk per controller.
Even in such case
The main user of nr_real does
genhd.c:part_show()
/* show the full disk and all non-0 size partitions of it */
for (n = 0; n < (gp->nr_real << gp->minor_shift); n++) {
if (gp->part[n].nr_sects) {
It's very tight loop when there's no real device.
If "(gp->nr_real << gp->minor_shift)" part is taken out from the loop
it'll be much more efficient.
And much simpler proof of "It's safe" is that
we are already living with monsters.
./drivers/md/md.c: nr_real: MAX_MD_DEVS, == 256
./drivers/md/lvm.c: .nr_real = MAX_LV, == 256
So using md or lvm simply adds overhead of 16 scsi controllers.
So we are safe or we are not safe already.
Soohoon Lee.
---------- Original Message ----------------------------------
From: Marcelo Tosatti <marcelo.tosatti@cyclades.com>
Date: Sat, 26 Feb 2005 10:48:52 -0300
>On Wed, Feb 16, 2005 at 11:23:53AM -0500, Luben Tuikov wrote:
>> Hi,
>>
>> This patch fixes the nr_real count in sd.c, which is also used
>> in genhd.c to print out the partitions/units. The problem is that
>> nr_real is decremented on detach, the genhd's nr_sects is
>> cleared but the entry is still there and is being counted
>> for when displaying the partitions. Thus when nr_real
>> is decremented _and_ a 0-ed partition/unit is counted,
>> we get to not display 1 or more entries of the tail of
>> the list.
>>
>> The solution is to not decrement nr_real on detach, effectively
>> never decrementing it, and so that it doesn't grow without a bound,
>> to throttle it on attach, incrementing it only if it would be
>> smaller than nr_dev.
>>
>> This was observed on a RH kernel and on the current BK kernel.
>> Tested and fixed on 2.4.30-pre1 (BK). This patch is against 2.4.30-pre1.
>>
>> To reproduce: assume 4 scsi disks sda, sdb, sdc, sdd.
>> #echo "scsi remove-single-device <sdb-HCTL>" > /proc/scsi/scsi
>> #cat /proc/partitions
>> <<sdb _and_ sdd are not listed>>
>
>Luben,
>
>On James Bottomley advice I have applied Soo Lee's fix, which looks cleaner.
>
>Also as James notice this will increase overhead of /proc/partitions which might be
>a problem on higher-end systems with many devices.
>
>Testing of it on such systems is highly appreciated.
>
>
># 05/02/26 slee@netengine1.com 1.1558
># [PATCH] Fix units/partition count in sd.c
>#
># Symptom:
># When a scsi disk is removed other scsi disk with biggest minor #
># disapears in /proc/partition at the same time.
>#
># Cause and fix:
># sd.c decreases nr_real on disk removal but because nr_real is not
># real # of devices but max # of devices of a major #,
># it doesn't need to be changed on disk add/remove.
>#
># 2.6 has little different structure but it does like this
>#
># sd.c:sd_probe()
># gd->minors = 16;
># --------------------------------------------
>#
>diff -Nru a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>--- a/drivers/scsi/sd.c Sat Feb 26 10:46:42 2005
>+++ b/drivers/scsi/sd.c Sat Feb 26 10:46:42 2005
>@@ -1220,7 +1220,7 @@
> goto cleanup_gendisks_part;
> memset(sd_gendisks[i].part, 0, (SCSI_DISKS_PER_MAJOR << 4) * sizeof(struct hd_struct));
> sd_gendisks[i].sizes = sd_sizes + (i * SCSI_DISKS_PER_MAJOR << 4);
>- sd_gendisks[i].nr_real = 0;
>+ sd_gendisks[i].nr_real = SCSI_DISKS_PER_MAJOR;
> sd_gendisks[i].real_devices =
> (void *) (rscsi_disks + i * SCSI_DISKS_PER_MAJOR);
> }
>@@ -1333,7 +1333,6 @@
> rscsi_disks[i].device = SDp;
> rscsi_disks[i].has_part_table = 0;
> sd_template.nr_dev++;
>- SD_GENDISK(i).nr_real++;
> devnum = i % SCSI_DISKS_PER_MAJOR;
> SD_GENDISK(i).de_arr[devnum] = SDp->de;
> if (SDp->removable)
>@@ -1447,7 +1446,6 @@
> SDp->attached--;
> sd_template.dev_noticed--;
> sd_template.nr_dev--;
>- SD_GENDISK(i).nr_real--;
> return;
> }
> return;
>
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fix units/partition count in sd.c (2.4.x)
2005-02-16 16:23 Luben Tuikov
@ 2005-02-26 13:48 ` Marcelo Tosatti
0 siblings, 0 replies; 4+ messages in thread
From: Marcelo Tosatti @ 2005-02-26 13:48 UTC (permalink / raw)
To: Luben Tuikov, slee; +Cc: Linux Kernel Mailing List, James Bottomley
On Wed, Feb 16, 2005 at 11:23:53AM -0500, Luben Tuikov wrote:
> Hi,
>
> This patch fixes the nr_real count in sd.c, which is also used
> in genhd.c to print out the partitions/units. The problem is that
> nr_real is decremented on detach, the genhd's nr_sects is
> cleared but the entry is still there and is being counted
> for when displaying the partitions. Thus when nr_real
> is decremented _and_ a 0-ed partition/unit is counted,
> we get to not display 1 or more entries of the tail of
> the list.
>
> The solution is to not decrement nr_real on detach, effectively
> never decrementing it, and so that it doesn't grow without a bound,
> to throttle it on attach, incrementing it only if it would be
> smaller than nr_dev.
>
> This was observed on a RH kernel and on the current BK kernel.
> Tested and fixed on 2.4.30-pre1 (BK). This patch is against 2.4.30-pre1.
>
> To reproduce: assume 4 scsi disks sda, sdb, sdc, sdd.
> #echo "scsi remove-single-device <sdb-HCTL>" > /proc/scsi/scsi
> #cat /proc/partitions
> <<sdb _and_ sdd are not listed>>
Luben,
On James Bottomley advice I have applied Soo Lee's fix, which looks cleaner.
Also as James notice this will increase overhead of /proc/partitions which might be
a problem on higher-end systems with many devices.
Testing of it on such systems is highly appreciated.
# 05/02/26 slee@netengine1.com 1.1558
# [PATCH] Fix units/partition count in sd.c
#
# Symptom:
# When a scsi disk is removed other scsi disk with biggest minor #
# disapears in /proc/partition at the same time.
#
# Cause and fix:
# sd.c decreases nr_real on disk removal but because nr_real is not
# real # of devices but max # of devices of a major #,
# it doesn't need to be changed on disk add/remove.
#
# 2.6 has little different structure but it does like this
#
# sd.c:sd_probe()
# gd->minors = 16;
# --------------------------------------------
#
diff -Nru a/drivers/scsi/sd.c b/drivers/scsi/sd.c
--- a/drivers/scsi/sd.c Sat Feb 26 10:46:42 2005
+++ b/drivers/scsi/sd.c Sat Feb 26 10:46:42 2005
@@ -1220,7 +1220,7 @@
goto cleanup_gendisks_part;
memset(sd_gendisks[i].part, 0, (SCSI_DISKS_PER_MAJOR << 4) * sizeof(struct hd_struct));
sd_gendisks[i].sizes = sd_sizes + (i * SCSI_DISKS_PER_MAJOR << 4);
- sd_gendisks[i].nr_real = 0;
+ sd_gendisks[i].nr_real = SCSI_DISKS_PER_MAJOR;
sd_gendisks[i].real_devices =
(void *) (rscsi_disks + i * SCSI_DISKS_PER_MAJOR);
}
@@ -1333,7 +1333,6 @@
rscsi_disks[i].device = SDp;
rscsi_disks[i].has_part_table = 0;
sd_template.nr_dev++;
- SD_GENDISK(i).nr_real++;
devnum = i % SCSI_DISKS_PER_MAJOR;
SD_GENDISK(i).de_arr[devnum] = SDp->de;
if (SDp->removable)
@@ -1447,7 +1446,6 @@
SDp->attached--;
sd_template.dev_noticed--;
sd_template.nr_dev--;
- SD_GENDISK(i).nr_real--;
return;
}
return;
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] fix units/partition count in sd.c (2.4.x)
@ 2005-02-16 16:23 Luben Tuikov
2005-02-26 13:48 ` Marcelo Tosatti
0 siblings, 1 reply; 4+ messages in thread
From: Luben Tuikov @ 2005-02-16 16:23 UTC (permalink / raw)
To: Linux Kernel Mailing List, Marcelo Tosatti
[-- Attachment #1: Type: text/plain, Size: 2706 bytes --]
Hi,
This patch fixes the nr_real count in sd.c, which is also used
in genhd.c to print out the partitions/units. The problem is that
nr_real is decremented on detach, the genhd's nr_sects is
cleared but the entry is still there and is being counted
for when displaying the partitions. Thus when nr_real
is decremented _and_ a 0-ed partition/unit is counted,
we get to not display 1 or more entries of the tail of
the list.
The solution is to not decrement nr_real on detach, effectively
never decrementing it, and so that it doesn't grow without a bound,
to throttle it on attach, incrementing it only if it would be
smaller than nr_dev.
This was observed on a RH kernel and on the current BK kernel.
Tested and fixed on 2.4.30-pre1 (BK). This patch is against 2.4.30-pre1.
To reproduce: assume 4 scsi disks sda, sdb, sdc, sdd.
#echo "scsi remove-single-device <sdb-HCTL>" > /proc/scsi/scsi
#cat /proc/partitions
<<sdb _and_ sdd are not listed>>
Signed-off-by: Luben Tuikov <luben_tuikov@adaptec.com>
===== sd.c 1.31 vs edited =====
--- 1.31/drivers/scsi/sd.c 2003-06-25 19:34:08 -04:00
+++ edited/sd.c 2005-02-14 17:09:43 -05:00
@@ -1332,8 +1332,8 @@
rscsi_disks[i].device = SDp;
rscsi_disks[i].has_part_table = 0;
- sd_template.nr_dev++;
- SD_GENDISK(i).nr_real++;
+ if (sd_template.nr_dev++ >= SD_GENDISK(i).nr_real)
+ SD_GENDISK(i).nr_real++;
devnum = i % SCSI_DISKS_PER_MAJOR;
SD_GENDISK(i).de_arr[devnum] = SDp->de;
if (SDp->removable)
@@ -1424,9 +1424,12 @@
for (dpnt = rscsi_disks, i = 0; i < sd_template.dev_max; i++, dpnt++)
if (dpnt->device == SDp) {
+ char nbuff[6];
/* If we are disconnecting a disk driver, sync and invalidate
* everything */
+ sd_devname(i, nbuff);
+
sdgd = &SD_GENDISK(i);
max_p = sd_gendisk.max_p;
start = i << sd_gendisk.minor_shift;
@@ -1447,7 +1450,12 @@
SDp->attached--;
sd_template.dev_noticed--;
sd_template.nr_dev--;
- SD_GENDISK(i).nr_real--;
+
+ printk("Detached scsi %sdisk %s at scsi%d, "
+ "channel %d, id %d, lun %d\n",
+ SDp->removable ? "removable " : "",
+ nbuff, SDp->host->host_no, SDp->channel,
+ SDp->id, SDp->lun);
return;
}
return;
Luben
P.S. Patch attached as well, for formatting.
[-- Attachment #2: sd.patch --]
[-- Type: octet/stream, Size: 1276 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2005-02-26 19:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-02-16 22:42 [PATCH] fix units/partition count in sd.c (2.4.x) soohoon.lee
-- strict thread matches above, loose matches on Subject: below --
2005-02-26 19:29 Soo Lee
2005-02-16 16:23 Luben Tuikov
2005-02-26 13:48 ` Marcelo Tosatti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).