Re: [PATCH 22/71] ncr5380: Eliminate selecting state
On Sunday 29 November 2015 01:46:03 Finn Thain wrote:
>
> On Sun, 29 Nov 2015, Ondrej Zary wrote:
>
> > > [...] I think that this should solve the problem:
> >
> > Yes, it does!
> >
> > [ 48.119367] scsi host2: Generic NCR5380/NCR53C400 SCSI, io_port 0x0,
> > n_io_port 0, base 0xd8000, irq 0, can_queue 16, cmd_per_lun 2, sg_tablesize
> > 128, this_id 7, flags { NO_DMA_FIXUP }, options { AUTOPROBE_IRQ PSEUDO_DMA }
> > [ 49.715388] scsi 2:0:1:0: Direct-Access QUANTUM LP240S GM240S01X
> > 4.6 PQ: 0 ANSI: 2 CCS
>
> That still takes about 1.6 seconds to scan a vacant bus ID. It should be
> more like 0.25 seconds. Did you have the entire patch series applied?
>
> The code presently in mainline spins for 500 iterations in the relevant
> busy-wait loop, with the comment, "8uS a cycle for the cpu access". That
> doesn't help me much. Does that refer to an ISA bus cycle? Or one
> iteration of the loop? Electrical conductance?
>
> If we knew how many loops_per_jiffy your machine gets, and your CONFIG_HZ
> value, we could figure out how long a chip register access takes (which
> might be 8 us).
loops_per_jiffy=4797252
CONFIG_HZ=250
# cat /proc/cpuinfo
processor : 0
vendor_id : GenuineIntel
cpu family : 6
model : 11
model name : Intel(R) Celeron(TM) CPU1200MHz
stepping: 1
microcode : 0x1c
cpu MHz : 1199.313
cache size : 256 KB
physical id : 0
siblings: 1
core id : 0
cpu cores : 1
apicid : 0
initial apicid : 0
fdiv_bug: no
f00f_bug: no
coma_bug: no
fpu : yes
fpu_exception : yes
cpuid level : 2
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 sep mtrr pge mca cmov
pse36 mmx fxsr sse
bugs:
bogomips: 2398.62
clflush size: 32
cache_alignment : 32
address sizes : 36 bits physical, 32 bits virtual
power management:
# cat iotest.c
#include
#include
int main(void) {
if (iopl(3)) {
perror("iopl");
return 1;
}
for (int i = 0; i < 1000; i++)
inb(0x280);
return 0;
}
# cc --std=c99 iotest.c -o iotest
# time ./iotest
real0m16.938s
user0m16.932s
sys 0m0.000s
outb() instead of inb():
# time ./iotest
real0m17.210s
user0m17.204s
sys 0m0.000s
> I'll try to figure out a similar timing for my Domex PCI card to see if
> there is some sort of compromise to be found.
--
Ondrej Zary
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 22/71] ncr5380: Eliminate selecting state
Hi Finn,
On Thu, Nov 26, 2015 at 11:32 PM, Finn Thain wrote:
>> The timekeeping warning does not appear when all 71 patches are applied.
>> Reverse-bisected it - the warning disappears after:
>> [PATCH 50/71] ncr5380: Change instance->host_lock to hostdata->lock
>>
>
> Makes sense. I think that this should solve the problem:
>
> Index: linux/drivers/scsi/NCR5380.c
> ===
> --- linux.orig/drivers/scsi/NCR5380.c 2015-11-27 09:21:40.0 +1100
> +++ linux/drivers/scsi/NCR5380.c2015-11-27 09:25:36.0 +1100
> @@ -230,7 +230,7 @@ static int NCR5380_poll_politely2(struct
> unsigned long n;
>
> /* Busy-wait for up to 1 jiffy */
> - n = loops_per_jiffy;
> + n = 1 + loops_per_jiffy / 10;
> do {
> if ((NCR5380_read(reg1) & bit1) == val1)
> return 0;
This still heavily depends on the processing time spent in NCR5380_read().
You should never use a value derived from loops_per_jiffy for a non-empty loop,
as it may take much longer. Always compare with an maximum end time instead.
E.g.:
end = jiffies + 2;/* 1 jiffie + 1 safeguard */
do {
if ((NCR5380_read(reg1) & bit1) == val1)
return 0;
cpu_relax();
} while (time_before(jiffies, end);
And a similar loop for "Busy-wait for up to 20 ms".
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 74/71] ncr5380: Enable PDMA for NCR53C400A
Add I/O register mapping for NCR53C400A and enable PDMA mode to
improve performance and fix non-working IRQ.
Tested with HP C2502 (and user-space enabler).
Signed-off-by: Ondrej Zary
---
drivers/scsi/g_NCR5380.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index ce444da..c3abe48 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -324,7 +324,10 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
#endif
break;
case BOARD_NCR53C400A:
+ flags = FLAG_NO_DMA_FIXUP;
+#ifndef PSEUDO_DMA
flags = FLAG_NO_PSEUDO_DMA;
+#endif
ports = ncr_53c400a_ports;
break;
case BOARD_DTC3181E:
@@ -412,6 +415,11 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
hostdata->c400_blk_cnt = 1;
hostdata->c400_host_buf = 4;
}
+ if (overrides[current_override].board == BOARD_NCR53C400A) {
+ hostdata->c400_ctl_status = 9;
+ hostdata->c400_blk_cnt = 10;
+ hostdata->c400_host_buf = 8;
+ }
#else
instance->base = overrides[current_override].NCR5380_map_name;
hostdata->iomem = iomem;
@@ -425,7 +433,8 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
if (NCR5380_init(instance, flags))
goto out_unregister;
- if (overrides[current_override].board == BOARD_NCR53C400)
+ if (overrides[current_override].board == BOARD_NCR53C400 ||
+ overrides[current_override].board == BOARD_NCR53C400A)
NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
NCR5380_maybe_reset_bus(instance);
--
Ondrej Zary
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 73/71] ncr5380: Use runtime register mapping
Convert compile-time C400_ register mapping to runtime mapping.
This removes the weird negative register offsets and allows adding
additional mappings.
Signed-off-by: Ondrej Zary
---
drivers/scsi/NCR5380.h | 13 +-
drivers/scsi/g_NCR5380.c | 61 ++
drivers/scsi/g_NCR5380.h | 12 ++---
3 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/scsi/NCR5380.h b/drivers/scsi/NCR5380.h
index 36779df..0d8ec43 100644
--- a/drivers/scsi/NCR5380.h
+++ b/drivers/scsi/NCR5380.h
@@ -163,8 +163,7 @@
/* Write any value to this register to start an ini mode DMA receive */
#define START_DMA_INITIATOR_RECEIVE_REG 7 /* wo */
-#define C400_CONTROL_STATUS_REG NCR53C400_register_offset-8/* rw */
-
+/* C400_CONTROL_STATUS_REG */
#define CSR_RESET 0x80/* wo Resets 53c400 */
#define CSR_53C80_REG 0x80/* ro 5380 registers busy */
#define CSR_TRANS_DIR 0x40/* rw Data transfer direction */
@@ -181,16 +180,6 @@
#define CSR_BASE CSR_53C80_INTR
#endif
-/* Number of 128-byte blocks to be transferred */
-#define C400_BLOCK_COUNTER_REG NCR53C400_register_offset-7 /* rw */
-
-/* Resume transfer after disconnect */
-#define C400_RESUME_TRANSFER_REG NCR53C400_register_offset-6 /* wo */
-
-/* Access to host buffer stack */
-#define C400_HOST_BUFFER NCR53C400_register_offset-4 /* rw */
-
-
/* Note : PHASE_* macros are based on the values of the STATUS register */
#define PHASE_MASK (SR_MSG | SR_CD | SR_IO)
diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index a9a237f..ce444da 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -253,6 +253,7 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
};
int flags;
struct Scsi_Host *instance;
+ struct NCR5380_hostdata *hostdata;
#ifdef SCSI_G_NCR5380_MEM
unsigned long base;
void __iomem *iomem;
@@ -395,6 +396,7 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
instance = scsi_register(tpnt, sizeof(struct NCR5380_hostdata));
if (instance == NULL)
goto out_release;
+ hostdata = shost_priv(instance);
#ifndef SCSI_G_NCR5380_MEM
instance->io_port =
overrides[current_override].NCR5380_map_name;
@@ -404,18 +406,27 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
* On NCR53C400 boards, NCR5380 registers are mapped 8 past
* the base address.
*/
- if (overrides[current_override].board == BOARD_NCR53C400)
+ if (overrides[current_override].board == BOARD_NCR53C400) {
instance->io_port += 8;
+ hostdata->c400_ctl_status = 0;
+ hostdata->c400_blk_cnt = 1;
+ hostdata->c400_host_buf = 4;
+ }
#else
instance->base = overrides[current_override].NCR5380_map_name;
- ((struct NCR5380_hostdata *)instance->hostdata)->iomem = iomem;
+ hostdata->iomem = iomem;
+ if (overrides[current_override].board == BOARD_NCR53C400) {
+ hostdata->c400_ctl_status = 0x100;
+ hostdata->c400_blk_cnt = 0x101;
+ hostdata->c400_host_buf = 0x104;
+ }
#endif
if (NCR5380_init(instance, flags))
goto out_unregister;
if (overrides[current_override].board == BOARD_NCR53C400)
- NCR5380_write(C400_CONTROL_STATUS_REG, CSR_BASE);
+ NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
NCR5380_maybe_reset_bus(instance);
@@ -523,30 +534,28 @@ generic_NCR5380_biosparam(struct scsi_device *sdev,
struct block_device *bdev,
static inline int NCR5380_pread(struct Scsi_Host *instance, unsigned char
*dst, int len)
{
-#ifdef SCSI_G_NCR5380_MEM
struct NCR5380_hostdata *hostdata = shost_priv(instance);
-#endif
int blocks = len / 128;
int start = 0;
int bl;
- NCR5380_write(C400_CONTROL_STATUS_REG, CSR_BASE | CSR_TRANS_DIR);
- NCR5380_write(C400_BLOCK_COUNTER_REG, blocks);
+ NCR5380_write(hostdata->c400_ctl_status, CSR_BASE | CSR_TRANS_DIR);
+ NCR5380_write(hostdata->c400_blk_cnt, blocks);
while (1) {
- if ((bl = NCR5380_read(C400_BLOCK_COUNTER_REG)) == 0) {
+ if ((bl = NCR5380_read(hostdata->c400_blk_cnt)) == 0) {
break;
}
- if (NCR5380_read(C400_CONTROL_STATUS_REG) &
CSR_GATED_53C80_IRQ) {
+ if (NCR5380_read(hostdata->c400_ctl_status) &
CSR_GATED_53C80_IRQ) {
printk(KERN_ERR "53C400r: Got 53C80_IRQ start=%d,
blocks=%d\n", s
[RFC PATCH 75/71] ncr5380: Remove FLAG_DTC3181E
The FLAG_DTC3181E is used to activate a work-around for arbitration lost
condition that these chips see when ICR is written during arbitration.
Move the ICR write (to set SEL and BSY) after the arbitration loss check
and remove FLAG_DTC3181E.
Signed-off-by: Ondrej Zary
---
Weird, we now have two consecutive checks for ICR_ARBITRATION_LOST and
do different things when they fail...
drivers/scsi/NCR5380.c | 23 +--
drivers/scsi/NCR5380.h |1 -
drivers/scsi/atari_NCR5380.c | 23 +--
drivers/scsi/dmx3191d.c |2 +-
drivers/scsi/g_NCR5380.c |2 +-
5 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index acb359a..b617f99 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -482,14 +482,13 @@ static void prepare_info(struct Scsi_Host *instance)
"base 0x%lx, irq %d, "
"can_queue %d, cmd_per_lun %d, "
"sg_tablesize %d, this_id %d, "
-"flags { %s%s%s%s}, "
+"flags { %s%s%s}, "
"options { %s} ",
instance->hostt->name, instance->io_port, instance->n_io_port,
instance->base, instance->irq,
instance->can_queue, instance->cmd_per_lun,
instance->sg_tablesize, instance->this_id,
hostdata->flags & FLAG_NO_DMA_FIXUP ? "NO_DMA_FIXUP " : "",
-hostdata->flags & FLAG_DTC3181E ? "DTC3181E " : "",
hostdata->flags & FLAG_NO_PSEUDO_DMA ? "NO_PSEUDO_DMA " : "",
hostdata->flags & FLAG_TOSHIBA_DELAY ? "TOSHIBA_DELAY " : "",
#ifdef AUTOPROBE_IRQ
@@ -1078,18 +1077,7 @@ static struct scsi_cmnd *NCR5380_select(struct Scsi_Host
*instance,
goto out;
}
- /* After/during arbitration, BSY should be asserted.
-* IBM DPES-31080 Version S31Q works now
-* Tnx to thomas_roe...@m2.maus.de for finding this! (Roman)
-*/
- NCR5380_write(INITIATOR_COMMAND_REG,
- ICR_BASE | ICR_ASSERT_SEL | ICR_ASSERT_BSY);
-
- /* RvC: DTC3181E has some trouble with this so we simply removed it.
-* Seems to work with only Mustek scanner attached.
-*/
- if (!(hostdata->flags & FLAG_DTC3181E) &&
- (NCR5380_read(INITIATOR_COMMAND_REG) & ICR_ARBITRATION_LOST)) {
+ if (NCR5380_read(INITIATOR_COMMAND_REG) & ICR_ARBITRATION_LOST) {
NCR5380_write(MODE_REG, MR_BASE);
NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE);
dsprintk(NDEBUG_ARBITRATION, instance, "arbitration lost,
negating SEL\n");
@@ -1097,6 +1085,13 @@ static struct scsi_cmnd *NCR5380_select(struct Scsi_Host
*instance,
goto out;
}
+ /* After/during arbitration, BSY should be asserted.
+* IBM DPES-31080 Version S31Q works now
+* Tnx to thomas_roe...@m2.maus.de for finding this! (Roman)
+*/
+ NCR5380_write(INITIATOR_COMMAND_REG,
+ ICR_BASE | ICR_ASSERT_SEL | ICR_ASSERT_BSY);
+
/*
* Again, bus clear + bus settle time is 1.2us, however, this is
* a minimum so we'll udelay ceil(1.2)
diff --git a/drivers/scsi/NCR5380.h b/drivers/scsi/NCR5380.h
index 0d8ec43..5092580 100644
--- a/drivers/scsi/NCR5380.h
+++ b/drivers/scsi/NCR5380.h
@@ -222,7 +222,6 @@
#define FLAG_NO_DMA_FIXUP 1 /* No DMA errata workarounds */
#define FLAG_NO_PSEUDO_DMA 8 /* Inhibit DMA */
-#define FLAG_DTC3181E 16 /* DTC3181E */
#define FLAG_LATE_DMA_SETUP32 /* Setup NCR before DMA H/W */
#define FLAG_TAGGED_QUEUING64 /* as X3T9.2 spelled it */
#define FLAG_TOSHIBA_DELAY 128 /* Allow for borken CD-ROMs */
diff --git a/drivers/scsi/atari_NCR5380.c b/drivers/scsi/atari_NCR5380.c
index d04374d..7640110 100644
--- a/drivers/scsi/atari_NCR5380.c
+++ b/drivers/scsi/atari_NCR5380.c
@@ -586,13 +586,12 @@ static void prepare_info(struct Scsi_Host *instance)
"base 0x%lx, irq %d, "
"can_queue %d, cmd_per_lun %d, "
"sg_tablesize %d, this_id %d, "
-"flags { %s%s%s}, "
+"flags { %s%s}, "
"options { %s} ",
instance->hostt->name, instance->io_port, instance->n_io_port,
instance->base, instance->irq,
instance->can_queue, instance->cmd_per_lun,
instance->sg_tablesize, instance->this_id,
-hostdata->flags & FLAG_DTC3181E ? "DTC3181E " : "",
hostdata->flags & FLAG_TAGGED_QUEUING ? "TAGGED_QUEUING " : "",
hostdata->flags & FLAG_TOSHIBA_DELAY ? "TOSHIBA_DELAY " : "",
#ifdef DIFFERENTIAL
@@ -1279,18 +1278,7 @@ static struct scsi_cmnd *NCR5380_select(struct Scsi_Host
Re: [PATCH 22/71] ncr5380: Eliminate selecting state
On Sun, 29 Nov 2015, Geert Uytterhoeven wrote:
> This still heavily depends on the processing time spent in
> NCR5380_read(). You should never use a value derived from
> loops_per_jiffy for a non-empty loop,
Sure but the time-out condition isn't supposed to be precise.
Plus/minus a jiffy is no problem. Plus/minus a second is no good.
> as it may take much longer. Always compare with an maximum end time
> instead.
That can't work with interrupts disabled, which was the problem I was
trying to solve by use of loops_per_jiffy.
NCR5380_poll_politely() in mainline doesn't work with interrupts disabled
either, hence patch 21.
>
> E.g.:
>
> end = jiffies + 2;/* 1 jiffie + 1 safeguard */
> do {
> if ((NCR5380_read(reg1) & bit1) == val1)
> return 0;
> cpu_relax();
> } while (time_before(jiffies, end);
>
> And a similar loop for "Busy-wait for up to 20 ms".
Interrupts may be disabled during that loop also. Please refer to (and/or
respond to) patch 21, "ncr5380: Sleep when polling, if possible", in which
these changes were made.
--
>
> Gr{oetje,eeting}s,
>
> Geert
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: block: Always check queue limits for cloned requests
On 2015.11.26 at 08:11 -0500, Mike Snitzer wrote:
> On Thu, Nov 26 2015 at 2:46am -0500,
> Hannes Reinecke wrote:
>
> > When a cloned request is retried on other queues it always needs
> > to be checked against the queue limits of that queue.
> > Otherwise the calculations for nr_phys_segments might be wrong,
> > leading to a crash in scsi_init_sgtable().
> >
> > To clarify this the patch renames blk_rq_check_limits()
> > to blk_cloned_rq_check_limits() and removes the symbol
> > export, as the new function should only be used for
> > cloned requests and never exported.
> >
> > Cc: Mike Snitzer
> > Cc: Ewan Milne
> > Cc: Jeff Moyer
> > Signed-off-by: Hannes Reinecke
>
> Patch looks good. Thanks for getting to the bottom of this.
>
> Jens, please add these extra tags when you pick this up:
>
> Fixes: e2a60da74 ("block: Clean up special command handling logic")
> Cc: sta...@vger.kernel.org # 3.7+
> Acked-by: Mike Snitzer
I'm still seeing the issue (BUG at drivers/scsi/scsi_lib.c:1096!) even
with this patch applied.
markus@x4 linux % git describe
v4.4-rc2-215-g081f3698e606
--
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 22/71] ncr5380: Eliminate selecting state
Hi Finn,
On Sun, Nov 29, 2015 at 11:25 AM, Finn Thain wrote:
> On Sun, 29 Nov 2015, Geert Uytterhoeven wrote:
>> This still heavily depends on the processing time spent in
>> NCR5380_read(). You should never use a value derived from
>> loops_per_jiffy for a non-empty loop,
>
> Sure but the time-out condition isn't supposed to be precise.
> Plus/minus a jiffy is no problem. Plus/minus a second is no good.
If an ISA access takes 8 µs, while the CPU runs at 1 GHz, i.e. 500M loops/s,
the difference will be huge.
>> as it may take much longer. Always compare with an maximum end time
>> instead.
>
> That can't work with interrupts disabled, which was the problem I was
> trying to solve by use of loops_per_jiffy.
Then you indeed can't use jiffies.
Perhaps you can calibrate an NCR5380_read() loop at driver init time,
and use the calibration value later?
> NCR5380_poll_politely() in mainline doesn't work with interrupts disabled
> either, hence patch 21.
>
>> E.g.:
>>
>> end = jiffies + 2;/* 1 jiffie + 1 safeguard */
>> do {
>> if ((NCR5380_read(reg1) & bit1) == val1)
>> return 0;
>> cpu_relax();
>> } while (time_before(jiffies, end);
>>
>> And a similar loop for "Busy-wait for up to 20 ms".
>
> Interrupts may be disabled during that loop also. Please refer to (and/or
> respond to) patch 21, "ncr5380: Sleep when polling, if possible", in which
> these changes were made.
So the above loop may never terminate. Oops...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: block: Always check queue limits for cloned requests
On 11/29/2015 12:49 PM, Markus Trippelsdorf wrote:
> On 2015.11.26 at 08:11 -0500, Mike Snitzer wrote:
>> On Thu, Nov 26 2015 at 2:46am -0500,
>> Hannes Reinecke wrote:
>>
>>> When a cloned request is retried on other queues it always needs
>>> to be checked against the queue limits of that queue.
>>> Otherwise the calculations for nr_phys_segments might be wrong,
>>> leading to a crash in scsi_init_sgtable().
>>>
>>> To clarify this the patch renames blk_rq_check_limits()
>>> to blk_cloned_rq_check_limits() and removes the symbol
>>> export, as the new function should only be used for
>>> cloned requests and never exported.
>>>
>>> Cc: Mike Snitzer
>>> Cc: Ewan Milne
>>> Cc: Jeff Moyer
>>> Signed-off-by: Hannes Reinecke
>>
>> Patch looks good. Thanks for getting to the bottom of this.
>>
>> Jens, please add these extra tags when you pick this up:
>>
>> Fixes: e2a60da74 ("block: Clean up special command handling logic")
>> Cc: sta...@vger.kernel.org # 3.7+
>> Acked-by: Mike Snitzer
>
> I'm still seeing the issue (BUG at drivers/scsi/scsi_lib.c:1096!) even
> with this patch applied.
>
> markus@x4 linux % git describe
> v4.4-rc2-215-g081f3698e606
>
Can you generate a crashdump?
I would need to cross-check with the other dumps I'm having to figure
out if this really is the same issue.
There have been other reports (and fixes) which show we're fighting
several distinct issues here.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: block: Always check queue limits for cloned requests
On 2015.11.29 at 16:43 +0100, Hannes Reinecke wrote:
> On 11/29/2015 12:49 PM, Markus Trippelsdorf wrote:
> > On 2015.11.26 at 08:11 -0500, Mike Snitzer wrote:
> >> On Thu, Nov 26 2015 at 2:46am -0500,
> >> Hannes Reinecke wrote:
> >>
> >>> When a cloned request is retried on other queues it always needs
> >>> to be checked against the queue limits of that queue.
> >>> Otherwise the calculations for nr_phys_segments might be wrong,
> >>> leading to a crash in scsi_init_sgtable().
> >>>
> >>> To clarify this the patch renames blk_rq_check_limits()
> >>> to blk_cloned_rq_check_limits() and removes the symbol
> >>> export, as the new function should only be used for
> >>> cloned requests and never exported.
> >>>
> >>> Cc: Mike Snitzer
> >>> Cc: Ewan Milne
> >>> Cc: Jeff Moyer
> >>> Signed-off-by: Hannes Reinecke
> >>
> >> Patch looks good. Thanks for getting to the bottom of this.
> >>
> >> Jens, please add these extra tags when you pick this up:
> >>
> >> Fixes: e2a60da74 ("block: Clean up special command handling logic")
> >> Cc: sta...@vger.kernel.org # 3.7+
> >> Acked-by: Mike Snitzer
> >
> > I'm still seeing the issue (BUG at drivers/scsi/scsi_lib.c:1096!) even
> > with this patch applied.
> >
> > markus@x4 linux % git describe
> > v4.4-rc2-215-g081f3698e606
> >
> Can you generate a crashdump?
> I would need to cross-check with the other dumps I'm having to figure
> out if this really is the same issue.
> There have been other reports (and fixes) which show we're fighting
> several distinct issues here.
Unfortunately no. The crash happens on the disk where I store my log
files. And after it happened the magic SysRq keys don't work anymore.
The crash only happens on my spinning rust drive that uses the cfq
scheduler. The SSDs (deadline) are fine.
The BUG happens reproducibly when building http://www.sagemath.org/ on
that drive.
--
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: block: Always check queue limits for cloned requests
On Sun, Nov 29 2015 at 11:15am -0500, Markus Trippelsdorf wrote: > On 2015.11.29 at 16:43 +0100, Hannes Reinecke wrote: > > On 11/29/2015 12:49 PM, Markus Trippelsdorf wrote: > > > > > > I'm still seeing the issue (BUG at drivers/scsi/scsi_lib.c:1096!) even > > > with this patch applied. > > > > > > markus@x4 linux % git describe > > > v4.4-rc2-215-g081f3698e606 > > > > > Can you generate a crashdump? > > I would need to cross-check with the other dumps I'm having to figure > > out if this really is the same issue. > > There have been other reports (and fixes) which show we're fighting > > several distinct issues here. > > Unfortunately no. The crash happens on the disk where I store my log > files. And after it happened the magic SysRq keys don't work anymore. > > The crash only happens on my spinning rust drive that uses the cfq > scheduler. The SSDs (deadline) are fine. > > The BUG happens reproducibly when building http://www.sagemath.org/ on > that drive. Are you using DM multipath? If unsure, please let us know which device(s) map to the "spinning rust drive", and provide output from: lsblk -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: block: Always check queue limits for cloned requests
On 2015.11.29 at 11:49 -0500, Mike Snitzer wrote: > On Sun, Nov 29 2015 at 11:15am -0500, > Markus Trippelsdorf wrote: > > > On 2015.11.29 at 16:43 +0100, Hannes Reinecke wrote: > > > On 11/29/2015 12:49 PM, Markus Trippelsdorf wrote: > > > > > > > > I'm still seeing the issue (BUG at drivers/scsi/scsi_lib.c:1096!) even > > > > with this patch applied. > > > > > > > > markus@x4 linux % git describe > > > > v4.4-rc2-215-g081f3698e606 > > > > > > > Can you generate a crashdump? > > > I would need to cross-check with the other dumps I'm having to figure > > > out if this really is the same issue. > > > There have been other reports (and fixes) which show we're fighting > > > several distinct issues here. > > > > Unfortunately no. The crash happens on the disk where I store my log > > files. And after it happened the magic SysRq keys don't work anymore. > > > > The crash only happens on my spinning rust drive that uses the cfq > > scheduler. The SSDs (deadline) are fine. > > > > The BUG happens reproducibly when building http://www.sagemath.org/ on > > that drive. > > Are you using DM multipath? If unsure, please let us know which > device(s) map to the "spinning rust drive", and provide output from: > lsblk No, I'm not using DM multipath. /dev/sdb2 on /var type btrfs (rw,relatime,compress=lzo,noacl,space_cache) /dev/sdb2 btrfs 1.9T 904G 944G 49% /var scsi 1:0:0:0: Direct-Access ATA ST2000DM001-1CH1 CC29 PQ: 0 ANSI: 5 sd 1:0:0:0: [sdb] 3907029168 512-byte logical blocks: (2.00 TB/1.81 TiB) sd 1:0:0:0: [sdb] 4096-byte physical blocks sd 1:0:0:0: [sdb] Write Protect is off sd 1:0:0:0: [sdb] Mode Sense: 00 3a 00 00 sd 1:0:0:0: Attached scsi generic sg1 type 0 sd 1:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA Model Family: Seagate Barracuda 7200.14 (AF) Device Model: ST2000DM001-1CH164 -- Markus -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] target: use offset_in_page macro
Hi Geliang,
On Wed, 2015-11-25 at 21:49 +0800, Geliang Tang wrote:
> Use offset_in_page macro instead of (addr & ~PAGE_MASK).
>
> Signed-off-by: Geliang Tang
> ---
> drivers/target/target_core_user.c | 2 +-
> drivers/target/tcm_fc/tfc_io.c| 8
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/target/target_core_user.c
> b/drivers/target/target_core_user.c
> index 937cebf..d5477c0 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -194,7 +194,7 @@ static struct tcmu_cmd *tcmu_alloc_cmd(struct se_cmd
> *se_cmd)
>
> static inline void tcmu_flush_dcache_range(void *vaddr, size_t size)
> {
> - unsigned long offset = (unsigned long) vaddr & ~PAGE_MASK;
> + unsigned long offset = offset_in_page(vaddr);
>
> size = round_up(size+offset, PAGE_SIZE);
> vaddr -= offset;
> diff --git a/drivers/target/tcm_fc/tfc_io.c b/drivers/target/tcm_fc/tfc_io.c
> index 847c1aa..6f7c65a 100644
> --- a/drivers/target/tcm_fc/tfc_io.c
> +++ b/drivers/target/tcm_fc/tfc_io.c
> @@ -154,9 +154,9 @@ int ft_queue_data_in(struct se_cmd *se_cmd)
> BUG_ON(!page);
> from = kmap_atomic(page + (mem_off >> PAGE_SHIFT));
> page_addr = from;
> - from += mem_off & ~PAGE_MASK;
> + from += offset_in_page(mem_off);
> tlen = min(tlen, (size_t)(PAGE_SIZE -
> - (mem_off & ~PAGE_MASK)));
> + offset_in_page(mem_off)));
> memcpy(to, from, tlen);
> kunmap_atomic(page_addr);
> to += tlen;
> @@ -314,9 +314,9 @@ void ft_recv_write_data(struct ft_cmd *cmd, struct
> fc_frame *fp)
>
> to = kmap_atomic(page + (mem_off >> PAGE_SHIFT));
> page_addr = to;
> - to += mem_off & ~PAGE_MASK;
> + to += offset_in_page(mem_off);
> tlen = min(tlen, (size_t)(PAGE_SIZE -
> - (mem_off & ~PAGE_MASK)));
> + offset_in_page(mem_off)));
> memcpy(to, from, tlen);
> kunmap_atomic(page_addr);
>
Applied to target-pending/for-next.
Thank you,
--nab
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 22/71] ncr5380: Eliminate selecting state
On Sun, 29 Nov 2015, Geert Uytterhoeven wrote: > If an ISA access takes 8 us, while the CPU runs at 1 GHz, i.e. 500M > loops/s, the difference will be huge. Ondrej showed that an ISA access can take about 1.6 us. I don't know what to make of the "8 uS" comment in the mainline driver. Maybe it was an even slower ISA card. Anyway, I made a measurement on my hardware and confirmed that lpj is a very bad proxy for device register access throughput. The "loops per access" gap is several orders of magnitude: lpj HZaccess time (us) lpa Ondrej's 5380 ISA card (PC): 4797252 250 1.6 1.9k My DMX3191D PCI card (PowerMac): 167079 100 0.42 7.0 > > Perhaps you can calibrate an NCR5380_read() loop at driver init time, > and use the calibration value later? I had the same idea but I didn't think that the complexity was justified by the low precision requirement. But now that I have some timings I have to agree. -- -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 75/71] ncr5380: Remove FLAG_DTC3181E
On Sun, 29 Nov 2015, Ondrej Zary wrote:
> The FLAG_DTC3181E is used to activate a work-around for arbitration lost
> condition that these chips see when ICR is written during arbitration.
>
> Move the ICR write (to set SEL and BSY) after the arbitration loss check
> and remove FLAG_DTC3181E.
The first test for ICR_ARBITRATION_LOST happens after the required
arbitration delay, 2.4 us. The second test for ICR_ARBITRATION_LOST
happens after ICR_ASSERT_SEL.
This second test seems to be pointless. It comes from the flow chart in
the NCR datasheet (see download link in patch 17). The spec does not
require this test but some 5380 devices may do. Who knows? It's almost
impossible to be sure, because it would mean losing a race with another
bus device right at the end of the arbitration delay (and we extend that
delay to 3 us anyway).
Certainly one can find other datasheets with sample code and flow charts
that don't do this second check. The reason is that ICR_ARBITRATION_LOST
can be triggered when SEL is asserted by any device, so it may be
triggered after we've won arbitration (because we then set ICR_ASSERT_SEL
ourselves in order to enter selection phase).
>
> ... Weird, we now have two consecutive checks for ICR_ARBITRATION_LOST
> and do different things when they fail...
They do different things because the second exit has to cleanup after the
ICR write.
I agree that it would be nice to remove the DTC3181E special case. It
would mean replacing patch 49.
The patch below is another version of your patch 75. It really needs to be
tested on all kinds of 5380 device, and if possible with a contested bus
(which would imply diconnection privileges, for which the driver still
requires that the chip has a working irq).
Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c 2015-11-30 15:34:39.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-11-30 15:34:39.0 +1100
@@ -482,14 +482,13 @@ static void prepare_info(struct Scsi_Hos
"base 0x%lx, irq %d, "
"can_queue %d, cmd_per_lun %d, "
"sg_tablesize %d, this_id %d, "
-"flags { %s%s%s%s}, "
+"flags { %s%s%s}, "
"options { %s} ",
instance->hostt->name, instance->io_port, instance->n_io_port,
instance->base, instance->irq,
instance->can_queue, instance->cmd_per_lun,
instance->sg_tablesize, instance->this_id,
hostdata->flags & FLAG_NO_DMA_FIXUP ? "NO_DMA_FIXUP " : "",
-hostdata->flags & FLAG_DTC3181E ? "DTC3181E " : "",
hostdata->flags & FLAG_NO_PSEUDO_DMA ? "NO_PSEUDO_DMA " : "",
hostdata->flags & FLAG_TOSHIBA_DELAY ? "TOSHIBA_DELAY " : "",
#ifdef AUTOPROBE_IRQ
@@ -1085,18 +1084,6 @@ static struct scsi_cmnd *NCR5380_select(
NCR5380_write(INITIATOR_COMMAND_REG,
ICR_BASE | ICR_ASSERT_SEL | ICR_ASSERT_BSY);
- /* RvC: DTC3181E has some trouble with this so we simply removed it.
-* Seems to work with only Mustek scanner attached.
-*/
- if (!(hostdata->flags & FLAG_DTC3181E) &&
- (NCR5380_read(INITIATOR_COMMAND_REG) & ICR_ARBITRATION_LOST)) {
- NCR5380_write(MODE_REG, MR_BASE);
- NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE);
- dsprintk(NDEBUG_ARBITRATION, instance, "arbitration lost,
negating SEL\n");
- spin_lock_irq(&hostdata->lock);
- goto out;
- }
-
/*
* Again, bus clear + bus settle time is 1.2us, however, this is
* a minimum so we'll udelay ceil(1.2)
Index: linux/drivers/scsi/NCR5380.h
===
--- linux.orig/drivers/scsi/NCR5380.h 2015-11-30 15:34:36.0 +1100
+++ linux/drivers/scsi/NCR5380.h2015-11-30 15:34:39.0 +1100
@@ -233,7 +233,6 @@
#define FLAG_NO_DMA_FIXUP 1 /* No DMA errata workarounds */
#define FLAG_NO_PSEUDO_DMA 8 /* Inhibit DMA */
-#define FLAG_DTC3181E 16 /* DTC3181E */
#define FLAG_LATE_DMA_SETUP32 /* Setup NCR before DMA H/W */
#define FLAG_TAGGED_QUEUING64 /* as X3T9.2 spelled it */
#define FLAG_TOSHIBA_DELAY 128 /* Allow for borken CD-ROMs */
Index: linux/drivers/scsi/atari_NCR5380.c
===
--- linux.orig/drivers/scsi/atari_NCR5380.c 2015-11-30 15:34:39.0
+1100
+++ linux/drivers/scsi/atari_NCR5380.c 2015-11-30 15:34:39.0 +1100
@@ -586,13 +586,12 @@ static void prepare_info(struct Scsi_Hos
"base 0x%lx, irq %d, "
"can_queue %d, cmd_per_lun %d, "
"sg_tablesize %d, this_id %d, "
-
Re: block: Always check queue limits for cloned requests
On Sun, 29 Nov 2015 18:05:06 +0100 Markus Trippelsdorf wrote: > On 2015.11.29 at 11:49 -0500, Mike Snitzer wrote: > > On Sun, Nov 29 2015 at 11:15am -0500, > > Markus Trippelsdorf wrote: > > > > > On 2015.11.29 at 16:43 +0100, Hannes Reinecke wrote: > > > > On 11/29/2015 12:49 PM, Markus Trippelsdorf wrote: > > > > > > > > > > I'm still seeing the issue (BUG at drivers/scsi/scsi_lib.c:1096!) even > > > > > with this patch applied. > > > > > > > > > > markus@x4 linux % git describe > > > > > v4.4-rc2-215-g081f3698e606 > > > > > > > > > Can you generate a crashdump? > > > > I would need to cross-check with the other dumps I'm having to figure > > > > out if this really is the same issue. > > > > There have been other reports (and fixes) which show we're fighting > > > > several distinct issues here. > > > > > > Unfortunately no. The crash happens on the disk where I store my log > > > files. And after it happened the magic SysRq keys don't work anymore. > > > > > > The crash only happens on my spinning rust drive that uses the cfq > > > scheduler. The SSDs (deadline) are fine. > > > > > > The BUG happens reproducibly when building http://www.sagemath.org/ on > > > that drive. > > > > Are you using DM multipath? If unsure, please let us know which > > device(s) map to the "spinning rust drive", and provide output from: > > lsblk > > No, I'm not using DM multipath. OK, I guess it is still one block merge issue, care to test the following patch? The patch can address one issue when bio->bi_seg_front_size is set as too small mistakenly, then fewer physical segment may be figured out. --- >From 7aa725205f400ee6823a0d19bf9f41a2464725ce Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Mon, 30 Nov 2015 13:10:12 +0800 Subject: [PATCH] blk-merge: fix computing bio->bi_seg_front_size in case of single segment When bio has only one physical segment, we should set bio's bi_seg_front_size as the real(final) size of the single segment. Fixes: 02e707424c2ea(blk-merge: fix blk_bio_segment_split) Reported-by: Markus Trippelsdorf Signed-off-by: Ming Lei --- block/blk-merge.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/blk-merge.c b/block/blk-merge.c index 41a55ba..e01405a 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -103,6 +103,9 @@ static struct bio *blk_bio_segment_split(struct request_queue *q, bvprv = bv; bvprvp = &bvprv; sectors += bv.bv_len >> 9; + + if (nsegs == 1 && seg_size > front_seg_size) + front_seg_size = seg_size; continue; } new_segment: -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: ufs: fix spelling mistake in error message
Zitat von Colin King :
From: Colin Ian King
Minor issue, fix spelling mistake, Intialization -> Initialization
Signed-off-by: Colin Ian King
---
drivers/scsi/ufs/ufshcd-pltfrm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c
b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 9714f2a..d2a7b12 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -333,7 +333,7 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
err = ufshcd_init(hba, mmio_base, irq);
if (err) {
- dev_err(dev, "Intialization failed\n");
+ dev_err(dev, "Initialization failed\n");
goto out_disable_rpm;
}
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Reviewed-by: Johannes Thumshirn
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: block: Always check queue limits for cloned requests
On 11/29/2015 06:05 PM, Markus Trippelsdorf wrote: > On 2015.11.29 at 11:49 -0500, Mike Snitzer wrote: >> On Sun, Nov 29 2015 at 11:15am -0500, >> Markus Trippelsdorf wrote: >> >>> On 2015.11.29 at 16:43 +0100, Hannes Reinecke wrote: On 11/29/2015 12:49 PM, Markus Trippelsdorf wrote: > > I'm still seeing the issue (BUG at drivers/scsi/scsi_lib.c:1096!) even > with this patch applied. > > markus@x4 linux % git describe > v4.4-rc2-215-g081f3698e606 > Can you generate a crashdump? I would need to cross-check with the other dumps I'm having to figure out if this really is the same issue. There have been other reports (and fixes) which show we're fighting several distinct issues here. >>> >>> Unfortunately no. The crash happens on the disk where I store my log >>> files. And after it happened the magic SysRq keys don't work anymore. >>> >>> The crash only happens on my spinning rust drive that uses the cfq >>> scheduler. The SSDs (deadline) are fine. >>> >>> The BUG happens reproducibly when building http://www.sagemath.org/ on >>> that drive. >> >> Are you using DM multipath? If unsure, please let us know which >> device(s) map to the "spinning rust drive", and provide output from: >> lsblk > > No, I'm not using DM multipath. > > /dev/sdb2 on /var type btrfs (rw,relatime,compress=lzo,noacl,space_cache) > /dev/sdb2 btrfs 1.9T 904G 944G 49% /var > > scsi 1:0:0:0: Direct-Access ATA ST2000DM001-1CH1 CC29 PQ: 0 ANSI: 5 > sd 1:0:0:0: [sdb] 3907029168 512-byte logical blocks: (2.00 TB/1.81 TiB) > sd 1:0:0:0: [sdb] 4096-byte physical blocks > sd 1:0:0:0: [sdb] Write Protect is off > sd 1:0:0:0: [sdb] Mode Sense: 00 3a 00 00 > sd 1:0:0:0: Attached scsi generic sg1 type 0 > sd 1:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support > DPO or FUA > > Model Family: Seagate Barracuda 7200.14 (AF) > Device Model: ST2000DM001-1CH164 > As Ming Lei indicated, this is probably a different issue. My patch is for fixing multipath-failover induced I/O errors only. So if you're not using multipath you won't be affected, neither by the original issue triggering the BUG_ON nor my patch attempting to fix it. Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: block: Always check queue limits for cloned requests
On 2015.11.30 at 14:11 +0800, Ming Lei wrote: > On Sun, 29 Nov 2015 18:05:06 +0100 > Markus Trippelsdorf wrote: > > > > No, I'm not using DM multipath. > > > OK, I guess it is still one block merge issue, care to test the > following patch? > > The patch can address one issue when bio->bi_seg_front_size > is set as too small mistakenly, then fewer physical segment may > be figured out. Many thanks. Your patch fixes the issue for me. -- Markus -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
