Re: [2.6 patch] scsi/qla2xxx/qla_os.c section fix
Adrian Bunk wrote: > qla2x00_remove_one() mustn't be __devexit since it's called from > qla2xxx_pci_error_detected(). > > This patch fixes the following section mismatch: > > <-- snip --> > > ... > WARNING: vmlinux.o(.text+0x2a4462): Section mismatch: reference to > .exit.text:qla2x00_remove_one (between 'qla2xxx_pci_error_detected' and > 'qla2x00_stop_timer') > ... > > <-- snip --> > > Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> ACK. Acked-by: Seokmann Ju <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: aic94xx or libsas crash on X7DB3 supermicro with enclosure and sata drives
On Friday 30 November 2007 22:33, Darrick J. Wong wrote: > On Fri, Nov 30, 2007 at 10:22:07AM +0100, Krzysztof B??aszkowski wrote: > > Hello all, > > > > I noticed this according to syslog. furthermore if aic94xx is connected > > to single sata drive only then there is no crash but device is not > > recognized too. (mysterious: "ERROR: Unidentified device type 5"). > > There's been a substantial amount of bugfixes (as well as SATA support) > that went into the aic94xx/libsas code between .22 and .23; could you > please give that a try? thank you. I've tried 2.6.23.9 and it seems to work okay and indeed there were made many changes some of them by you. > > Also, what kind of devices are attached when the system crashes? From > that stack trace it looks like the software thought there was a SATA > disk attached to an expander...? yes, i connected aic to the expander (LSISASX28) which was loaded with 16 drives. Best regards, Krzysztof > > --D - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: aic94xx or libsas crash on X7DB3 supermicro with enclosure and sata drives
I noticed also another failure when i removed a drive. The event was not
notified by anything (ie the block device and corresponding sg were
registered) so i run dd on this truly "virtual" drive.
dd reached D state (as well as scsi_wq) . i think it shouldn't happen no
matter it was AIC failure or LSI expander failure.
>
> --D
Regards,
Krzysztof
ata26.00: ATA-6: ST3120026AS, 3.18, max UDMA/133
ata26.00: 234441648 sectors, multi 0: LBA48
ata26.00: ata_hpa_resize 1: hpa sectors (1) is smaller than sectors (234441648)
ata26.00: configured for UDMA/133
scsi 6:0:20:0: Direct-Access ATA ST3120026AS 3.18 PQ: 0 ANSI: 5
sd 6:0:20:0: [sdb] 234441648 512-byte hardware sectors (120034 MB)
sd 6:0:20:0: [sdb] Write Protect is off
sd 6:0:20:0: [sdb] Mode Sense: 00 3a 00 00
sd 6:0:20:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 6:0:20:0: [sdb] 234441648 512-byte hardware sectors (120034 MB)
sd 6:0:20:0: [sdb] Write Protect is off
sd 6:0:20:0: [sdb] Mode Sense: 00 3a 00 00
sd 6:0:20:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sdb: unknown partition table
sd 6:0:20:0: [sdb] Attached SCSI disk
sd 6:0:20:0: Attached scsi generic sg1 type 0
sd 6:0:20:0: [sdb] Synchronizing SCSI cache
ata26: translated ATA stat/err 0x01/04 to SCSI SK/ASC/ASCQ 0xb/00/00
ata26: status=0x01 { Error }
ata26: error=0x04 { DriveStatusError }
sd 6:0:20:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_TIMEOUT,SUGGEST_OK
sd 6:0:19:0: [sda] Synchronizing SCSI cache
SysRq : Show Blocked State
taskPC stack pid father
scsi_wq_6 D 40246817 0 3727 2
f3d7dc64 0046 f72d5550 40246817 0006 40128e47 f618b468 f73deac0
42748e00 f72d5698 f72d5550 f3d7dd30 f3d7dd34 f3d7dc80 f3d7dcb8 40402896
f72d5550 4011b8a0 4010fa7d f618b3fc f76d8070
Call Trace:
[<40246817>] elv_next_request+0xb7/0x210
[<40128e47>] lock_timer_base+0x27/0x60
[<40402896>] wait_for_completion+0x86/0xc0
[<4011b8a0>] default_wake_function+0x0/0x10
[<4010fa7d>] native_smp_send_reschedule+0x1d/0x30
[<4011b8a0>] default_wake_function+0x0/0x10
[<4024a511>] blk_execute_rq+0xa1/0xe0
[<4024a770>] blk_end_sync_rq+0x0/0x30
[<4013426b>] autoremove_wake_function+0x1b/0x50
[<4011b8e7>] __wake_up_common+0x37/0x70
[<403067a3>] scsi_execute+0xe3/0x110
[<40306845>] scsi_execute_req+0x75/0xb0
[<4031a860>] sd_sync_cache+0x70/0xb0
[<40258ccf>] kobject_get+0xf/0x20
[<4031ce34>] sd_shutdown+0x64/0x140
[<4031cbe2>] sd_remove+0x32/0x70
[<402e15c4>] __device_release_driver+0x94/0xb0
[<402e15fe>] device_release_driver+0x1e/0x40
[<402e0869>] bus_remove_device+0x59/0x80
[<402dee33>] device_del+0x53/0x2c0
[<4030bed1>] __scsi_remove_device+0x51/0x90
[<4030bf2f>] scsi_remove_device+0x1f/0x30
[<4030bfcf>] __scsi_remove_target+0x8f/0xc0
[<4030c000>] __remove_child+0x0/0x20
[<4030c018>] __remove_child+0x18/0x20
[<402df0f2>] device_for_each_child+0x22/0x40
[<4030c05e>] scsi_remove_target+0x3e/0x50
[] sas_rphy_remove+0x58/0x80 [scsi_transport_sas]
[] sas_rphy_delete+0x8/0x10 [scsi_transport_sas]
[] sas_unregister_dev+0x8e/0xa0 [libsas]
[] sas_unregister_devs_sas_addr+0x11f/0x130 [libsas]
[] sas_rediscover_dev+0x116/0x150 [libsas]
[] sas_rediscover+0xb2/0xe0 [libsas]
[] sas_revalidate_domain+0x0/0x50 [libsas]
[] sas_ex_revalidate_domain+0x31/0x70 [libsas]
[<40130511>] run_workqueue+0x71/0x100
[<4013061f>] worker_thread+0x7f/0xd0
[<40134250>] autoremove_wake_function+0x0/0x50
[<4040254a>] schedule+0x21a/0x4e0
[<40134250>] autoremove_wake_function+0x0/0x50
[<401305a0>] worker_thread+0x0/0xd0
[<40133ca4>] kthread+0x64/0xa0
[<40133c40>] kthread+0x0/0xa0
[<40104887>] kernel_thread_helper+0x7/0x10
===
ddD 40249148 0 18935 16194
f1fc7d88 0086 f7d41aa0 40249148 4237b300 f3c52900
4273fe00 f7d41be8 f7d41aa0 4273fe00 f1fc7de4 42708a64 f1fc7d94 40402eed
f1fc7ddc 401517c5 4040318f 40151780 401342a0 f1fc7ddc f1fc7dd8
Call Trace:
[<40249148>] blk_backing_dev_unplug+0x48/0xa0
[<40402eed>] io_schedule+0x1d/0x30
[<401517c5>] sync_page+0x45/0x50
[<4040318f>] __wait_on_bit_lock+0x3f/0x70
[<40151780>] sync_page+0x0/0x50
[<401342a0>] wake_bit_function+0x0/0x60
[<401520ca>] __lock_page+0x9a/0xb0
[<401342a0>] wake_bit_function+0x0/0x60
[<401342a0>] wake_bit_function+0x0/0x60
[<4015279e>] do_generic_mapping_read+0x22e/0x4b0
[<40152da0>] generic_file_aio_read+0x1c0/0x1f0
[<40152a20>] file_read_actor+0x0/0x110
[<40172d6d>] do_sync_read+0xbd/0x110
[<40134250>] autoremove_wake_function+0x0/0x50
[<40116455>] do_page_fault+0x1b5/0x630
[<4012cd5f>] sys_rt_sigaction+0x5f/0xb0
[<40172e83>] vfs_read+0xc3/0x150
[<401731c1>] sys_read+0x41/0x70
[<40103c36>] sysenter_past_esp+0x5f/0x85
[<4040>] clip_setup+0x20/0x50
===
Re: [PATCH 26/28] blk_end_request: changing ide-cd (take 3)
Bartlomiej Zolnierkiewicz wrote: [PATCH] ide-cd: remove dead post_transform_command() post_transform_command() call in cdrom_newpc_intr() has no effect because it is done after the request has already been fully completed (rq->bio and rq->data are always NULL). It was verified to be true regardless whether INQUIRY command is using DMA or PIO to transfer data (by using modified Tejun Heo's test-shortsg.c utility and adding a few printk()-s to ide-cd). This was uncovered thanks to the "blk_end_request: full I/O completion handler (take 3)" patch series from Kiyoshi Ueda. Cc: [EMAIL PROTECTED] Cc: [EMAIL PROTECTED] Cc: Kiyoshi Ueda <[EMAIL PROTECTED] Cc: Jun'ichi Nomura <[EMAIL PROTECTED]> Cc: Tejun Heo <[EMAIL PROTECTED]> Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> Acked-by: Sergei Shtylyov <[EMAIL PROTECTED]> MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: aic94xx or libsas crash on X7DB3 supermicro with enclosure and sata drives
On Mon, Dec 03, 2007 at 05:09:54PM +0100, Krzysztof B??aszkowski wrote: > > I noticed also another failure when i removed a drive. The event was not > notified by anything (ie the block device and corresponding sg were > registered) so i run dd on this truly "virtual" drive. > > dd reached D state (as well as scsi_wq) . i think it shouldn't happen no > matter it was AIC failure or LSI expander failure. "It's wireless!" ;) Seriously, though, it's a good idea to tell the kernel that you're about to unplug a disk before actually doing it: echo 1 > /sys/block/sdX/device/delete This way, the kernel can tell the disk to flush its caches long before power actually gets removed. Otherwise, the device removal code can get hung up just like you observed, and whatever's in the write cache may or may not actually get written to the media. --D - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: aic94xx or libsas crash on X7DB3 supermicro with enclosure and sata drives
Darrick J. Wong wrote: On Mon, Dec 03, 2007 at 05:09:54PM +0100, Krzysztof B??aszkowski wrote: I noticed also another failure when i removed a drive. The event was not notified by anything (ie the block device and corresponding sg were registered) so i run dd on this truly "virtual" drive. dd reached D state (as well as scsi_wq) . i think it shouldn't happen no matter it was AIC failure or LSI expander failure. "It's wireless!" ;) Seriously, though, it's a good idea to tell the kernel that you're about to unplug a disk before actually doing it: echo 1 > /sys/block/sdX/device/delete This way, the kernel can tell the disk to flush its caches long before power actually gets removed. Otherwise, the device removal code can get hung up just like you observed, and whatever's in the write cache may or may not actually get written to the media. What you say is quite true about write cache -- you can clearly lose some data by hot-unplugging a device. And there's nothing we can do about that. But what do you mean by "device removal code can get hung up"? That sounds like a bug we should fix. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: aic94xx or libsas crash on X7DB3 supermicro with enclosure and sata drives
Hi Darrick, On Monday 03 December 2007 20:36, Darrick J. Wong wrote: > On Mon, Dec 03, 2007 at 05:09:54PM +0100, Krzysztof B??aszkowski wrote: > > I noticed also another failure when i removed a drive. The event was not > > notified by anything (ie the block device and corresponding sg were > > registered) so i run dd on this truly "virtual" drive. > > > > dd reached D state (as well as scsi_wq) . i think it shouldn't happen no > > matter it was AIC failure or LSI expander failure. > > "It's wireless!" ;) yep :) and energy from positive thinking spins disk's plates ;) > > Seriously, though, it's a good idea to tell the kernel that you're > about to unplug a disk before actually doing it: > > echo 1 > /sys/block/sdX/device/delete > > This way, the kernel can tell the disk to flush its caches long before > power actually gets removed. Otherwise, the device removal code can > get hung up just like you observed, and whatever's in the write cache > may or may not actually get written to the media. > imagine just raining Monday and someone who put hand on the drive thus he had to reboot whole box. Thanks, Krzysztof > --D - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] 2.6.24-rc3-git2 softlockup detected
On Fri, 30 Nov 2007 12:58:06 +0530
Kamalesh Babulal <[EMAIL PROTECTED]> wrote:
> Andrew Morton wrote:
> > On Thu, 29 Nov 2007 23:00:47 -0800 Andrew Morton <[EMAIL PROTECTED]> wrote:
> >
> >> On Fri, 30 Nov 2007 01:39:29 -0500 Kyle McMartin <[EMAIL PROTECTED]> wrote:
> >>
> >>> On Thu, Nov 29, 2007 at 12:35:33AM -0800, Andrew Morton wrote:
> ten million is close enough to infinity for me to assume that we broke
> the
> driver and that's never going to terminate.
>
> >>> how about this? doesn't break things on my pa8800:
> >>>
> >>> diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c
> >>> b/drivers/scsi/sym53c8xx_2/sym_hipd.c
> >>> index 463f119..ef01cb1 100644
> >>> --- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
> >>> +++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
> >>> @@ -1037,10 +1037,13 @@ restart_test:
> >>> /*
> >>>* Wait 'til done (with timeout)
> >>>*/
> >>> - for (i=0; i >>> + do {
> >>> if (INB(np, nc_istat) & (INTF|SIP|DIP))
> >>> break;
> >>> - if (i>=SYM_SNOOP_TIMEOUT) {
> >>> + msleep(10);
> >>> + } while (i++ < SYM_SNOOP_TIMEOUT);
> >>> +
> >>> + if (i >= SYM_SNOOP_TIMEOUT) {
> >>> printf ("CACHE TEST FAILED: timeout.\n");
> >>> return (0x20);
> >>> }
> >>> diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.h
> >>> b/drivers/scsi/sym53c8xx_2/sym_hipd.h
> >>> index ad07880..85c483b 100644
> >>> --- a/drivers/scsi/sym53c8xx_2/sym_hipd.h
> >>> +++ b/drivers/scsi/sym53c8xx_2/sym_hipd.h
> >>> @@ -339,7 +339,7 @@
> >>> /*
> >>> * Misc.
> >>> */
> >>> -#define SYM_SNOOP_TIMEOUT (1000)
> >>> +#define SYM_SNOOP_TIMEOUT (1000)
> >>> #define BUS_8_BIT0
> >>> #define BUS_16_BIT 1
> >>>
> >> That might be the fix, but do we know what we're actually fixing? afaik
> >> 2.6.24-rc3 doesn't get this timeout, 2.6.24-rc3-mm2 does get it and we
> >> don't know why?
> >>
> >
> >
> >
> >
> >
> > So 2.6.24-rc3 was OK and 2.6.24-rc3-git2 is not?
>
> Yes, the 2.6.24-rc3 was Ok and this is seen from 2.6.24-rc3-git2/3/4.
>
There are effectively no drivers/scsi/ changes after 2.6.24-rc3 and we
don't (I believe) have a clue what caused this regression.
Can you please do a bisection search on this?
Thanks.
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: aic94xx or libsas crash on X7DB3 supermicro with enclosure and sata drives
On Mon, Dec 03, 2007 at 02:43:09PM -0500, Jeff Garzik wrote: > But what do you mean by "device removal code can get hung up"? That sounds > like a bug we should fix. At the moment, libsas' sas_rphy_remove function doesn't distinguish between removing a device before or after the disk has been disconnected. Hence, sd_shutdown tries to tell the disk to flush the write cache, even in the case that the disk is already gone. Maybe the solution is to modify aic94xx to remove the device's DDB registration prior to sending the "device gone" event to libsas so that all subsequent commands bounce with "no such device" instead of going out to lunch. (I'll look into this later, as I myself am going out to lunch right now.) --D - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 26/28] blk_end_request: changing ide-cd (take 3)
Hi Bartlomiej,
On Sat, 1 Dec 2007 23:42:51 +0100, Bartlomiej Zolnierkiewicz <[EMAIL
PROTECTED]> wrote:
> On Saturday 01 December 2007, Kiyoshi Ueda wrote:
> > This patch converts ide-cd (cdrom_newpc_intr()) to use blk_end_request().
> >
> > ide-cd (cdrom_newpc_intr()) has some tricky behaviors below which
> > need to use blk_end_request_callback().
> > Needs to:
> > 1. call post_transform_command() to modify request contents
>
> Seems like post_transform_command() call can be removed (patch below).
>
> > 2. wait completing request until DRQ_STAT is cleared
>
> Would be great if somebody convert cdrom_newpc_intr() to use scatterlists
> also for PIO transfers (ide_pio_sector() in ide-taskfile.c should serve
> as a good starting base to see how to do PIO transfers using scatterlists)
> so we could get rid of partial request completions in cdrom_newpc_intr()
> and just fully complete request when the transfer is done. Shouldn't be
> difficult but I guess that we can live with blk_end_request_callback() for
> the time being...
>
> > after end_that_request_first() and before end_that_request_last().
> >
> > As for the second one, ide-cd will wait for the interrupt from device.
> > So blk_end_request_callback() has to return without completing request
> > even if no leftover in the request.
> > ide-cd uses a dummy callback function, which just returns value '1',
> > to tell blk_end_request_callback() about that.
> >
> > Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]>
> > Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]>
>
> [PATCH] ide-cd: remove dead post_transform_command()
>
> post_transform_command() call in cdrom_newpc_intr() has no effect because
> it is done after the request has already been fully completed (rq->bio and
> rq->data are always NULL). It was verified to be true regardless whether
> INQUIRY command is using DMA or PIO to transfer data (by using modified
> Tejun Heo's test-shortsg.c utility and adding a few printk()-s to ide-cd).
>
> This was uncovered thanks to the "blk_end_request: full I/O completion
> handler (take 3)" patch series from Kiyoshi Ueda.
>
> Cc: [EMAIL PROTECTED]
> Cc: [EMAIL PROTECTED]
> Cc: Kiyoshi Ueda <[EMAIL PROTECTED]
> Cc: Jun'ichi Nomura <[EMAIL PROTECTED]>
> Cc: Tejun Heo <[EMAIL PROTECTED]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]>
> ---
> Kiyoshi: please rebase your patch on top of this one (I'll send
> it to Linus in the next IDE update), should make your patch a bit
> simpler.
>
> Tejun: you had really good timing with posting test-shortsg.c
> (it saved me some time coding user-space SG_IO tester), thanks!
>
> drivers/ide/ide-cd.c | 28
> 1 file changed, 28 deletions(-)
>
> Index: b/drivers/ide/ide-cd.c
> ===
> --- a/drivers/ide/ide-cd.c
> +++ b/drivers/ide/ide-cd.c
> @@ -1650,31 +1650,6 @@ static int cdrom_write_check_ireason(ide
> return 1;
> }
>
> -static void post_transform_command(struct request *req)
> -{
> - u8 *c = req->cmd;
> - char *ibuf;
> -
> - if (!blk_pc_request(req))
> - return;
> -
> - if (req->bio)
> - ibuf = bio_data(req->bio);
> - else
> - ibuf = req->data;
> -
> - if (!ibuf)
> - return;
> -
> - /*
> - * set ansi-revision and response data as atapi
> - */
> - if (c[0] == GPCMD_INQUIRY) {
> - ibuf[2] |= 2;
> - ibuf[3] = (ibuf[3] & 0xf0) | 2;
> - }
> -}
> -
> typedef void (xfer_func_t)(ide_drive_t *, void *, u32);
>
> /*
> @@ -1810,9 +1785,6 @@ static ide_startstop_t cdrom_newpc_intr(
> return ide_started;
>
> end_request:
> - if (!rq->data_len)
> - post_transform_command(rq);
> -
> spin_lock_irqsave(&ide_lock, flags);
> blkdev_dequeue_request(rq);
> end_that_request_last(rq, 1);
Thank you for the comments.
I rebased my patch on top of 2.6.24-rc3-mm2 + the patch to remove
post_transform_command().
As a result, one callback function for DMA mode has been removed.
What do you think about the patch below?
Subject: [PATCH 26/28] blk_end_request: changing ide-cd (take 3)
This patch converts ide-cd (cdrom_newpc_intr()) to use blk_end_request
interfaces.
In PIO mode, ide-cd (cdrom_newpc_intr()) needs to defer
end_that_request_last() until the device clears DRQ_STAT and raises
an interrupt after end_that_request_first().
So blk_end_request() has to return without completing request
even if no leftover in the request.
ide-cd uses blk_end_request_callback() and a dummy callback function,
which just returns value '1', to tell blk_end_request_callback()
about that.
Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]>
Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]>
---
drivers/ide/ide-cd.c | 49 +++--
1 files changed, 35 insertions(+), 14 deletions(-)
Index: 2.6.24-rc3-mm2/drivers/ide/ide-cd.c
===
Re: [PATCH 09/28] blk_end_request: changing ps3disk (take 3)
Hi Geert,
On Sun, 2 Dec 2007 10:34:56 +0100 (CET), Geert Uytterhoeven <[EMAIL PROTECTED]>
wrote:
> On Fri, 30 Nov 2007, Kiyoshi Ueda wrote:
> > This patch converts ps3disk to use blk_end_request().
> ^^^
> Patch subject and description are inconsistent with actual change.
>
> > Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]>
> > Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]>
> > ---
> > drivers/block/ps3disk.c |6 +-
> > 1 files changed, 1 insertion(+), 5 deletions(-)
> >
> > Index: 2.6.24-rc3-mm2/drivers/block/ps3disk.c
> > ===
> > --- 2.6.24-rc3-mm2.orig/drivers/block/ps3disk.c
> > +++ 2.6.24-rc3-mm2/drivers/block/ps3disk.c
> > @@ -280,11 +280,7 @@ static irqreturn_t ps3disk_interrupt(int
> > }
> >
> > spin_lock(&priv->lock);
> > - if (!end_that_request_first(req, uptodate, num_sectors)) {
> > - add_disk_randomness(req->rq_disk);
> > - blkdev_dequeue_request(req);
> > - end_that_request_last(req, uptodate);
> > - }
> > + __blk_end_request(req, uptodate, num_sectors << 9);
> ^
Thank you for the comment.
The description meant the blk_end_request family, not actual function,
blk_end_request(). But as you pointed out, it is misleading.
I'll change the description of all related patches.
Thanks,
Kiyoshi Ueda
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 24/28] blk_end_request: changing ide normal caller (take 3)
Hi Bartlomiej,
On Sat, 1 Dec 2007 23:53:05 +0100, Bartlomiej Zolnierkiewicz <[EMAIL
PROTECTED]> wrote:
> On Saturday 01 December 2007, Kiyoshi Ueda wrote:
> > This patch converts "normal" parts of ide to use blk_end_request().
> >
> > Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]>
> > Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]>
> > ---
> > drivers/ide/ide-cd.c |6 +++---
> > drivers/ide/ide-io.c | 17 ++---
> > 2 files changed, 9 insertions(+), 14 deletions(-)
>
> [...]
>
> > Index: 2.6.24-rc3-mm2/drivers/ide/ide-io.c
> > ===
> > --- 2.6.24-rc3-mm2.orig/drivers/ide/ide-io.c
> > +++ 2.6.24-rc3-mm2/drivers/ide/ide-io.c
> > @@ -78,14 +78,9 @@ static int __ide_end_request(ide_drive_t
> > ide_dma_on(drive);
> > }
> >
> > - if (!end_that_request_chunk(rq, uptodate, nr_bytes)) {
> > - add_disk_randomness(rq->rq_disk);
> > - if (dequeue) {
> > - if (!list_empty(&rq->queuelist))
> > - blkdev_dequeue_request(rq);
> > + if (!__blk_end_request(rq, uptodate, nr_bytes)) {
> > + if (dequeue)
> > HWGROUP(drive)->rq = NULL;
> > - }
> > - end_that_request_last(rq, uptodate);
> > ret = 0;
> > }
>
> Hmmm, this seems to change the old behavior (the request should
> be dequeued from the queue only if 'dequeue' variable is set)
> and AFAIR some error handling code (in ide-cd?) depends on the
> old behavior so please revisit this patch.
blk_end_request() takes care of the dequeue like below,
so I think no problem. (Please see PATCH 01)
> + /* rq->queuelist of dequeued request should be list_empty() */
> + if (!list_empty(&rq->queuelist))
> + blkdev_dequeue_request(rq);
In the case of ide-cd,
o 'dequeue' variable is 1 only when the request is still linked
to the queue (i.e. rq->queuelist is not empty)
o 'dequeue' variable is 0 only when the request has already been
removed from the queue (i.e. rq->queuelist is empty)
So blk_end_request() can handle it correctly.
If there are any drivers which don't want dequeue the queued request,
the code above would not work.
But, as far as I investigated, I have never seen such a requirement
in device drivers.
Do you think that ide may still gets a problem for the 'dequeue'?
Thanks,
Kiyoshi Ueda
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] blk request timeout minor fixes...
Matthew Wilcox [EMAIL PROTECTED] wrote:
> On Sun, Dec 02, 2007 at 05:53:30PM -0800, [EMAIL PROTECTED] wrote:
> Can I suggest running 'pahole' over this when compiled on 64-bit?
> You've just introduced a 4-byte hole.
This one fixes the 4-byte hole. Thank you very much.
diff --git a/block/ll_rw_blk.c b/block/ll_rw_blk.c
index 79ed268..5e95fa8 100644
--- a/block/ll_rw_blk.c
+++ b/block/ll_rw_blk.c
@@ -259,6 +259,7 @@ static void rq_init(struct request_queue *q, struct request
*rq)
INIT_LIST_HEAD(&rq->timeout_list);
rq->timeout = 0;
+ rq->deadline = 0;
rq->errors = 0;
rq->bio = rq->biotail = NULL;
INIT_HLIST_NODE(&rq->hash);
@@ -3707,21 +3708,24 @@ static void blk_rq_timed_out(struct request *req)
static void blk_rq_timed_out_timer(unsigned long data)
{
struct request_queue *q = (struct request_queue *) data;
- unsigned long flags, next = 0;
+ unsigned long flags, uninitialized_var(next), next_set = 0;
struct request *rq, *tmp;
spin_lock_irqsave(q->queue_lock, flags);
list_for_each_entry_safe(rq, tmp, &q->timeout_list, timeout_list) {
- if (!next || time_before(next, rq->timeout))
- next = rq->timeout;
- if (time_after_eq(jiffies, rq->timeout)) {
+ if (time_after_eq(jiffies, rq->deadline)) {
list_del_init(&rq->timeout_list);
blk_rq_timed_out(rq);
}
+ if (!next_set) {
+ next = rq->deadline;
+ next_set = 1;
+ } else if (time_after(next, rq->deadline))
+ next = rq->deadline;
}
- if (next)
+ if (next_set)
mod_timer(&q->timeout, round_jiffies(next));
spin_unlock_irqrestore(q->queue_lock, flags);
@@ -3760,9 +3764,9 @@ void blk_add_timer(struct request *req)
BUG_ON(!list_empty(&req->timeout_list));
if (req->timeout)
- req->timeout += jiffies;
+ req->deadline = jiffies + req->timeout;
else
- req->timeout = jiffies + q->rq_timeout;
+ req->deadline = jiffies + q->rq_timeout;
list_add_tail(&req->timeout_list, &q->timeout_list);
@@ -3771,7 +3775,7 @@ void blk_add_timer(struct request *req)
* than an existing one, modify the timer. Round to next nearest
* second.
*/
- expiry = round_jiffies(req->timeout);
+ expiry = round_jiffies(req->deadline);
if (!timer_pending(&q->timeout) ||
time_before(expiry, q->timeout.expires))
mod_timer(&q->timeout, expiry);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 917fe86..834d097 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -295,8 +295,9 @@ struct request {
void *data;
void *sense;
- unsigned long timeout;
+ unsigned long deadline;
struct list_head timeout_list;
+ unsigned int timeout;
int retries;
/*
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
