Re: [PATCH] REQ-flags to/from BIO-flags bugfix
On Wed, Dec 12, 2007 at 08:54:07AM -0700, Matthew Wilcox wrote:
> On Wed, Dec 12, 2007 at 08:18:14AM -0700, Matthew Wilcox wrote:
> > I don't know whether BIO_RW_BARRIER is __REQ_SOFTBARRIER or
> > __REQ_HARDBARRIER, so I didn't include that in this patch. There also
> > doesn't seem to be a __REQ equivalent to BIO_RW_AHEAD, but we can do
> > the other four bits (and leave gaps for those two).
>
> Hm. BIO_RW_AHEAD seems unused:
>
> [EMAIL PROTECTED]:~/kernel/linux-2.6$ grep -r BIO_RW_AHEAD *
> block/blktrace.c: (((rw) & (1 << BIO_RW_AHEAD)) << (2 - BIO_RW_AHEAD))
> include/linux/bio.h:#define BIO_RW_AHEAD1
> include/linux/bio.h:#define bio_rw_ahead(bio) ((bio)->bi_rw & (1 <<
> BIO_RW_AHEAD))
> [EMAIL PROTECTED]:~/kernel/linux-2.6$ grep -r bio_rw_ahead *
> block/ll_rw_blk.c: if (bio_rw_ahead(bio) || bio_failfast(bio))
> drivers/md/dm-mpath.c: if ((error == -EWOULDBLOCK) && bio_rw_ahead(bio))
> drivers/md/multipath.c: else if (!bio_rw_ahead(bio)) {
> include/linux/bio.h:#define bio_rw_ahead(bio) ((bio)->bi_rw & (1 <<
> BIO_RW_AHEAD))
That would say to me that READA is not hooked up correctly. i.e:
#define READ 0
#define WRITE 1
#define READA 2 /* read-ahead - don't block if no resources */
#define SWRITE 3/* for ll_rw_block() - wait for buffer lock */
#define READ_SYNC (READ | (1 << BIO_RW_SYNC))
#define READ_META (READ | (1 << BIO_RW_META))
#define WRITE_SYNC (WRITE | (1 << BIO_RW_SYNC))
#define WRITE_BARRIER ((1 << BIO_RW) | (1 << BIO_RW_BARRIER))
i.e. it should be:
#define READA (1 << BIO_RW_AHEAD)
Right?
FWIW, dm does this:
if (bio_rw(bio) != READA)
Which really should be if (bio_rw_ahead(bio)).
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
-
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 01/30] blk_end_request: add new request completion interface (take 4)
Hi James, Jens,
On Wed, 12 Dec 2007 07:53:36 -0500, James Bottomley wrote:
> On Tue, 2007-12-11 at 17:40 -0500, Kiyoshi Ueda wrote:
> > This patch adds 2 new interfaces for request completion:
> > o blk_end_request() : called without queue lock
> > o __blk_end_request() : called with queue lock held
> >
> > blk_end_request takes 'error' as an argument instead of 'uptodate',
> > which current end_that_request_* take.
> > The meanings of values are below and the value is used when bio is
> > completed.
> > 0 : success
> > < 0 : error
> >
> > Some device drivers call some generic functions below between
> > end_that_request_{first/chunk} and end_that_request_last().
> > o add_disk_randomness()
> > o blk_queue_end_tag()
> > o blkdev_dequeue_request()
>
> If we can roll the whole thing together, that would be nice. However,
> the way you're doing it with this patch, we now have an asymmetrical
> interface: The request routine must explicitly start the tag, but now
> doesn't have to end it.
>
> We really need symmetry. Either go back to start tag/end tag, or absorb
> the whole lot into the block infrastructure.
>
> The original reason for the explicit start/end is that there are some
> requests on a tagged device that aren't able to be tagged by the block
> layer (some devices reserve tag numbers for specific meanings).
> However, I don't think there's any driver that actually implemented this
> feature.
As far as I investigated in 2.6.24-rc5, only scsi uses the blk_queue_tag
and no files in drivers/scsi reserving tag_index in Scsi_Host->bqt.
So I would like to take the "absorbing start tag in the block layer" way.
The patch below is on top of the blk_end_request patch-set.
Is it acceptable?
Thanks,
Kiyoshi Ueda
Subject: [PATCH 31/31] blk_end_request: merge start_tag to block layer
This patch merges blk_queue_start_tag() into blkdev_dequeue_request().
blk_queue_start_tag() and blk_queue_end_tag() are a pair of
interfaces for starting/ending request tagging.
Since with the new blk_end_request interfaces, blk_queue_end_tag()
is done implicitly in the block layer, blk_queue_start_tag() should
be done in the block layer, too, for keeping the interface symmetric.
Originally, the start/end tag was not done by the block layer so that
drivers can choose tag numbers for their specific meanings.
But no driver uses the feature.
Scsi is the only user of the block layer tagging and it uses
the generic blk_queue_start/end_tag.
So moving the start/end tag to the block layer is not an issue now.
Cc: James Bottomley <[EMAIL PROTECTED]>
Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]>
Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]>
---
block/ll_rw_blk.c |2 +-
drivers/scsi/scsi_lib.c |3 +--
include/linux/blkdev.h | 11 ++-
3 files changed, 8 insertions(+), 8 deletions(-)
Index: 2.6.24-rc5/block/ll_rw_blk.c
===
--- 2.6.24-rc5.orig/block/ll_rw_blk.c
+++ 2.6.24-rc5/block/ll_rw_blk.c
@@ -1115,7 +1115,7 @@ int blk_queue_start_tag(struct request_q
rq->cmd_flags |= REQ_QUEUED;
rq->tag = tag;
bqt->tag_index[tag] = rq;
- blkdev_dequeue_request(rq);
+ elv_dequeue_request(q, rq);
list_add(&rq->queuelist, &q->tag_busy_list);
bqt->busy++;
return 0;
Index: 2.6.24-rc5/drivers/scsi/scsi_lib.c
===
--- 2.6.24-rc5.orig/drivers/scsi/scsi_lib.c
+++ 2.6.24-rc5/drivers/scsi/scsi_lib.c
@@ -1530,8 +1530,7 @@ static void scsi_request_fn(struct reque
/*
* Remove the request from the request list.
*/
- if (!(blk_queue_tagged(q) && !blk_queue_start_tag(q, req)))
- blkdev_dequeue_request(req);
+ blkdev_dequeue_request(req);
sdev->device_busy++;
spin_unlock(q->queue_lock);
Index: 2.6.24-rc5/include/linux/blkdev.h
===
--- 2.6.24-rc5.orig/include/linux/blkdev.h
+++ 2.6.24-rc5/include/linux/blkdev.h
@@ -747,11 +747,6 @@ extern void blk_complete_request(struct
extern unsigned int blk_rq_bytes(struct request *rq);
extern unsigned int blk_rq_cur_bytes(struct request *rq);
-static inline void blkdev_dequeue_request(struct request *req)
-{
- elv_dequeue_request(req->q, req);
-}
-
/*
* Access functions for manipulating queue properties
*/
@@ -814,6 +809,12 @@ static inline struct request *blk_map_qu
return bqt->tag_index[tag];
}
+static inline void blkdev_dequeue_request(struct request *req)
+{
+ if (!(blk_queue_tagged(req->q) && !blk_queue_start_tag(req->q, req)))
+ elv_dequeue_request(req->q, req);
+}
+
extern int blkdev_issue_flush(struct block_device *, sector_t *);
#define MAX_PHYS_SEGMENTS 128
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
Re: [PATCH 19/30] blk_end_request: changing ide-scsi (take 4)
On Tuesday 11 December 2007, Kiyoshi Ueda wrote: > This patch converts ide-scsi to use blk_end_request interfaces. > Related 'uptodate' arguments are converted to 'error'. > > Cc: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> > Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]> > Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]> looks good, ACK ditto for patches #23 and #25 - 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: [stable] broken dpt_i2o in 2.6.23 (was: ext2 check page: bad entry in directory) (fwd)
On Wed, Dec 12, 2007 at 02:54:54PM -0500, James Bottomley wrote: > > On Wed, 2007-12-12 at 11:16 -0800, Andrew Morton wrote: > > On Wed, 12 Dec 2007 14:43:42 +0100 Anders Henke <[EMAIL PROTECTED]> wrote: > > > > > Am 12.12.2007 schrieb Miquel van Smoorenburg: > > > > On Wed, 2007-12-12 at 03:38 -0800, Andrew Morton wrote: > > > > > On Wed, 12 Dec 2007 11:58:41 +0100 Anders Henke <[EMAIL PROTECTED]> > > > > > wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > I'd like to let you now that my boxes are running a 32-bit kernel, > > > > > > so > > > > > > the 64-bit-uncleanliness shouldn't apply to my boxes; however, > > > > > > > > > > > > http://www.miquels.cistron.nl/linux/dpt_i2o-64bit-2.6.23.patch > > > > > > > > > > > > fixed the issue on my testbox. > > > > > > > > > > > > I took a clean 2.6.23, applied patch, recompiled the kernel, > > > > > > reboot: works. > > > > > > > > > > What a huge patch :( > > > > > > > > > > We already reverted the offening patch so I assume that 2.6.24-rc5 is > > > > > working for you? > > > > > > > > > > I guess we need to look at restoring "dpt_i2o: convert to SCSI hotplug > > > > > model" and then absorbing what Miquel has done there. > > > > > > > > This was just a patch I had lying around, if it worked it would confirm > > > > my suspicion, which it has. > > > > > > > > The minimal patch which is suitable for 2.6.23-stable and 2.6.24 would > > > > be the attached one-liner. The "dpt_i2o: convert to SCSI hotplug model" > > > > patch could be restored then. > > > > > > > > (if the list eats the attachment, it's also available here: > > > > http://www.miquels.cistron.nl/linux/linux-2.6.23+24-dpt_i2o-dma64.patch > > > > ) > > > > > > > > Anders, does this one-liner patch work for you ? > > > > > > Got it - and it works! > > > > > > I took a clean 2.6.23, applied the patch, recompiled the kernel and > > > rebooted my testbox: came up with the fresh-compiled kernel > > > (verified by "uname -a"). > > > > > > > That looks appropriate for 2.6.23.x: > > > > --- linux-2.6.23.9.orig/drivers/scsi/dpt_i2o.c 2007-11-26 > > 18:51:43.0 +0100 > > +++ linux-2.6.23.9/drivers/scsi/dpt_i2o.c 2007-12-12 13:21:05.0 > > +0100 > > @@ -905,8 +905,7 @@ > > } > > > > pci_set_master(pDev); > > - if (pci_set_dma_mask(pDev, DMA_64BIT_MASK) && > > - pci_set_dma_mask(pDev, DMA_32BIT_MASK)) > > + if (pci_set_dma_mask(pDev, DMA_32BIT_MASK)) > > return -EINVAL; > > Yes, this has to be in ... the mptr filling the scatterlist on the > current driver is only a u32 and so will silently truncate. > > > base_addr0_phys = pci_resource_start(pDev,0); > > > > > > However it is a bit mystifying that > > 55d9fcf57ba5ec427544fca7abc335cf3da78160 would cause a dma mask problem > > (isn't it?) > > > > The scsi people might want to restore > > 55d9fcf57ba5ec427544fca7abc335cf3da78160 and then apply Miquel's patch on > > top for 2.6.24, or do it for 2.6.25? > > I think it's a bit late in the game for 2.6.24, so I'm happy to leave > the hotplug reverted. We'll try adding back hotplug plus this for > 2.6.25 I think. So, what should be added to 2.6.23-stable then? And, can I get a real changelog entry for it? thanks, greg k-h - 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 05/30] blk_end_request: changing um (take 4)
On Tue, Dec 11, 2007 at 05:42:53PM -0500, Kiyoshi Ueda wrote: > This patch converts um to use blk_end_request interfaces. > Related 'uptodate' arguments are converted to 'error'. > > As a result, the interface of internal function, ubd_end_request(), > is changed. Looks OK to me... Jeff -- Work email - jdike at linux dot intel dot com - 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: scsi/lpfc/lpfc_attr.c: bogus code
This has already been fixed. It's in our 8.2.3 patches, which were merged into
James's scsi-misc-2.6 tree at the beginning of November, and targeted for
2.6.25.
-- james s
Adrian Bunk wrote:
Commit 2e0fef85e098f6794956b8b80b79fbb4cbb7 added the folowing code
to drivers/scsi/lpfc/lpfc_attr.c that was most likely not intended to be
dead code:
<-- snip -->
...
lpfc_state_show(struct class_device *cdev, char *buf)
{
...
switch (vport->port_state) {
len += snprintf(buf + len, PAGE_SIZE-len,
"initializing\n");
break;
...
<-- snip -->
Spotted by the GNU C compiler version 3.3.
cu
Adrian
-
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 12/30] blk_end_request: changing ub (take 4)
Hi Pete,
On Tue, 11 Dec 2007 15:48:03 -0800, Pete Zaitcev <[EMAIL PROTECTED]> wrote:
> > if (scsi_status == 0) {
> > - uptodate = 1;
> > + error = 0;
> > } else {
> > - uptodate = 0;
> > + error = -EIO;
> > rq->errors = scsi_status;
> > }
> > - end_that_request_first(rq, uptodate, rq->hard_nr_sectors);
> > - end_that_request_last(rq, uptodate);
> > + if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
> > + BUG();
>
> Acked-by: Pete Zaitcev <[EMAIL PROTECTED]>
>
> I follow the discussion, actually, and wanted to ask someone to look
> closer if it's appropriate to use __blk_end_request() here.
> My understanding was, blk_end_request() is the same thing, only
> takes the queue lock. But then, should I refactor ub so that it
> calls __blk_end_request if request function ends with an error
> and blk_end_request if the end-of-IO even is processed? If not,
> and the above is sufficient, why have blk_end_request at all?
The difference between blk_end_request() and __blk_end_request() is
whether the queue lock is held or not when end_that_request_last()
is called.
It's not relevant to the status of the request (error or not).
I'm using __blk_end_request() here and I think it's sufficient, because:
o end_that_request_last() must be called with the queue lock held
o ub_end_rq() calls end_that_request_last() without taking
the queue lock in itself.
So the queue lock must have been taken outside ub_end_rq().
But, if ub is calling end_that_request_last() without the queue lock,
it is a bug in the original code and we should use blk_end_request()
to fix that.
Does that answer satisfy you?
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: broken dpt_i2o in 2.6.23 (was: ext2 check page: bad entry in directory) (fwd)
On Wed, 2007-12-12 at 11:16 -0800, Andrew Morton wrote: > On Wed, 12 Dec 2007 14:43:42 +0100 Anders Henke <[EMAIL PROTECTED]> wrote: > > > Am 12.12.2007 schrieb Miquel van Smoorenburg: > > > On Wed, 2007-12-12 at 03:38 -0800, Andrew Morton wrote: > > > > On Wed, 12 Dec 2007 11:58:41 +0100 Anders Henke <[EMAIL PROTECTED]> > > > > wrote: > > > > > > > > > Hi, > > > > > > > > > > I'd like to let you now that my boxes are running a 32-bit kernel, so > > > > > the 64-bit-uncleanliness shouldn't apply to my boxes; however, > > > > > > > > > > http://www.miquels.cistron.nl/linux/dpt_i2o-64bit-2.6.23.patch > > > > > > > > > > fixed the issue on my testbox. > > > > > > > > > > I took a clean 2.6.23, applied patch, recompiled the kernel, reboot: > > > > > works. > > > > > > > > What a huge patch :( > > > > > > > > We already reverted the offening patch so I assume that 2.6.24-rc5 is > > > > working for you? > > > > > > > > I guess we need to look at restoring "dpt_i2o: convert to SCSI hotplug > > > > model" and then absorbing what Miquel has done there. > > > > > > This was just a patch I had lying around, if it worked it would confirm > > > my suspicion, which it has. > > > > > > The minimal patch which is suitable for 2.6.23-stable and 2.6.24 would > > > be the attached one-liner. The "dpt_i2o: convert to SCSI hotplug model" > > > patch could be restored then. > > > > > > (if the list eats the attachment, it's also available here: > > > http://www.miquels.cistron.nl/linux/linux-2.6.23+24-dpt_i2o-dma64.patch > > > ) > > > > > > Anders, does this one-liner patch work for you ? > > > > Got it - and it works! > > > > I took a clean 2.6.23, applied the patch, recompiled the kernel and > > rebooted my testbox: came up with the fresh-compiled kernel > > (verified by "uname -a"). > > > > That looks appropriate for 2.6.23.x: > > --- linux-2.6.23.9.orig/drivers/scsi/dpt_i2o.c2007-11-26 > 18:51:43.0 +0100 > +++ linux-2.6.23.9/drivers/scsi/dpt_i2o.c 2007-12-12 13:21:05.0 > +0100 > @@ -905,8 +905,7 @@ > } > > pci_set_master(pDev); > - if (pci_set_dma_mask(pDev, DMA_64BIT_MASK) && > - pci_set_dma_mask(pDev, DMA_32BIT_MASK)) > + if (pci_set_dma_mask(pDev, DMA_32BIT_MASK)) > return -EINVAL; Yes, this has to be in ... the mptr filling the scatterlist on the current driver is only a u32 and so will silently truncate. > base_addr0_phys = pci_resource_start(pDev,0); > > > However it is a bit mystifying that > 55d9fcf57ba5ec427544fca7abc335cf3da78160 would cause a dma mask problem > (isn't it?) > > The scsi people might want to restore > 55d9fcf57ba5ec427544fca7abc335cf3da78160 and then apply Miquel's patch on > top for 2.6.24, or do it for 2.6.25? I think it's a bit late in the game for 2.6.24, so I'm happy to leave the hotplug reverted. We'll try adding back hotplug plus this for 2.6.25 I think. James - 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: broken dpt_i2o in 2.6.23 (was: ext2 check page: bad entry in directory) (fwd)
On Wed, 12 Dec 2007 14:43:42 +0100 Anders Henke <[EMAIL PROTECTED]> wrote: > Am 12.12.2007 schrieb Miquel van Smoorenburg: > > On Wed, 2007-12-12 at 03:38 -0800, Andrew Morton wrote: > > > On Wed, 12 Dec 2007 11:58:41 +0100 Anders Henke <[EMAIL PROTECTED]> wrote: > > > > > > > Hi, > > > > > > > > I'd like to let you now that my boxes are running a 32-bit kernel, so > > > > the 64-bit-uncleanliness shouldn't apply to my boxes; however, > > > > > > > > http://www.miquels.cistron.nl/linux/dpt_i2o-64bit-2.6.23.patch > > > > > > > > fixed the issue on my testbox. > > > > > > > > I took a clean 2.6.23, applied patch, recompiled the kernel, reboot: > > > > works. > > > > > > What a huge patch :( > > > > > > We already reverted the offening patch so I assume that 2.6.24-rc5 is > > > working for you? > > > > > > I guess we need to look at restoring "dpt_i2o: convert to SCSI hotplug > > > model" and then absorbing what Miquel has done there. > > > > This was just a patch I had lying around, if it worked it would confirm > > my suspicion, which it has. > > > > The minimal patch which is suitable for 2.6.23-stable and 2.6.24 would > > be the attached one-liner. The "dpt_i2o: convert to SCSI hotplug model" > > patch could be restored then. > > > > (if the list eats the attachment, it's also available here: > > http://www.miquels.cistron.nl/linux/linux-2.6.23+24-dpt_i2o-dma64.patch > > ) > > > > Anders, does this one-liner patch work for you ? > > Got it - and it works! > > I took a clean 2.6.23, applied the patch, recompiled the kernel and > rebooted my testbox: came up with the fresh-compiled kernel > (verified by "uname -a"). > That looks appropriate for 2.6.23.x: --- linux-2.6.23.9.orig/drivers/scsi/dpt_i2o.c 2007-11-26 18:51:43.0 +0100 +++ linux-2.6.23.9/drivers/scsi/dpt_i2o.c 2007-12-12 13:21:05.0 +0100 @@ -905,8 +905,7 @@ } pci_set_master(pDev); - if (pci_set_dma_mask(pDev, DMA_64BIT_MASK) && - pci_set_dma_mask(pDev, DMA_32BIT_MASK)) + if (pci_set_dma_mask(pDev, DMA_32BIT_MASK)) return -EINVAL; base_addr0_phys = pci_resource_start(pDev,0); However it is a bit mystifying that 55d9fcf57ba5ec427544fca7abc335cf3da78160 would cause a dma mask problem (isn't it?) The scsi people might want to restore 55d9fcf57ba5ec427544fca7abc335cf3da78160 and then apply Miquel's patch on top for 2.6.24, or do it for 2.6.25? - 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 21/30] blk_end_request: changing cciss (take 4)
> > > > Why is this removed? > > Sorry for the less explanation. > > Because it is done in __end_that_request_first() called from > blk_end_request(). > I'll add the explanation to the patch description when I > update the patch. > Thank you. I've Acked the patch. -- mikem - 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 21/30] blk_end_request: changing cciss (take 4)
> -Original Message-
> From: Kiyoshi Ueda [mailto:[EMAIL PROTECTED]
> Sent: Tuesday, December 11, 2007 4:50 PM
> To: [EMAIL PROTECTED]
> Cc: [EMAIL PROTECTED]; linux-scsi@vger.kernel.org;
> [EMAIL PROTECTED]; [EMAIL PROTECTED];
> [EMAIL PROTECTED]; [EMAIL PROTECTED]; Miller, Mike (OS Dev)
> Subject: [PATCH 21/30] blk_end_request: changing cciss (take 4)
>
> This patch converts cciss to use blk_end_request interfaces.
> Related 'uptodate' arguments are converted to 'error'.
>
> cciss is a little bit different from "normal" drivers.
> cciss directly calls bio_endio() and disk_stat_add() when
> completing request. But those can be replaced with
> __end_that_request_first().
> After the replacement, request completion procedures of those
> drivers become like the following:
> o end_that_request_first()
> o add_disk_randomness()
> o end_that_request_last()
> This can be converted to blk_end_request() by following the
> rule (a) mentioned in the patch subject "[PATCH 01/30]
> blk_end_request: add new request completion interface".
>
> Cc: Mike Miller <[EMAIL PROTECTED]>
> Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]>
> Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]>
Acked-by: Mike Miller <[EMAIL PROTECTED]>
> ---
> drivers/block/cciss.c | 25 +++--
> 1 files changed, 3 insertions(+), 22 deletions(-)
>
> Index: 2.6.24-rc4/drivers/block/cciss.c
> ===
> --- 2.6.24-rc4.orig/drivers/block/cciss.c
> +++ 2.6.24-rc4/drivers/block/cciss.c
> @@ -1187,17 +1187,6 @@ static int cciss_ioctl(struct inode *ino
> }
> }
>
> -static inline void complete_buffers(struct bio *bio, int status) -{
> - while (bio) {
> - struct bio *xbh = bio->bi_next;
> -
> - bio->bi_next = NULL;
> - bio_endio(bio, status ? 0 : -EIO);
> - bio = xbh;
> - }
> -}
> -
> static void cciss_check_queues(ctlr_info_t *h) {
> int start_queue = h->next_to_run; @@ -1263,21
> +1252,14 @@ static void cciss_softirq_done(struct re
> pci_unmap_page(h->pdev, temp64.val,
> cmd->SG[i].Len, ddir);
> }
>
> - complete_buffers(rq->bio, (rq->errors == 0));
> -
> - if (blk_fs_request(rq)) {
> - const int rw = rq_data_dir(rq);
> -
> - disk_stat_add(rq->rq_disk, sectors[rw],
> rq->nr_sectors);
> - }
> -
> #ifdef CCISS_DEBUG
> printk("Done with %p\n", rq);
> #endif /* CCISS_DEBUG */
>
> - add_disk_randomness(rq->rq_disk);
> + if (blk_end_request(rq, (rq->errors == 0) ? 0 : -EIO,
> blk_rq_bytes(rq)))
> + BUG();
> +
> spin_lock_irqsave(&h->lock, flags);
> - end_that_request_last(rq, (rq->errors == 0));
> cmd_free(h, cmd, 1);
> cciss_check_queues(h);
> spin_unlock_irqrestore(&h->lock, flags); @@ -2544,7
> +2526,6 @@ after_error_processing:
> }
> cmd->rq->data_len = 0;
> cmd->rq->completion_data = cmd;
> - blk_add_trace_rq(cmd->rq->q, cmd->rq, BLK_TA_COMPLETE);
> blk_complete_request(cmd->rq);
> }
>
>
-
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 20/30] blk_end_request: changing xsysace (take 4)
On Wed, Dec 12 2007 at 19:06 +0200, Kiyoshi Ueda <[EMAIL PROTECTED]> wrote:
> Hi,
>
> On Wed, 12 Dec 2007 11:09:12 +0200, Boaz Harrosh <[EMAIL PROTECTED]> wrote:
>>> Index: 2.6.24-rc4/drivers/block/xsysace.c
>>> ===
>>> --- 2.6.24-rc4.orig/drivers/block/xsysace.c
>>> +++ 2.6.24-rc4/drivers/block/xsysace.c
>>> @@ -703,7 +703,7 @@ static void ace_fsm_dostate(struct ace_d
>>>
>>> /* bio finished; is there another one? */
>>> i = ace->req->current_nr_sectors;
>>> - if (end_that_request_first(ace->req, 1, i)) {
>>> + if (__blk_end_request(ace->req, 0, i)) {
>> end_that_request_first() took sectors __blk_end_request() now takes
>> bytes
>
> Thank you for pointing it out! And I'm very sorry for the bug.
> I have checked all conversions between sectors and bytes through
> all patches again, and I found no other miss conversions.
>
> Below is the revised patch for xsysace.
>
> Thanks,
> Kiyoshi Ueda
>
>
NP, I know how it is, after you rebased your work for the
"who's counting" times, you become blind. You need fresh
eyes to look at it. Thanks for doing all this.
Boaz
-
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] scsi device recovery
[Hmm, resending since mail after more than 30min still not on the ML, maybe the attachment was too large? I have uploaded the log to http://www.pci.uni-heidelberg.de/tc/usr/bernd/downloads/scsi/kern.log.1] On Wednesday 12 December 2007 16:59:36 James Bottomley wrote: > On Wed, 2007-12-12 at 15:36 +0100, Bernd Schubert wrote: > > On Wednesday 12 December 2007 14:39:27 Matthew Wilcox wrote: > > > On Wed, Dec 12, 2007 at 01:54:14PM +0100, Bernd Schubert wrote: > > > > below is a patch introducing device recovery, trying to prevent i/o > > > > errors when a DID_NO_CONNECT or SOFT_ERROR does happen. > > > > > > Why doesn't the regular scsi_eh do what you need? > > > > First of all, it is presently simply not called when the two errors above > > do happen. This could be changed, of course. > > Erm, I think you'll find the error handler does activate on > DID_SOFT_ERROR. It causes a retry via the eh. DID_NO_CONNECT is an Dec 7 23:48:45 beo-96 kernel: [94605.297924] sd 2:0:5:0: [sdd] Result: hostbyte=DID_SOFT_ERROR driverbyte=DRIVER_OK,SUGGEST_OK Dec 7 23:48:45 beo-96 kernel: [94605.297932] end_request: I/O error, dev sdd, sector 7706802052 Dec 7 23:48:45 beo-96 kernel: [94605.297937] raid5:md5: read error not correctable (sector 871932472 on sdd3). Full log attached. > immediate error with no eh intervention because it means that the target > went away. Handling this as a retryable error isn't an option because > it will interfere with hotplug. Then we need a sysfs flag one can set to manually enable eh for these devices on DID_NO_CONNECT. > > > Secondly, I think scsi_eh is in most cases doing too much. We are > > fighting with flaky Infortrend boxes here, and scsi_eh sometimes manages > > to crash their scsi channels. In most cases it is sufficient to stall any > > io to the device and then to resume. > > But that's basically the default behaviour of the error handler (stall > then resume). > > > For most scsi devices one probably doesn't need a suspend time or it can > > be very small, this still needs to become configurable via sysfs. > > You mean a wait time beyond what the error handler currently does > (basically it waits for the quiesce, begins error handling and then > sends a test unit ready when it finishes before restarting). In deh just waits on the first error and then only does a DV. For these infortrend devices, thats mostly sufficient. > > > Thirdly, scsi_eh doesn't give up, in most cases, when the scsi channel of > > a Infortrend box crashed, it tried forever to recover. > > To improve this is still on my todo list. > > Could you send traces for this. I thought the error handler had been > fixed over the last few years always to terminate. If there's a case > where it doesn't, this needs fixing. I'm attaching the syslog, this is 2.6.22 + additional printks, dump_stack()'s and msleep()'s. At 03:59:36 the system finally went into wait_for_completion(), similar to the "everything in wait_for_completion, what is my system doing?" thread. Thanks, Bernd -- Bernd Schubert Q-Leap Networks GmbH - 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 21/30] blk_end_request: changing cciss (take 4)
Hi Mike, On Wed, 12 Dec 2007 15:25:10 +, "Miller, Mike (OS Dev)" <[EMAIL PROTECTED]> wrote: > > Index: 2.6.24-rc4/drivers/block/cciss.c > > === > > --- 2.6.24-rc4.orig/drivers/block/cciss.c > > +++ 2.6.24-rc4/drivers/block/cciss.c snip > > +2526,6 @@ after_error_processing: > > } > > cmd->rq->data_len = 0; > > cmd->rq->completion_data = cmd; > > - blk_add_trace_rq(cmd->rq->q, cmd->rq, BLK_TA_COMPLETE); > > Why is this removed? Sorry for the less explanation. Because it is done in __end_that_request_first() called from blk_end_request(). I'll add the explanation to the patch description when I update the patch. 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 20/30] blk_end_request: changing xsysace (take 4)
Hi,
On Wed, 12 Dec 2007 11:09:12 +0200, Boaz Harrosh <[EMAIL PROTECTED]> wrote:
> > Index: 2.6.24-rc4/drivers/block/xsysace.c
> > ===
> > --- 2.6.24-rc4.orig/drivers/block/xsysace.c
> > +++ 2.6.24-rc4/drivers/block/xsysace.c
> > @@ -703,7 +703,7 @@ static void ace_fsm_dostate(struct ace_d
> >
> > /* bio finished; is there another one? */
> > i = ace->req->current_nr_sectors;
> > - if (end_that_request_first(ace->req, 1, i)) {
> > + if (__blk_end_request(ace->req, 0, i)) {
>
> end_that_request_first() took sectors __blk_end_request() now takes
> bytes
Thank you for pointing it out! And I'm very sorry for the bug.
I have checked all conversions between sectors and bytes through
all patches again, and I found no other miss conversions.
Below is the revised patch for xsysace.
Thanks,
Kiyoshi Ueda
Subject: [PATCH 20/30] blk_end_request: changing xsysace (take 4)
This patch converts xsysace to use blk_end_request interfaces.
Related 'uptodate' arguments are converted to 'error'.
xsysace is a little bit different from "normal" drivers.
xsysace driver has a state machine in it.
It calls end_that_request_first() and end_that_request_last()
from different states. (ACE_FSM_STATE_REQ_TRANSFER and
ACE_FSM_STATE_REQ_COMPLETE, respectively.)
However, those states are consecutive and without any interruption
inbetween.
So we can just follow the standard conversion rule (b) mentioned in
the patch subject "[PATCH 01/30] blk_end_request: add new request
completion interface".
In ace_fsm_dostate(), the variable 'i' was used only for passing
sector size of the request to end_that_request_first().
So I removed it and changed the code to pass the size in bytes
directly to __blk_end_request().
Cc: Grant Likely <[EMAIL PROTECTED]>
Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]>
Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]>
---
drivers/block/xsysace.c |9 ++---
1 files changed, 2 insertions(+), 7 deletions(-)
Index: 2.6.24-rc4/drivers/block/xsysace.c
===
--- 2.6.24-rc4.orig/drivers/block/xsysace.c
+++ 2.6.24-rc4/drivers/block/xsysace.c
@@ -483,7 +483,6 @@ static void ace_fsm_dostate(struct ace_d
u32 status;
u16 val;
int count;
- int i;
#if defined(DEBUG)
dev_dbg(ace->dev, "fsm_state=%i, id_req_count=%i\n",
@@ -688,7 +687,6 @@ static void ace_fsm_dostate(struct ace_d
}
/* Transfer the next buffer */
- i = 16;
if (ace->fsm_task == ACE_TASK_WRITE)
ace->reg_ops->dataout(ace);
else
@@ -702,8 +700,8 @@ static void ace_fsm_dostate(struct ace_d
}
/* bio finished; is there another one? */
- i = ace->req->current_nr_sectors;
- if (end_that_request_first(ace->req, 1, i)) {
+ if (__blk_end_request(ace->req, 0,
+ blk_rq_cur_bytes(ace->req))) {
/* dev_dbg(ace->dev, "next block; h=%li c=%i\n",
* ace->req->hard_nr_sectors,
* ace->req->current_nr_sectors);
@@ -718,9 +716,6 @@ static void ace_fsm_dostate(struct ace_d
break;
case ACE_FSM_STATE_REQ_COMPLETE:
- /* Complete the block request */
- blkdev_dequeue_request(ace->req);
- end_that_request_last(ace->req, 1);
ace->req = NULL;
/* Finished request; go to idle state */
-
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] REQ-flags to/from BIO-flags bugfix
On Wed, Dec 12, 2007 at 06:06:45PM +0200, Boaz Harrosh wrote: > Thats not enough You still need to fix code in ll_rw_blk(), I would > define a rq_flags_bio_match_mask = 0xf for that. > (and also add what Jens called "needed" with the > BIO_RW_AHEAD selects REQ_FAILFAST.) Yes, I appreciate it's not enough; that's why I didn't sign-off on it. Nobody currently sets BIO_RW_AHEAD, so I don't understand why we need to do anything ;-) > And I still don't understand why, for example, "Domain Validation" fails > with the original patch. What sets BIO_RW_FAILFAST and than panics > on Errors? > (All I see is this flag set in dm/multipath.c & dm-mpath.c) No idea ... -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." - 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] REQ-flags to/from BIO-flags bugfix
On Wed, Dec 12 2007 at 17:18 +0200, Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> On Wed, Dec 12, 2007 at 01:03:10PM +0200, Boaz Harrosh wrote:
>> - BIO flags bio->bi_rw and REQ flags req->cmd_flags no longer match.
>>Remove comments and do a proper translation between the 2 systems.
>
> I'd rather see them resynchronised ... in a way that makes it obvious
> that they should be desynced again:
>
> I don't know whether BIO_RW_BARRIER is __REQ_SOFTBARRIER or
> __REQ_HARDBARRIER, so I didn't include that in this patch. There also
> doesn't seem to be a __REQ equivalent to BIO_RW_AHEAD, but we can do
> the other four bits (and leave gaps for those two).
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index d18ee67..6aef34b 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -167,11 +167,13 @@ enum {
> };
>
> /*
> - * request type modified bits. first three bits match BIO_RW* bits, important
> + * request type modified bits. Don't change without looking at bi_rw flags
> */
> enum rq_flag_bits {
> - __REQ_RW, /* not set, read. set, write */
> - __REQ_FAILFAST, /* no low level driver retries */
> + __REQ_RW = BIO_RW, /* not set, read. set, write */
> + __REQ_FAILFAST = BIO_RW_FAILFAST, /* no low level driver retries */
> + __REQ_RW_SYNC = BIO_RW_SYNC,/* request is sync (O_DIRECT) */
> + __REQ_RW_META = BIO_RW_META,/* metadata io request */
> __REQ_SORTED, /* elevator knows about this request */
> __REQ_SOFTBARRIER, /* may not be passed by ioscheduler */
> __REQ_HARDBARRIER, /* may not be passed by drive either */
> @@ -185,9 +187,7 @@ enum rq_flag_bits {
> __REQ_QUIET,/* don't worry about errors */
> __REQ_PREEMPT, /* set for "ide_preempt" requests */
> __REQ_ORDERED_COLOR,/* is before or after barrier */
> - __REQ_RW_SYNC, /* request is sync (O_DIRECT) */
> __REQ_ALLOCED, /* request came from our alloc pool */
> - __REQ_RW_META, /* metadata io request */
> __REQ_NR_BITS, /* stops here */
> };
>
>
Thats not enough You still need to fix code in ll_rw_blk(), I would
define a rq_flags_bio_match_mask = 0xf for that.
(and also add what Jens called "needed" with the
BIO_RW_AHEAD selects REQ_FAILFAST.)
And I still don't understand why, for example, "Domain Validation" fails
with the original patch. What sets BIO_RW_FAILFAST and than panics
on Errors?
(All I see is this flag set in dm/multipath.c & dm-mpath.c)
Boaz
-
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] scsi device recovery
On Wed, 2007-12-12 at 15:36 +0100, Bernd Schubert wrote: > On Wednesday 12 December 2007 14:39:27 Matthew Wilcox wrote: > > On Wed, Dec 12, 2007 at 01:54:14PM +0100, Bernd Schubert wrote: > > > below is a patch introducing device recovery, trying to prevent i/o > > > errors when a DID_NO_CONNECT or SOFT_ERROR does happen. > > > > Why doesn't the regular scsi_eh do what you need? > > First of all, it is presently simply not called when the two errors above do > happen. This could be changed, of course. Erm, I think you'll find the error handler does activate on DID_SOFT_ERROR. It causes a retry via the eh. DID_NO_CONNECT is an immediate error with no eh intervention because it means that the target went away. Handling this as a retryable error isn't an option because it will interfere with hotplug. > Secondly, I think scsi_eh is in most cases doing too much. We are fighting > with flaky Infortrend boxes here, and scsi_eh sometimes manages to crash > their scsi channels. In most cases it is sufficient to stall any io to the > device and then to resume. But that's basically the default behaviour of the error handler (stall then resume). > For most scsi devices one probably doesn't need a suspend time or it can be > very small, this still needs to become configurable via sysfs. You mean a wait time beyond what the error handler currently does (basically it waits for the quiesce, begins error handling and then sends a test unit ready when it finishes before restarting). > Thirdly, scsi_eh doesn't give up, in most cases, when the scsi channel of a > Infortrend box crashed, it tried forever to recover. > To improve this is still on my todo list. Could you send traces for this. I thought the error handler had been fixed over the last few years always to terminate. If there's a case where it doesn't, this needs fixing. James - 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] REQ-flags to/from BIO-flags bugfix
On Wed, Dec 12, 2007 at 08:18:14AM -0700, Matthew Wilcox wrote:
> I don't know whether BIO_RW_BARRIER is __REQ_SOFTBARRIER or
> __REQ_HARDBARRIER, so I didn't include that in this patch. There also
> doesn't seem to be a __REQ equivalent to BIO_RW_AHEAD, but we can do
> the other four bits (and leave gaps for those two).
Hm. BIO_RW_AHEAD seems unused:
[EMAIL PROTECTED]:~/kernel/linux-2.6$ grep -r BIO_RW_AHEAD *
block/blktrace.c: (((rw) & (1 << BIO_RW_AHEAD)) << (2 - BIO_RW_AHEAD))
include/linux/bio.h:#define BIO_RW_AHEAD1
include/linux/bio.h:#define bio_rw_ahead(bio) ((bio)->bi_rw & (1 <<
BIO_RW_AHEAD))
[EMAIL PROTECTED]:~/kernel/linux-2.6$ grep -r bio_rw_ahead *
block/ll_rw_blk.c: if (bio_rw_ahead(bio) || bio_failfast(bio))
drivers/md/dm-mpath.c: if ((error == -EWOULDBLOCK) && bio_rw_ahead(bio))
drivers/md/multipath.c: else if (!bio_rw_ahead(bio)) {
include/linux/bio.h:#define bio_rw_ahead(bio) ((bio)->bi_rw & (1 <<
BIO_RW_AHEAD))
BIO_RW_BARRIER seems to be __REQ_HARDBARRIER, but we set it
explicitly in init_request_from_bio(). We could probably simplify
init_request_from_bio() with the patch in the previous message.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
-
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 21/30] blk_end_request: changing cciss (take 4)
>
> This patch converts cciss to use blk_end_request interfaces.
> Related 'uptodate' arguments are converted to 'error'.
>
> cciss is a little bit different from "normal" drivers.
> cciss directly calls bio_endio() and disk_stat_add() when
> completing request. But those can be replaced with
> __end_that_request_first().
> After the replacement, request completion procedures of those
> drivers become like the following:
> o end_that_request_first()
> o add_disk_randomness()
> o end_that_request_last()
> This can be converted to blk_end_request() by following the
> rule (a) mentioned in the patch subject "[PATCH 01/30]
> blk_end_request: add new request completion interface".
>
> Cc: Mike Miller <[EMAIL PROTECTED]>
> Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]>
> Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]>
> ---
> drivers/block/cciss.c | 25 +++--
> 1 files changed, 3 insertions(+), 22 deletions(-)
>
> Index: 2.6.24-rc4/drivers/block/cciss.c
> ===
> --- 2.6.24-rc4.orig/drivers/block/cciss.c
> +++ 2.6.24-rc4/drivers/block/cciss.c
> @@ -1187,17 +1187,6 @@ static int cciss_ioctl(struct inode *ino
> }
> }
>
> -static inline void complete_buffers(struct bio *bio, int status) -{
> - while (bio) {
> - struct bio *xbh = bio->bi_next;
> -
> - bio->bi_next = NULL;
> - bio_endio(bio, status ? 0 : -EIO);
> - bio = xbh;
> - }
> -}
> -
> static void cciss_check_queues(ctlr_info_t *h) {
> int start_queue = h->next_to_run; @@ -1263,21
> +1252,14 @@ static void cciss_softirq_done(struct re
> pci_unmap_page(h->pdev, temp64.val,
> cmd->SG[i].Len, ddir);
> }
>
> - complete_buffers(rq->bio, (rq->errors == 0));
> -
> - if (blk_fs_request(rq)) {
> - const int rw = rq_data_dir(rq);
> -
> - disk_stat_add(rq->rq_disk, sectors[rw],
> rq->nr_sectors);
> - }
> -
> #ifdef CCISS_DEBUG
> printk("Done with %p\n", rq);
> #endif /* CCISS_DEBUG */
>
> - add_disk_randomness(rq->rq_disk);
> + if (blk_end_request(rq, (rq->errors == 0) ? 0 : -EIO,
> blk_rq_bytes(rq)))
> + BUG();
> +
> spin_lock_irqsave(&h->lock, flags);
> - end_that_request_last(rq, (rq->errors == 0));
> cmd_free(h, cmd, 1);
> cciss_check_queues(h);
> spin_unlock_irqrestore(&h->lock, flags); @@ -2544,7
> +2526,6 @@ after_error_processing:
> }
> cmd->rq->data_len = 0;
> cmd->rq->completion_data = cmd;
> - blk_add_trace_rq(cmd->rq->q, cmd->rq, BLK_TA_COMPLETE);
Why is this removed?
-- mikem
> blk_complete_request(cmd->rq);
> }
>
>
-
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 22/30] blk_end_request: changing cpqarray (take 4)
> -Original Message-
> From: Kiyoshi Ueda [mailto:[EMAIL PROTECTED]
> Sent: Tuesday, December 11, 2007 4:50 PM
> To: [EMAIL PROTECTED]
> Cc: [EMAIL PROTECTED]; linux-scsi@vger.kernel.org;
> [EMAIL PROTECTED]; [EMAIL PROTECTED];
> [EMAIL PROTECTED]; [EMAIL PROTECTED]; Miller, Mike (OS Dev)
> Subject: [PATCH 22/30] blk_end_request: changing cpqarray (take 4)
>
> This patch converts cpqarray to use blk_end_request interfaces.
> Related 'ok' arguments are converted to 'error'.
>
> cpqarray is a little bit different from "normal" drivers.
> cpqarray directly calls bio_endio() and disk_stat_add() when
> completing request. But those can be replaced with
> __end_that_request_first().
> After the replacement, request completion procedures of those
> drivers become like the following:
> o end_that_request_first()
> o add_disk_randomness()
> o end_that_request_last()
> This can be converted to __blk_end_request() by following the
> rule (b) mentioned in the patch subject "[PATCH 01/30]
> blk_end_request: add new request completion interface".
>
> Cc: Mike Miller <[EMAIL PROTECTED]>
> Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]>
> Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]>
Acked-by: Mike Miller <[EMAIL PROTECTED]>
> ---
> drivers/block/cpqarray.c | 36 +++-
> 1 files changed, 7 insertions(+), 29 deletions(-)
>
> Index: 2.6.24-rc4/drivers/block/cpqarray.c
> ===
> --- 2.6.24-rc4.orig/drivers/block/cpqarray.c
> +++ 2.6.24-rc4/drivers/block/cpqarray.c
> @@ -167,7 +167,6 @@ static void start_io(ctlr_info_t *h);
>
> static inline void addQ(cmdlist_t **Qptr, cmdlist_t *c);
> static inline cmdlist_t *removeQ(cmdlist_t **Qptr, cmdlist_t
> *c); -static inline void complete_buffers(struct bio *bio,
> int ok); static inline void complete_command(cmdlist_t *cmd,
> int timeout);
>
> static irqreturn_t do_ida_intr(int irq, void *dev_id); @@
> -980,26 +979,13 @@ static void start_io(ctlr_info_t *h)
> }
> }
>
> -static inline void complete_buffers(struct bio *bio, int ok) -{
> - struct bio *xbh;
> -
> - while (bio) {
> - xbh = bio->bi_next;
> - bio->bi_next = NULL;
> -
> - bio_endio(bio, ok ? 0 : -EIO);
> -
> - bio = xbh;
> - }
> -}
> /*
> * Mark all buffers that cmd was responsible for
> */
> static inline void complete_command(cmdlist_t *cmd, int timeout) {
> struct request *rq = cmd->rq;
> - int ok=1;
> + int error = 0;
> int i, ddir;
>
> if (cmd->req.hdr.rcode & RCODE_NONFATAL && @@
> -1011,16 +997,17 @@ static inline void complete_command(cmdl
> if (cmd->req.hdr.rcode & RCODE_FATAL) {
> printk(KERN_WARNING "Fatal error on ida/c%dd%d\n",
> cmd->ctlr, cmd->hdr.unit);
> - ok = 0;
> + error = -EIO;
> }
> if (cmd->req.hdr.rcode & RCODE_INVREQ) {
> printk(KERN_WARNING "Invalid
> request on ida/c%dd%d = (cmd=%x sect=%d cnt=%d sg=%d ret=%x)\n",
> cmd->ctlr, cmd->hdr.unit,
> cmd->req.hdr.cmd,
> cmd->req.hdr.blk,
> cmd->req.hdr.blk_cnt,
> cmd->req.hdr.sg_cnt,
> cmd->req.hdr.rcode);
> - ok = 0;
> + error = -EIO;
> }
> - if (timeout) ok = 0;
> + if (timeout)
> + error = -EIO;
> /* unmap the DMA mapping for all the scatter gather
> elements */
> if (cmd->req.hdr.cmd == IDA_READ)
> ddir = PCI_DMA_FROMDEVICE; @@ -1030,18
> +1017,9 @@ static inline void complete_command(cmdl
> pci_unmap_page(hba[cmd->ctlr]->pci_dev,
> cmd->req.sg[i].addr,
> cmd->req.sg[i].size, ddir);
>
> - complete_buffers(rq->bio, ok);
> -
> - if (blk_fs_request(rq)) {
> - const int rw = rq_data_dir(rq);
> -
> - disk_stat_add(rq->rq_disk, sectors[rw],
> rq->nr_sectors);
> - }
> -
> - add_disk_randomness(rq->rq_disk);
> -
> DBGPX(printk("Done with %p\n", rq););
> - end_that_request_last(rq, ok ? 1 : -EIO);
> + if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
> + BUG();
> }
>
> /*
>
-
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] REQ-flags to/from BIO-flags bugfix
On Wed, Dec 12, 2007 at 01:03:10PM +0200, Boaz Harrosh wrote:
> - BIO flags bio->bi_rw and REQ flags req->cmd_flags no longer match.
>Remove comments and do a proper translation between the 2 systems.
I'd rather see them resynchronised ... in a way that makes it obvious
that they should be desynced again:
I don't know whether BIO_RW_BARRIER is __REQ_SOFTBARRIER or
__REQ_HARDBARRIER, so I didn't include that in this patch. There also
doesn't seem to be a __REQ equivalent to BIO_RW_AHEAD, but we can do
the other four bits (and leave gaps for those two).
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d18ee67..6aef34b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -167,11 +167,13 @@ enum {
};
/*
- * request type modified bits. first three bits match BIO_RW* bits, important
+ * request type modified bits. Don't change without looking at bi_rw flags
*/
enum rq_flag_bits {
- __REQ_RW, /* not set, read. set, write */
- __REQ_FAILFAST, /* no low level driver retries */
+ __REQ_RW = BIO_RW, /* not set, read. set, write */
+ __REQ_FAILFAST = BIO_RW_FAILFAST, /* no low level driver retries */
+ __REQ_RW_SYNC = BIO_RW_SYNC,/* request is sync (O_DIRECT) */
+ __REQ_RW_META = BIO_RW_META,/* metadata io request */
__REQ_SORTED, /* elevator knows about this request */
__REQ_SOFTBARRIER, /* may not be passed by ioscheduler */
__REQ_HARDBARRIER, /* may not be passed by drive either */
@@ -185,9 +187,7 @@ enum rq_flag_bits {
__REQ_QUIET,/* don't worry about errors */
__REQ_PREEMPT, /* set for "ide_preempt" requests */
__REQ_ORDERED_COLOR,/* is before or after barrier */
- __REQ_RW_SYNC, /* request is sync (O_DIRECT) */
__REQ_ALLOCED, /* request came from our alloc pool */
- __REQ_RW_META, /* metadata io request */
__REQ_NR_BITS, /* stops here */
};
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
-
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] scsi device recovery
On Wednesday 12 December 2007 14:39:27 Matthew Wilcox wrote: > On Wed, Dec 12, 2007 at 01:54:14PM +0100, Bernd Schubert wrote: > > below is a patch introducing device recovery, trying to prevent i/o > > errors when a DID_NO_CONNECT or SOFT_ERROR does happen. > > Why doesn't the regular scsi_eh do what you need? First of all, it is presently simply not called when the two errors above do happen. This could be changed, of course. Secondly, I think scsi_eh is in most cases doing too much. We are fighting with flaky Infortrend boxes here, and scsi_eh sometimes manages to crash their scsi channels. In most cases it is sufficient to stall any io to the device and then to resume. For most scsi devices one probably doesn't need a suspend time or it can be very small, this still needs to become configurable via sysfs. Thirdly, scsi_eh doesn't give up, in most cases, when the scsi channel of a Infortrend box crashed, it tried forever to recover. To improve this is still on my todo list. Thanks, Bernd -- Bernd Schubert Q-Leap Networks GmbH - 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: broken dpt_i2o in 2.6.23 (was: ext2 check page: bad entry in directory) (fwd)
Am 12.12.2007 schrieb Andrew Morton: > On Wed, 12 Dec 2007 11:58:41 +0100 Anders Henke <[EMAIL PROTECTED]> wrote: > > > Hi, > > > > I'd like to let you now that my boxes are running a 32-bit kernel, so > > the 64-bit-uncleanliness shouldn't apply to my boxes; however, > > > > http://www.miquels.cistron.nl/linux/dpt_i2o-64bit-2.6.23.patch > > > > fixed the issue on my testbox. > > > > I took a clean 2.6.23, applied patch, recompiled the kernel, reboot: works. > > What a huge patch :( > > We already reverted the offening patch so I assume that 2.6.24-rc5 is > working for you? Yes, the vanilla 2.6.24-rc5 works fine (at least it's booting :-). Linux rdb140 2.6.24-rc5 #1 SMP Wed Dec 12 15:06:05 CET 2007 i686 GNU/Linux > I guess we need to look at restoring "dpt_i2o: convert to SCSI hotplug > model" and then absorbing what Miquel has done there. I've tried 2.6.23 with http://www.miquels.cistron.nl/linux/linux-2.6.23+24-dpt_i2o-dma64.patch ... and that's enough to make my boxes boot again. Regards, Anders -- 1&1 Internet AGEnter any 11-digit prime number to continue. Brauerstrasse 48 v://49.721.91374.50 D-76135 Karlsruhe f://49.721.91374.225 Amtsgericht Montabaur HRB 6484 Vorstand: Henning Ahlert, Ralph Dommermuth, Matthias Ehrlich, Andreas Gauger, Thomas Gottschlich, Matthias Greve, Robert Hoffmann, Norbert Lang, Achim Weiss Aufsichtsratsvorsitzender: Michael Scheeren - 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: broken dpt_i2o in 2.6.23 (was: ext2 check page: bad entry in directory) (fwd)
Am 12.12.2007 schrieb Miquel van Smoorenburg: > On Wed, 2007-12-12 at 03:38 -0800, Andrew Morton wrote: > > On Wed, 12 Dec 2007 11:58:41 +0100 Anders Henke <[EMAIL PROTECTED]> wrote: > > > > > Hi, > > > > > > I'd like to let you now that my boxes are running a 32-bit kernel, so > > > the 64-bit-uncleanliness shouldn't apply to my boxes; however, > > > > > > http://www.miquels.cistron.nl/linux/dpt_i2o-64bit-2.6.23.patch > > > > > > fixed the issue on my testbox. > > > > > > I took a clean 2.6.23, applied patch, recompiled the kernel, reboot: > > > works. > > > > What a huge patch :( > > > > We already reverted the offening patch so I assume that 2.6.24-rc5 is > > working for you? > > > > I guess we need to look at restoring "dpt_i2o: convert to SCSI hotplug > > model" and then absorbing what Miquel has done there. > > This was just a patch I had lying around, if it worked it would confirm > my suspicion, which it has. > > The minimal patch which is suitable for 2.6.23-stable and 2.6.24 would > be the attached one-liner. The "dpt_i2o: convert to SCSI hotplug model" > patch could be restored then. > > (if the list eats the attachment, it's also available here: > http://www.miquels.cistron.nl/linux/linux-2.6.23+24-dpt_i2o-dma64.patch > ) > > Anders, does this one-liner patch work for you ? Got it - and it works! I took a clean 2.6.23, applied the patch, recompiled the kernel and rebooted my testbox: came up with the fresh-compiled kernel (verified by "uname -a"). Regards, Anders -- 1&1 Internet AG System Design Brauerstrasse 48 v://49.721.91374.50 D-76135 Karlsruhef://49.721.91374.225 Amtsgericht Montabaur HRB 6484 Vorstand: Henning Ahlert, Ralph Dommermuth, Matthias Ehrlich, Andreas Gauger, Thomas Gottschlich, Matthias Greve, Robert Hoffmann, Norbert Lang, Achim Weiss Aufsichtsratsvorsitzender: Michael Scheeren - 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] scsi device recovery
On Wed, Dec 12, 2007 at 01:54:14PM +0100, Bernd Schubert wrote: > below is a patch introducing device recovery, trying to prevent i/o errors > when a DID_NO_CONNECT or SOFT_ERROR does happen. Why doesn't the regular scsi_eh do what you need? -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." - 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: broken dpt_i2o in 2.6.23 (was: ext2_check_page: bad entry indirectory) (fwd)
ACK, patch looks good. Thanks for composing this patch. Glad to hear of successful test results. Sincerely -- Mark Salyzyn > -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of Miquel > van Smoorenburg . . . > > I just recompiled 2.6.23.9 with the 64 bit patch for dpt_i2o > and now it > boots just fine. > > The patch is here: > http://www.miquels.cistron.nl/linux/dpt_i2o-64bit-2.6.23.patch > > It's not the final version - it needs a few cleanups before it can be > submitted, but perhaps you can test if it also works for you. > > Mike. - 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: broken dpt_i2o in 2.6.23 (was: ext2 check page: bad entry in directory) (fwd)
On Wed, 2007-12-12 at 03:38 -0800, Andrew Morton wrote: > On Wed, 12 Dec 2007 11:58:41 +0100 Anders Henke <[EMAIL PROTECTED]> wrote: > > > Hi, > > > > I'd like to let you now that my boxes are running a 32-bit kernel, so > > the 64-bit-uncleanliness shouldn't apply to my boxes; however, > > > > http://www.miquels.cistron.nl/linux/dpt_i2o-64bit-2.6.23.patch > > > > fixed the issue on my testbox. > > > > I took a clean 2.6.23, applied patch, recompiled the kernel, reboot: works. > > What a huge patch :( > > We already reverted the offening patch so I assume that 2.6.24-rc5 is > working for you? > > I guess we need to look at restoring "dpt_i2o: convert to SCSI hotplug > model" and then absorbing what Miquel has done there. This was just a patch I had lying around, if it worked it would confirm my suspicion, which it has. The minimal patch which is suitable for 2.6.23-stable and 2.6.24 would be the attached one-liner. The "dpt_i2o: convert to SCSI hotplug model" patch could be restored then. (if the list eats the attachment, it's also available here: http://www.miquels.cistron.nl/linux/linux-2.6.23+24-dpt_i2o-dma64.patch ) Anders, does this one-liner patch work for you ? Mike. diff -ruN linux-2.6.23.9.orig/drivers/scsi/dpt_i2o.c linux-2.6.23.9/drivers/scsi/dpt_i2o.c --- linux-2.6.23.9.orig/drivers/scsi/dpt_i2o.c 2007-11-26 18:51:43.0 +0100 +++ linux-2.6.23.9/drivers/scsi/dpt_i2o.c 2007-12-12 13:21:05.0 +0100 @@ -905,8 +905,7 @@ } pci_set_master(pDev); - if (pci_set_dma_mask(pDev, DMA_64BIT_MASK) && - pci_set_dma_mask(pDev, DMA_32BIT_MASK)) + if (pci_set_dma_mask(pDev, DMA_32BIT_MASK)) return -EINVAL; base_addr0_phys = pci_resource_start(pDev,0);
Re: [PATCH 27/30] blk_end_request: changing scsi (take 4)
On Tue, 2007-12-11 at 17:52 -0500, Kiyoshi Ueda wrote: > This patch converts scsi mid-layer to use blk_end_request interfaces. > Related 'uptodate' arguments are converted to 'error'. > > As a result, the interface of internal function, scsi_end_request(), > is changed. This looks fine, as far as it goes. One of the things that might actually be useful in future is as well as communicating the error, the ability to say more information about what happened. For instance dm-mp needs to know whether the error is transport or device related (the former being retryable on a different path). James - 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
[PATCH] scsi device recovery
Hi,
below is a patch introducing device recovery, trying to prevent i/o errors
when a DID_NO_CONNECT or SOFT_ERROR does happen.
The patch still needs quite some work:
1.) I still didn't figure out what is the best place to run
sdev->deh.ehandler = kthread_run(scsi_device_error_handler, ...)
2.) As I see it, its not a good idea to run spi_schedule_dv_device() in
scsi_error.c, since spi_schedule_dv_device() is in scsi_transport_spi.c,
which seems to be separated from the core scsi-layer.
So what is another way to initiate a DV in scsi_error.c?
3.) Maybe related to 2), for now I'm calling spi_schedule_dv_device(), but
this is not always doing what I want.
[ 406.785104] sd 5:0:2:0: deh: scheduling domain validation
[ 408.422530] target5:0:2: Beginning Domain Validation
[ 408.466620] target5:0:2: Domain Validation skipping write tests
[ 408.472771] target5:0:2: Ending Domain Validation
Hmm, somehow related to sdev->inquiry_len, but isn't it the task of
spi_schedule_dv_device() and subfunctions to do that properly?
Any comments, hints and help is appreciated.
Signed-of-by: Bernd Schubert <[EMAIL PROTECTED]>
Index: linux-2.6.22/drivers/scsi/scsi_error.c
===
--- linux-2.6.22.orig/drivers/scsi/scsi_error.c 2007-12-12 12:26:20.0
+0100
+++ linux-2.6.22/drivers/scsi/scsi_error.c 2007-12-12 13:08:40.0
+0100
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include "scsi_priv.h"
#include "scsi_logging.h"
@@ -1589,6 +1590,153 @@ int scsi_error_handler(void *data)
return 0;
}
+/**
+ * scsi_unjam_sdev - try to revover a failed scsi-device
+ * @sdev: scsi device we are recovering
+ */
+static int scsi_unjam_sdev(struct scsi_device *sdev)
+{
+ int rtn;
+
+ sdev_printk(KERN_CRIT, sdev, "resetting device\n");
+ rtn = scsi_reset_provider(sdev, SCSI_TRY_RESET_DEVICE);
+ scsi_report_device_reset(sdev->host, sdev->channel, sdev->id);
+ if (rtn == SUCCESS)
+ sdev_printk(KERN_INFO, sdev, "device reset succeeded, "
+ "set device to running state\n");
+ return SUCCESS;
+}
+
+/**
+ * scsi_schedule_deh - schedule EH for SCSI device
+ * @sdev: SCSI device to invoke error handling on.
+ *
+ **/
+void scsi_schedule_deh(struct scsi_device *sdev)
+{
+#if 0
+ if (sdev->deh.error) {
+ /* blocking the device does not work! another recovery was
+* scheduled, though no i/o should go to the device now! */
+ sdev_printk(KERN_CRIT, sdev,
+ "device already in recovery, but another recovery "
+ "was scheduled\n");
+ dump_stack();
+ }
+#endif
+ if (sdev->deh.error)
+ return; /* recovery already running */
+
+ if (sdev->deh.last_recovery
+ && jiffies < sdev->deh.last_recovery + 300 * HZ)
+ sdev->deh.count++;
+ else
+ sdev->deh.count = 0;
+
+ if (sdev->deh.count >= 10) {
+ sdev_printk(KERN_WARNING, sdev,
+ "too many errors within time limit, setting "
+ "device offline\n");
+ scsi_device_set_state(sdev, SDEV_OFFLINE);
+ return;
+ } else if (sdev->deh.count >= 5) {
+ sdev_printk(KERN_INFO, sdev, "Initiating host recovery\n");
+ scsi_schedule_eh(sdev->host); /* host recovery */
+ return;
+ } else
+ sdev->deh.count++;
+
+ sdev_printk(KERN_INFO, sdev, "n-error: %d\n", sdev->deh.count);
+
+ if (!scsi_internal_device_block(sdev)) {
+ sdev->deh.error = 1;
+ if (sdev->deh.ehandler)
+ wake_up_process(sdev->deh.ehandler);
+ else
+ sdev_printk(KERN_WARNING, sdev,
+ "deh handler missing\n");
+ } else {
+ sdev_printk(KERN_WARNING, sdev,
+ "Couldn't block device, calling host recovery\n");
+ scsi_schedule_eh(sdev->host);
+ }
+}
+EXPORT_SYMBOL_GPL(scsi_schedule_deh);
+
+/**
+ * scsi_device_error_handler - SCSI error handler thread
+ * @data: Device for which we are running.
+ *
+ * Notes:
+ *This is the main device error handling loop. This is run as a kernel
thread
+ *for every SCSI device and handles all device error handling activity.
+ **/
+int scsi_device_error_handler(void *data)
+{
+ struct scsi_device *sdev = data;
+ int sleeptime = 30;
+
+ current->flags |= PF_NOFREEZE;
+
+ /*
+* We use TASK_INTERRUPTIBLE so that the thread is not
+* counted against the load average as a running process.
+* We never actually get interrupted because kthread_run
+* disables singal delivery for the created thread.
+*/
+ set_curr
Re: [PATCH 01/30] blk_end_request: add new request completion interface (take 4)
On Tue, 2007-12-11 at 17:40 -0500, Kiyoshi Ueda wrote:
> This patch adds 2 new interfaces for request completion:
> o blk_end_request() : called without queue lock
> o __blk_end_request() : called with queue lock held
>
> blk_end_request takes 'error' as an argument instead of 'uptodate',
> which current end_that_request_* take.
> The meanings of values are below and the value is used when bio is
> completed.
> 0 : success
> < 0 : error
>
> Some device drivers call some generic functions below between
> end_that_request_{first/chunk} and end_that_request_last().
> o add_disk_randomness()
> o blk_queue_end_tag()
> o blkdev_dequeue_request()
If we can roll the whole thing together, that would be nice. However,
the way you're doing it with this patch, we now have an asymmetrical
interface: The request routine must explicitly start the tag, but now
doesn't have to end it.
We really need symmetry. Either go back to start tag/end tag, or absorb
the whole lot into the block infrastructure.
The original reason for the explicit start/end is that there are some
requests on a tagged device that aren't able to be tagged by the block
layer (some devices reserve tag numbers for specific meanings).
However, I don't think there's any driver that actually implemented this
feature.
James
-
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 00/30] blk_end_request: full I/O completion handler (take 4)
On Tue, Dec 11 2007, Kiyoshi Ueda wrote: > Hi Jens, Boaz, > > The following is the updated patch-set for blk_end_request(). > I have done some interface/implementation changes based on > feedbacks/discussions since the previous version. > (Although this patch-set was made on top of 2.6.24-rc4, I confirmed > it can be applied to 2.6.24-rc5, too. Also, I confirmed it has > no build error on my IA64 box.) Trying to apply this, but it fails from patch #24 and on: patching file block/ll_rw_blk.c Hunk #1 succeeded at 3852 (offset 49 lines). Hunk #2 succeeded at 3867 with fuzz 2 (offset 49 lines). Hunk #3 succeeded at 3904 with fuzz 1 (offset 66 lines). Hunk #4 FAILED at 3916. Hunk #5 succeeded at 3967 with fuzz 2 (offset 60 lines). 1 out of 5 hunks FAILED -- saving rejects to file block/ll_rw_blk.c.rej patching file include/linux/blkdev.h Reversed (or previously applied) patch detected! Assume -R? [n] This is 2.6.24-rc5 (pretty close to at least, no changes to ll_rw_blk.c from that version). Are you using quilt and forgot to refresh, or something like that? -- Jens Axboe - 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: 2.6.24-rc3-mm1
On Wed, Dec 12 2007, Boaz Harrosh wrote:
> On Tue, Dec 11 2007 at 18:33 +0200, James Bottomley <[EMAIL PROTECTED]> wrote:
> > On Mon, 2007-11-26 at 22:15 -0800, Andrew Morton wrote:
> >> OK, thanks. I'll assume that James and Hannes have this in hand (or will
> >> have, by mid-week) and I won't do anything here.
> >
> > Just to confirm what I think I'm going to be doing: rebasing the
> > scsi-misc tree to remove this commit:
> >
> > commit 8655a546c83fc43f0a73416bbd126d02de7ad6c0
> > Author: Hannes Reinecke <[EMAIL PROTECTED]>
> > Date: Tue Nov 6 09:23:40 2007 +0100
> >
> > [SCSI] Do not requeue requests if REQ_FAILFAST is set
> >
> > And its allied fix ups:
> >
> > commit 983289045faa96fba8841d3c51b98bb8623d9504
> > Author: James Bottomley <[EMAIL PROTECTED]>
> > Date: Sat Nov 24 19:47:25 2007 +0200
> >
> > [SCSI] fix up REQ_FASTFAIL not to fail when state is QUIESCE
> >
> > commit 9dd15a13b332e9f5c8ee752b1ccd9b84cb5bdf17
> > Author: James Bottomley <[EMAIL PROTECTED]>
> > Date: Sat Nov 24 19:55:53 2007 +0200
> >
> > [SCSI] fix domain validation to work again
> >
> > James
> >
>
> The problems caused by this patch where nagging me at the back of my head
> from the begging. Why should we fail on a check of FAIL_FAST in all kind
> of weird places like boots, when the only place that should ever set the
> flag should be one of the multi-path drivers. finally it struck me:
>
> It might be a bug in ll_rw_blk at blk_rq_bio_prep() there is this:
>
> static void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
> struct bio *bio)
> {
> /* first two bits are identical in rq->cmd_flags and bio->bi_rw */
> rq->cmd_flags |= (bio->bi_rw & 3);
> ...
>
> Now this is no longer true and is a bug.
> Second bit of bio->bi_rw defined in bio.h is:
> #define BIO_RW_AHEAD 1
> but
> Second bit of rq->cmd_flags is __REQ_FAILFAST
>
> so maybe we are getting FAILFAST in the wrong places?
But that's actually on purpose, though the comment is pretty much crap.
We don't want to be retrying readahead requests, those should always
just be tossable.
--
Jens Axboe
-
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: broken dpt_i2o in 2.6.23 (was: ext2 check page: bad entry in directory) (fwd)
On Wed, 12 Dec 2007 11:58:41 +0100 Anders Henke <[EMAIL PROTECTED]> wrote: > Hi, > > I'd like to let you now that my boxes are running a 32-bit kernel, so > the 64-bit-uncleanliness shouldn't apply to my boxes; however, > > http://www.miquels.cistron.nl/linux/dpt_i2o-64bit-2.6.23.patch > > fixed the issue on my testbox. > > I took a clean 2.6.23, applied patch, recompiled the kernel, reboot: works. What a huge patch :( We already reverted the offening patch so I assume that 2.6.24-rc5 is working for you? I guess we need to look at restoring "dpt_i2o: convert to SCSI hotplug model" and then absorbing what Miquel has done there. - 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
[PATCH] REQ-flags to/from BIO-flags bugfix
- BIO flags bio->bi_rw and REQ flags req->cmd_flags no longer match.
Remove comments and do a proper translation between the 2 systems.
(Please look in ll_rw_blk.c/blk_rq_bio_prep() below if we need more flags)
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
block/ll_rw_blk.c| 23 +--
include/linux/blktrace_api.h |8 +++-
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/block/ll_rw_blk.c b/block/ll_rw_blk.c
index 8b91994..c6a84bb 100644
--- a/block/ll_rw_blk.c
+++ b/block/ll_rw_blk.c
@@ -1990,10 +1990,6 @@ blk_alloc_request(struct request_queue *q, int rw, int
priv, gfp_t gfp_mask)
if (!rq)
return NULL;
- /*
-* first three bits are identical in rq->cmd_flags and bio->bi_rw,
-* see bio.h and blkdev.h
-*/
rq->cmd_flags = rw | REQ_ALLOCED;
if (priv) {
@@ -3772,8 +3768,23 @@ EXPORT_SYMBOL(end_request);
static void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
struct bio *bio)
{
- /* first two bits are identical in rq->cmd_flags and bio->bi_rw */
- rq->cmd_flags |= (bio->bi_rw & 3);
+ if (bio_data_dir(bio))
+ rq->cmd_flags |= REQ_RW;
+ else
+ rq->cmd_flags &= ~REQ_RW;
+
+ if (bio->bi_rw & (1cmd_flags &= ~REQ_RW_SYNC;
+ /* FIXME: what about other flags, should we sync these too? */
+ /*
+ BIO_RW_AHEAD==> ??
+ BIO_RW_BARRIER ==> REQ_SOFTBARRIER/REQ_HARDBARRIER
+ BIO_RW_FAILFAST ==> REQ_FAILFAST
+ BIO_RW_SYNC ==> REQ_RW_SYNC
+ BIO_RW_META ==> REQ_RW_META
+ */
rq->nr_phys_segments = bio_phys_segments(q, bio);
rq->nr_hw_segments = bio_hw_segments(q, bio);
diff --git a/include/linux/blktrace_api.h b/include/linux/blktrace_api.h
index 7e11d23..9e7ce65 100644
--- a/include/linux/blktrace_api.h
+++ b/include/linux/blktrace_api.h
@@ -165,7 +165,13 @@ static inline void blk_add_trace_rq(struct request_queue
*q, struct request *rq,
u32 what)
{
struct blk_trace *bt = q->blk_trace;
- int rw = rq->cmd_flags & 0x03;
+ /* blktrace.c prints them according to bio flags */
+ int rw = (((rq_rw_dir(rq) == WRITE) << BIO_RW) |
+ (((rq->cmd_flags & (REQ_SOFTBARRIER|REQ_HARDBARRIER)) != 0) <<
+ BIO_RW_BARRIER) |
+ (((rq->cmd_flags & REQ_FAILFAST) != 0) << BIO_RW_FAILFAST) |
+ (((rq->cmd_flags & REQ_RW_SYNC) != 0) << BIO_RW_SYNC) |
+ (((rq->cmd_flags & REQ_RW_META) != 0) << BIO_RW_META));
if (likely(!bt))
return;
--
1.5.3.3
-
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: broken dpt_i2o in 2.6.23 (was: ext2 check page: bad entry in directory) (fwd)
Hi, I'd like to let you now that my boxes are running a 32-bit kernel, so the 64-bit-uncleanliness shouldn't apply to my boxes; however, http://www.miquels.cistron.nl/linux/dpt_i2o-64bit-2.6.23.patch fixed the issue on my testbox. I took a clean 2.6.23, applied patch, recompiled the kernel, reboot: works. Regards, Anders PS: Sorry for breaking the threading, I'm not a regular subscriber to linux-kernel and haven't received Miguel's message by mail. -- 1&1 Internet AG System Design Brauerstrasse 48 v://49.721.91374.50 D-76135 Karlsruhef://49.721.91374.225 Amtsgericht Montabaur HRB 6484 Vorstand: Henning Ahlert, Ralph Dommermuth, Matthias Ehrlich, Andreas Gauger, Thomas Gottschlich, Matthias Greve, Robert Hoffmann, Norbert Lang, Achim Weiss Aufsichtsratsvorsitzender: Michael Scheeren - 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: 2.6.24-rc3-mm1
On Tue, Dec 11 2007 at 18:33 +0200, James Bottomley <[EMAIL PROTECTED]> wrote:
> On Mon, 2007-11-26 at 22:15 -0800, Andrew Morton wrote:
>> OK, thanks. I'll assume that James and Hannes have this in hand (or will
>> have, by mid-week) and I won't do anything here.
>
> Just to confirm what I think I'm going to be doing: rebasing the
> scsi-misc tree to remove this commit:
>
> commit 8655a546c83fc43f0a73416bbd126d02de7ad6c0
> Author: Hannes Reinecke <[EMAIL PROTECTED]>
> Date: Tue Nov 6 09:23:40 2007 +0100
>
> [SCSI] Do not requeue requests if REQ_FAILFAST is set
>
> And its allied fix ups:
>
> commit 983289045faa96fba8841d3c51b98bb8623d9504
> Author: James Bottomley <[EMAIL PROTECTED]>
> Date: Sat Nov 24 19:47:25 2007 +0200
>
> [SCSI] fix up REQ_FASTFAIL not to fail when state is QUIESCE
>
> commit 9dd15a13b332e9f5c8ee752b1ccd9b84cb5bdf17
> Author: James Bottomley <[EMAIL PROTECTED]>
> Date: Sat Nov 24 19:55:53 2007 +0200
>
> [SCSI] fix domain validation to work again
>
> James
>
The problems caused by this patch where nagging me at the back of my head
from the begging. Why should we fail on a check of FAIL_FAST in all kind
of weird places like boots, when the only place that should ever set the
flag should be one of the multi-path drivers. finally it struck me:
It might be a bug in ll_rw_blk at blk_rq_bio_prep() there is this:
static void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
struct bio *bio)
{
/* first two bits are identical in rq->cmd_flags and bio->bi_rw */
rq->cmd_flags |= (bio->bi_rw & 3);
...
Now this is no longer true and is a bug.
Second bit of bio->bi_rw defined in bio.h is:
#define BIO_RW_AHEAD1
but
Second bit of rq->cmd_flags is __REQ_FAILFAST
so maybe we are getting FAILFAST in the wrong places?
(I will look for an old patch I sent a year ago that fixes
this bug)
Boaz
-
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 20/30] blk_end_request: changing xsysace (take 4)
Kiyoshi Ueda wrote:
> This patch converts xsysace to use blk_end_request interfaces.
> Related 'uptodate' arguments are converted to 'error'.
>
> xsysace is a little bit different from "normal" drivers.
> xsysace driver has a state machine in it.
> It calls end_that_request_first() and end_that_request_last()
> from different states. (ACE_FSM_STATE_REQ_TRANSFER and
> ACE_FSM_STATE_REQ_COMPLETE, respectively.)
>
> However, those states are consecutive and without any interruption
> inbetween.
> So we can just follow the standard conversion rule (b) mentioned in
> the patch subject "[PATCH 01/30] blk_end_request: add new request
> completion interface".
>
> Cc: Grant Likely <[EMAIL PROTECTED]>
> Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]>
> Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]>
> ---
> drivers/block/xsysace.c |5 +
> 1 files changed, 1 insertion(+), 4 deletions(-)
>
> Index: 2.6.24-rc4/drivers/block/xsysace.c
> ===
> --- 2.6.24-rc4.orig/drivers/block/xsysace.c
> +++ 2.6.24-rc4/drivers/block/xsysace.c
> @@ -703,7 +703,7 @@ static void ace_fsm_dostate(struct ace_d
>
> /* bio finished; is there another one? */
> i = ace->req->current_nr_sectors;
> - if (end_that_request_first(ace->req, 1, i)) {
> + if (__blk_end_request(ace->req, 0, i)) {
end_that_request_first() took sectors __blk_end_request() now takes
bytes
> /* dev_dbg(ace->dev, "next block; h=%li c=%i\n",
>* ace->req->hard_nr_sectors,
>* ace->req->current_nr_sectors);
> @@ -718,9 +718,6 @@ static void ace_fsm_dostate(struct ace_d
> break;
>
> case ACE_FSM_STATE_REQ_COMPLETE:
> - /* Complete the block request */
> - blkdev_dequeue_request(ace->req);
> - end_that_request_last(ace->req, 1);
> ace->req = NULL;
>
> /* Finished request; go to idle state */
> -
> 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
Boaz
-
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
