* [PATCH 3/6] libata: remove the done allback from ata_scsi_args
From: Christoph Hellwig @ 2017-01-10 8:41 UTC (permalink / raw)
To: tj; +Cc: geert, linux-ide
In-Reply-To: <1484037708-654-1-git-send-email-hch@lst.de>
It's always the scsi_done callback, and we can get at that easily
in the place where ->done is called.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/ata/libata-scsi.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 43694f5..28e2530 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2061,7 +2061,6 @@ struct ata_scsi_args {
struct ata_device *dev;
u16 *id;
struct scsi_cmnd *cmd;
- void (*done)(struct scsi_cmnd *);
};
/**
@@ -2140,7 +2139,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
if (rc == 0)
cmd->result = SAM_STAT_GOOD;
- args->done(cmd);
+ cmd->scsi_done(cmd);
}
/**
@@ -4357,7 +4356,6 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
args.dev = dev;
args.id = dev->id;
args.cmd = cmd;
- args.done = cmd->scsi_done;
switch(scsicmd[0]) {
case INQUIRY:
--
2.1.4
^ permalink raw reply related
* [PATCH 4/6] libata: call ->scsi_done from ata_scsi_simulate
From: Christoph Hellwig @ 2017-01-10 8:41 UTC (permalink / raw)
To: tj; +Cc: geert, linux-ide
In-Reply-To: <1484037708-654-1-git-send-email-hch@lst.de>
We always need to call ->scsi_done after we've finished emulating a
command, so do it in a single place at the end of ata_scsi_simulate.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/ata/libata-scsi.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 28e2530..6078bc2 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -484,13 +484,6 @@ struct device_attribute *ata_common_sdev_attrs[] = {
};
EXPORT_SYMBOL_GPL(ata_common_sdev_attrs);
-static void ata_scsi_invalid_field(struct ata_device *dev,
- struct scsi_cmnd *cmd, u16 field)
-{
- ata_scsi_set_invalid_field(dev, cmd, field, 0xff);
- cmd->scsi_done(cmd);
-}
-
/**
* ata_std_bios_param - generic bios head/sector/cylinder calculator used by sd.
* @sdev: SCSI device for which BIOS geometry is to be determined
@@ -2139,7 +2132,6 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
if (rc == 0)
cmd->result = SAM_STAT_GOOD;
- cmd->scsi_done(cmd);
}
/**
@@ -4360,7 +4352,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
switch(scsicmd[0]) {
case INQUIRY:
if (scsicmd[1] & 2) /* is CmdDt set? */
- ata_scsi_invalid_field(dev, cmd, 1);
+ ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
else if ((scsicmd[1] & 1) == 0) /* is EVPD clear? */
ata_scsi_rbuf_fill(&args, ata_scsiop_inq_std);
else switch (scsicmd[2]) {
@@ -4392,7 +4384,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
}
/* Fallthrough */
default:
- ata_scsi_invalid_field(dev, cmd, 2);
+ ata_scsi_set_invalid_field(dev, cmd, 2, 0xff);
break;
}
break;
@@ -4410,7 +4402,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
if ((scsicmd[1] & 0x1f) == SAI_READ_CAPACITY_16)
ata_scsi_rbuf_fill(&args, ata_scsiop_read_cap);
else
- ata_scsi_invalid_field(dev, cmd, 1);
+ ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
break;
case REPORT_LUNS:
@@ -4420,7 +4412,6 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
case REQUEST_SENSE:
ata_scsi_set_sense(dev, cmd, 0, 0, 0);
cmd->result = (DRIVER_SENSE << 24);
- cmd->scsi_done(cmd);
break;
/* if we reach this, then writeback caching is disabled,
@@ -4442,23 +4433,24 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
if ((tmp8 == 0x4) && (!scsicmd[3]) && (!scsicmd[4]))
ata_scsi_rbuf_fill(&args, ata_scsiop_noop);
else
- ata_scsi_invalid_field(dev, cmd, 1);
+ ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
break;
case MAINTENANCE_IN:
if (scsicmd[1] == MI_REPORT_SUPPORTED_OPERATION_CODES)
ata_scsi_rbuf_fill(&args, ata_scsiop_maint_in);
else
- ata_scsi_invalid_field(dev, cmd, 1);
+ ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
break;
/* all other commands */
default:
ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x20, 0x0);
/* "Invalid command operation code" */
- cmd->scsi_done(cmd);
break;
}
+
+ cmd->scsi_done(cmd);
}
int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
--
2.1.4
^ permalink raw reply related
* [PATCH 5/6] libata: don't call ata_scsi_rbuf_fill for command without a response buffer
From: Christoph Hellwig @ 2017-01-10 8:41 UTC (permalink / raw)
To: tj; +Cc: geert, linux-ide
In-Reply-To: <1484037708-654-1-git-send-email-hch@lst.de>
No need to copy a zeroed buffer to the caller if the command is defined
to not have a response in the SCSI spec.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/ata/libata-scsi.c | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 6078bc2..395c859 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2453,23 +2453,6 @@ static unsigned int ata_scsiop_inq_b6(struct ata_scsi_args *args, u8 *rbuf)
}
/**
- * ata_scsiop_noop - Command handler that simply returns success.
- * @args: device IDENTIFY data / SCSI command of interest.
- * @rbuf: Response buffer, to which simulated SCSI cmd output is sent.
- *
- * No operation. Simply returns success to caller, to indicate
- * that the caller should successfully complete this SCSI command.
- *
- * LOCKING:
- * spin_lock_irqsave(host lock)
- */
-static unsigned int ata_scsiop_noop(struct ata_scsi_args *args, u8 *rbuf)
-{
- VPRINTK("ENTER\n");
- return 0;
-}
-
-/**
* modecpy - Prepare response for MODE SENSE
* @dest: output buffer
* @src: data being copied
@@ -4425,14 +4408,11 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
case SEEK_6:
case SEEK_10:
case TEST_UNIT_READY:
- ata_scsi_rbuf_fill(&args, ata_scsiop_noop);
break;
case SEND_DIAGNOSTIC:
tmp8 = scsicmd[1] & ~(1 << 3);
- if ((tmp8 == 0x4) && (!scsicmd[3]) && (!scsicmd[4]))
- ata_scsi_rbuf_fill(&args, ata_scsiop_noop);
- else
+ if (tmp8 != 0x4 || scsicmd[3] || scsicmd[4])
ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
break;
--
2.1.4
^ permalink raw reply related
* [PATCH 6/6] libata: switch to dynamic allocation insted of ata_scsi_rbuf
From: Christoph Hellwig @ 2017-01-10 8:41 UTC (permalink / raw)
To: tj; +Cc: geert, linux-ide
In-Reply-To: <1484037708-654-1-git-send-email-hch@lst.de>
Note of the emulated commands in the pageout/pagein path, so just do
a GFP_NOIO dynamic allocation.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/ata/libata-scsi.c | 122 ++++++++++++++--------------------------------
1 file changed, 36 insertions(+), 86 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 395c859..785eb382 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -57,9 +57,6 @@
#define ATA_SCSI_RBUF_SIZE 4096
-static DEFINE_SPINLOCK(ata_scsi_rbuf_lock);
-static u8 ata_scsi_rbuf[ATA_SCSI_RBUF_SIZE];
-
typedef unsigned int (*ata_xlat_func_t)(struct ata_queued_cmd *qc);
static struct ata_device *__ata_scsi_find_dev(struct ata_port *ap,
@@ -2057,53 +2054,6 @@ struct ata_scsi_args {
};
/**
- * ata_scsi_rbuf_get - Map response buffer.
- * @cmd: SCSI command containing buffer to be mapped.
- * @flags: unsigned long variable to store irq enable status
- * @copy_in: copy in from user buffer
- *
- * Prepare buffer for simulated SCSI commands.
- *
- * LOCKING:
- * spin_lock_irqsave(ata_scsi_rbuf_lock) on success
- *
- * RETURNS:
- * Pointer to response buffer.
- */
-static void *ata_scsi_rbuf_get(struct scsi_cmnd *cmd, bool copy_in,
- unsigned long *flags)
-{
- spin_lock_irqsave(&ata_scsi_rbuf_lock, *flags);
-
- memset(ata_scsi_rbuf, 0, ATA_SCSI_RBUF_SIZE);
- if (copy_in)
- sg_copy_to_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
- ata_scsi_rbuf, ATA_SCSI_RBUF_SIZE);
- return ata_scsi_rbuf;
-}
-
-/**
- * ata_scsi_rbuf_put - Unmap response buffer.
- * @cmd: SCSI command containing buffer to be unmapped.
- * @copy_out: copy out result
- * @flags: @flags passed to ata_scsi_rbuf_get()
- *
- * Returns rbuf buffer. The result is copied to @cmd's buffer if
- * @copy_back is true.
- *
- * LOCKING:
- * Unlocks ata_scsi_rbuf_lock.
- */
-static inline void ata_scsi_rbuf_put(struct scsi_cmnd *cmd, bool copy_out,
- unsigned long *flags)
-{
- if (copy_out)
- sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
- ata_scsi_rbuf, ATA_SCSI_RBUF_SIZE);
- spin_unlock_irqrestore(&ata_scsi_rbuf_lock, *flags);
-}
-
-/**
* ata_scsi_rbuf_fill - wrapper for SCSI command simulators
* @args: device IDENTIFY data / SCSI command of interest.
* @actor: Callback hook for desired SCSI command simulator
@@ -2121,17 +2071,22 @@ static inline void ata_scsi_rbuf_put(struct scsi_cmnd *cmd, bool copy_out,
static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
unsigned int (*actor)(struct ata_scsi_args *args, u8 *rbuf))
{
- u8 *rbuf;
- unsigned int rc;
struct scsi_cmnd *cmd = args->cmd;
- unsigned long flags;
+ u8 *buf;
- rbuf = ata_scsi_rbuf_get(cmd, false, &flags);
- rc = actor(args, rbuf);
- ata_scsi_rbuf_put(cmd, rc == 0, &flags);
+ buf = kzalloc(ATA_SCSI_RBUF_SIZE, GFP_NOIO);
+ if (!buf) {
+ ata_scsi_set_sense(args->dev, cmd, NOT_READY, 0x08, 0);
+ return;
+ }
- if (rc == 0)
+ if (actor(args, buf) == 0) {
+ sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
+ buf, ATA_SCSI_RBUF_SIZE);
cmd->result = SAM_STAT_GOOD;
+ }
+
+ kfree(buf);
}
/**
@@ -3363,24 +3318,17 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
*
* Return: Number of bytes copied into sglist.
*/
-static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
+static ssize_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
u64 sector, u32 count)
{
- struct scsi_device *sdp = cmd->device;
- size_t len = sdp->sector_size;
size_t r;
__le64 *buf;
u32 i = 0;
- unsigned long flags;
- WARN_ON(len > ATA_SCSI_RBUF_SIZE);
-
- if (len > ATA_SCSI_RBUF_SIZE)
- len = ATA_SCSI_RBUF_SIZE;
+ buf = kzalloc(cmd->device->sector_size, GFP_NOFS);
+ if (!buf)
+ return -ENOMEM;
- spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
- buf = ((void *)ata_scsi_rbuf);
- memset(buf, 0, len);
while (i < trmax) {
u64 entry = sector |
((u64)(count > 0xffff ? 0xffff : count) << 48);
@@ -3390,9 +3338,9 @@ static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
count -= 0xffff;
sector += 0xffff;
}
- r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, len);
- spin_unlock_irqrestore(&ata_scsi_rbuf_lock, flags);
-
+ r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf,
+ cmd->device->sector_size);
+ kfree(buf);
return r;
}
@@ -3408,16 +3356,15 @@ static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
*
* Return: Number of bytes copied into sglist.
*/
-static size_t ata_format_sct_write_same(struct scsi_cmnd *cmd, u64 lba, u64 num)
+static ssize_t ata_format_sct_write_same(struct scsi_cmnd *cmd, u64 lba,
+ u64 num)
{
- struct scsi_device *sdp = cmd->device;
- size_t len = sdp->sector_size;
size_t r;
u16 *buf;
- unsigned long flags;
- spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
- buf = ((void *)ata_scsi_rbuf);
+ buf = kzalloc(cmd->device->sector_size, GFP_NOIO);
+ if (!buf)
+ return -ENOMEM;
put_unaligned_le16(0x0002, &buf[0]); /* SCT_ACT_WRITE_SAME */
put_unaligned_le16(0x0101, &buf[1]); /* WRITE PTRN FG */
@@ -3425,14 +3372,9 @@ static size_t ata_format_sct_write_same(struct scsi_cmnd *cmd, u64 lba, u64 num)
put_unaligned_le64(num, &buf[6]);
put_unaligned_le32(0u, &buf[10]); /* pattern */
- WARN_ON(len > ATA_SCSI_RBUF_SIZE);
-
- if (len > ATA_SCSI_RBUF_SIZE)
- len = ATA_SCSI_RBUF_SIZE;
-
- r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, len);
- spin_unlock_irqrestore(&ata_scsi_rbuf_lock, flags);
-
+ r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf,
+ cmd->device->sector_size);
+ kfree(buf);
return r;
}
@@ -3451,7 +3393,7 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
struct ata_taskfile *tf = &qc->tf;
struct scsi_cmnd *scmd = qc->scsicmd;
struct scsi_device *sdp = scmd->device;
- size_t len = sdp->sector_size;
+ ssize_t len = sdp->sector_size;
struct ata_device *dev = qc->dev;
const u8 *cdb = scmd->cmnd;
u64 block;
@@ -3508,6 +3450,8 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
*/
if (unmap) {
size = ata_format_dsm_trim_descr(scmd, trmax, block, n_block);
+ if (size < 0)
+ goto comm_fail;
if (size != len)
goto invalid_param_len;
@@ -3531,6 +3475,8 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
}
} else {
size = ata_format_sct_write_same(scmd, block, n_block);
+ if (size < 0)
+ goto comm_fail;
if (size != len)
goto invalid_param_len;
@@ -3569,6 +3515,10 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
/* "Invalid command operation code" */
ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST, 0x20, 0x0);
return 1;
+comm_fail:
+ /* "Logical unit communication failure" */
+ ata_scsi_set_sense(dev, scmd, NOT_READY, 0x08, 0);
+ return 1;
}
/**
--
2.1.4
^ permalink raw reply related
* Help Desk Email User
From: канц Ін-т імпульсн.проц.і технол. @ 2017-01-10 10:44 UTC (permalink / raw)
To: ian.falconer@adelaide.edu.au
Your e-mailbox password will soon expire. To keep your password active. Click Here<http://rtj1.weebly.com/> to update
Thank you,
Help Desk 2017©
^ permalink raw reply
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
From: Bartlomiej Zolnierkiewicz @ 2017-01-10 12:53 UTC (permalink / raw)
To: Michael Schmitz
Cc: Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k,
Linux Kernel Development
In-Reply-To: <CAOmrzkJP6s34DPOHd4OFFx=CXko35Kt=CfeDZthOPvKDJy-9YA@mail.gmail.com>
Hi,
On Friday, January 06, 2017 10:01:49 AM Michael Schmitz wrote:
> Hi Bartlomiej,
>
> thanks for caring to support our legacy PATA systems!
>
> On Sat, Dec 31, 2016 at 3:01 AM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> > Hi,
> >
> > This patchset adds m68k/Atari Falcon PATA support to libata.
> > The major difference in the new libata's pata_falcon host
> > driver when compared to legacy IDE's falconide host driver is
> > that we are using polled PIO mode and thus avoiding the need
> > for STDMA locking magic altogether.
>
> I don't suppose this is the default libata mode for PIO?
No, by default it is used only for some commands (i.e. IDENTIFY,
SET FEATURES - XFER MODE).
> How is polling implemented in libata? Sleeping for something
> approximating the average seek latency shouldn't hurt but spinning
> wont be acceptable for a low performance single CPU architecture like
> the Falcon.
You can find actual implementation in libata-sff.c.
Please see ata_sff_pio_task() for the main polling logic:
fsm_start:
WARN_ON_ONCE(ap->hsm_task_state == HSM_ST_IDLE);
/*
* This is purely heuristic. This is a fast path.
* Sometimes when we enter, BSY will be cleared in
* a chk-status or two. If not, the drive is probably seeking
* or something. Snooze for a couple msecs, then
* chk-status again. If still busy, queue delayed work.
*/
status = ata_sff_busy_wait(ap, ATA_BUSY, 5);
if (status & ATA_BUSY) {
spin_unlock_irq(ap->lock);
ata_msleep(ap, 2);
spin_lock_irq(ap->lock);
status = ata_sff_busy_wait(ap, ATA_BUSY, 10);
if (status & ATA_BUSY) {
ata_sff_queue_pio_task(link, ATA_SHORT_PAUSE);
goto out_unlock;
}
}
/*
* hsm_move() may trigger another command to be processed.
* clean the link beforehand.
*/
ap->sff_pio_task_link = NULL;
/* move the HSM */
poll_next = ata_sff_hsm_move(ap, qc, status, 1);
/* another command or interrupt handler
* may be running at this point.
*/
if (poll_next)
goto fsm_start;
out_unlock:
ata_sff_busy_wait()'s last argument is number of 10uS waits so
first check (50uS) should be quite quick. If device is still
busy we sleep for 2ms. Then we quickly (100uS) busy wait and
if needed queue delayed work (with ATA_SHORT_PAUSE == 16 ms
delay).
Overall the current heuristic looks fine and spinning should be
minimal.
> > Tested under ARAnyM emulator.
>
> Not sure that the emulator is really feature complete enough - I'll
> get this tested on my Falcon in the next few weeks. I'm a bit worried
Great!
> about IDE still generating interrupts on seek completion (did you spot
> anything like that, Geert?).
>
> Cheers,
>
> Michael
BTW according to comment in arch/m68k/atari/stdma.c:
/* On the Falcon, the IDE bus uses just the ACSI/Floppy interrupt, but */
/* not the ST-DMA chip itself. So falhd.c needs not to lock the */
/* chip. The interrupt is routed to falhd.c if IDE is configured, the */
/* model is a Falcon and the interrupt was caused by the HD controller */
/* (can be determined by looking at its status register). */
it should be okay to use IDE at the same time as SCSI/Floppy which
is what the new driver does (the old one is serialized operations by
ST-DMA related IRQ handling magic).
Also the comment itself may need some fixups as on Falcon it is SCSI
not ACSI (according to the earlier comment in same file) and the old
IDE host driver name is not falhd.c but falconide.c.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH 6/6] libata: switch to dynamic allocation insted of ata_scsi_rbuf
From: Christoph Hellwig @ 2017-01-10 14:40 UTC (permalink / raw)
To: tj; +Cc: geert, linux-ide
In-Reply-To: <1484037708-654-7-git-send-email-hch@lst.de>
> struct scsi_cmnd *scmd = qc->scsicmd;
> struct scsi_device *sdp = scmd->device;
> - size_t len = sdp->sector_size;
> + ssize_t len = sdp->sector_size;
Julia Lawall's magic checker tool pointed out that len doesn't have
to change to ssize_t here, but the size variable should instead so that
we can properly handle the ENOMEM case below.
I'll wait for some more feedback on the patches and will eventually
resend with that fix.
^ permalink raw reply
* Re: libata: remove the global response buffer
From: Tejun Heo @ 2017-01-10 15:56 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: geert, linux-ide
In-Reply-To: <1484037708-654-1-git-send-email-hch@lst.de>
On Tue, Jan 10, 2017 at 09:41:42AM +0100, Christoph Hellwig wrote:
> Avoid the need for a global 4k buffer by allocating no response buffer
> for commands that don't return data, a small on-stack one for the ATAPI
> fixup, or a kmalloced one for all other emulated commands.
Applied 1-5 to libata/for-4.11. Will wait for the updated version of
the last patch.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH 1/1] libata: Fix ATA request sense
From: Tejun Heo @ 2017-01-10 15:59 UTC (permalink / raw)
To: Damien Le Moal; +Cc: linux-ide, Hannes Reinecke
In-Reply-To: <4ecc1395-3ac9-3381-3a90-b8fb58b46923@wdc.com>
On Tue, Jan 10, 2017 at 09:54:44AM +0900, Damien Le Moal wrote:
> Should we send this patch to stable too ?
> This bug does break applications on kernels 4.7 to 4.9 (e.g. libzbc test
> suite and many tools we have using SGIO to directly send ATA commands to
> SATA disks). I suspect sg-utils and hdparm may be affected in some way too.
Already marked for stable.
Thanks.
--
tejun
^ permalink raw reply
* [PATCH v2] libata: switch to dynamic allocation insted of ata_scsi_rbuf
From: Christoph Hellwig @ 2017-01-10 16:04 UTC (permalink / raw)
To: tj; +Cc: geert, linux-ide
Note of the emulated commands in the pageout/pagein path, so just do
a GFP_NOIO dynamic allocation.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/ata/libata-scsi.c | 122 ++++++++++++++--------------------------------
1 file changed, 36 insertions(+), 86 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 395c859..4de273b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -57,9 +57,6 @@
#define ATA_SCSI_RBUF_SIZE 4096
-static DEFINE_SPINLOCK(ata_scsi_rbuf_lock);
-static u8 ata_scsi_rbuf[ATA_SCSI_RBUF_SIZE];
-
typedef unsigned int (*ata_xlat_func_t)(struct ata_queued_cmd *qc);
static struct ata_device *__ata_scsi_find_dev(struct ata_port *ap,
@@ -2057,53 +2054,6 @@ struct ata_scsi_args {
};
/**
- * ata_scsi_rbuf_get - Map response buffer.
- * @cmd: SCSI command containing buffer to be mapped.
- * @flags: unsigned long variable to store irq enable status
- * @copy_in: copy in from user buffer
- *
- * Prepare buffer for simulated SCSI commands.
- *
- * LOCKING:
- * spin_lock_irqsave(ata_scsi_rbuf_lock) on success
- *
- * RETURNS:
- * Pointer to response buffer.
- */
-static void *ata_scsi_rbuf_get(struct scsi_cmnd *cmd, bool copy_in,
- unsigned long *flags)
-{
- spin_lock_irqsave(&ata_scsi_rbuf_lock, *flags);
-
- memset(ata_scsi_rbuf, 0, ATA_SCSI_RBUF_SIZE);
- if (copy_in)
- sg_copy_to_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
- ata_scsi_rbuf, ATA_SCSI_RBUF_SIZE);
- return ata_scsi_rbuf;
-}
-
-/**
- * ata_scsi_rbuf_put - Unmap response buffer.
- * @cmd: SCSI command containing buffer to be unmapped.
- * @copy_out: copy out result
- * @flags: @flags passed to ata_scsi_rbuf_get()
- *
- * Returns rbuf buffer. The result is copied to @cmd's buffer if
- * @copy_back is true.
- *
- * LOCKING:
- * Unlocks ata_scsi_rbuf_lock.
- */
-static inline void ata_scsi_rbuf_put(struct scsi_cmnd *cmd, bool copy_out,
- unsigned long *flags)
-{
- if (copy_out)
- sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
- ata_scsi_rbuf, ATA_SCSI_RBUF_SIZE);
- spin_unlock_irqrestore(&ata_scsi_rbuf_lock, *flags);
-}
-
-/**
* ata_scsi_rbuf_fill - wrapper for SCSI command simulators
* @args: device IDENTIFY data / SCSI command of interest.
* @actor: Callback hook for desired SCSI command simulator
@@ -2121,17 +2071,22 @@ static inline void ata_scsi_rbuf_put(struct scsi_cmnd *cmd, bool copy_out,
static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
unsigned int (*actor)(struct ata_scsi_args *args, u8 *rbuf))
{
- u8 *rbuf;
- unsigned int rc;
struct scsi_cmnd *cmd = args->cmd;
- unsigned long flags;
+ u8 *buf;
- rbuf = ata_scsi_rbuf_get(cmd, false, &flags);
- rc = actor(args, rbuf);
- ata_scsi_rbuf_put(cmd, rc == 0, &flags);
+ buf = kzalloc(ATA_SCSI_RBUF_SIZE, GFP_NOIO);
+ if (!buf) {
+ ata_scsi_set_sense(args->dev, cmd, NOT_READY, 0x08, 0);
+ return;
+ }
- if (rc == 0)
+ if (actor(args, buf) == 0) {
+ sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
+ buf, ATA_SCSI_RBUF_SIZE);
cmd->result = SAM_STAT_GOOD;
+ }
+
+ kfree(buf);
}
/**
@@ -3363,24 +3318,17 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
*
* Return: Number of bytes copied into sglist.
*/
-static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
+static ssize_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
u64 sector, u32 count)
{
- struct scsi_device *sdp = cmd->device;
- size_t len = sdp->sector_size;
size_t r;
__le64 *buf;
u32 i = 0;
- unsigned long flags;
-
- WARN_ON(len > ATA_SCSI_RBUF_SIZE);
- if (len > ATA_SCSI_RBUF_SIZE)
- len = ATA_SCSI_RBUF_SIZE;
+ buf = kzalloc(cmd->device->sector_size, GFP_NOFS);
+ if (!buf)
+ return -ENOMEM;
- spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
- buf = ((void *)ata_scsi_rbuf);
- memset(buf, 0, len);
while (i < trmax) {
u64 entry = sector |
((u64)(count > 0xffff ? 0xffff : count) << 48);
@@ -3390,9 +3338,9 @@ static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
count -= 0xffff;
sector += 0xffff;
}
- r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, len);
- spin_unlock_irqrestore(&ata_scsi_rbuf_lock, flags);
-
+ r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf,
+ cmd->device->sector_size);
+ kfree(buf);
return r;
}
@@ -3408,16 +3356,15 @@ static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
*
* Return: Number of bytes copied into sglist.
*/
-static size_t ata_format_sct_write_same(struct scsi_cmnd *cmd, u64 lba, u64 num)
+static ssize_t ata_format_sct_write_same(struct scsi_cmnd *cmd, u64 lba,
+ u64 num)
{
- struct scsi_device *sdp = cmd->device;
- size_t len = sdp->sector_size;
size_t r;
u16 *buf;
- unsigned long flags;
- spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
- buf = ((void *)ata_scsi_rbuf);
+ buf = kzalloc(cmd->device->sector_size, GFP_NOIO);
+ if (!buf)
+ return -ENOMEM;
put_unaligned_le16(0x0002, &buf[0]); /* SCT_ACT_WRITE_SAME */
put_unaligned_le16(0x0101, &buf[1]); /* WRITE PTRN FG */
@@ -3425,14 +3372,9 @@ static size_t ata_format_sct_write_same(struct scsi_cmnd *cmd, u64 lba, u64 num)
put_unaligned_le64(num, &buf[6]);
put_unaligned_le32(0u, &buf[10]); /* pattern */
- WARN_ON(len > ATA_SCSI_RBUF_SIZE);
-
- if (len > ATA_SCSI_RBUF_SIZE)
- len = ATA_SCSI_RBUF_SIZE;
-
- r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, len);
- spin_unlock_irqrestore(&ata_scsi_rbuf_lock, flags);
-
+ r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf,
+ cmd->device->sector_size);
+ kfree(buf);
return r;
}
@@ -3457,7 +3399,7 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
u64 block;
u32 n_block;
const u32 trmax = len >> 3;
- u32 size;
+ ssize_t size;
u16 fp;
u8 bp = 0xff;
u8 unmap = cdb[1] & 0x8;
@@ -3508,6 +3450,8 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
*/
if (unmap) {
size = ata_format_dsm_trim_descr(scmd, trmax, block, n_block);
+ if (size < 0)
+ goto comm_fail;
if (size != len)
goto invalid_param_len;
@@ -3531,6 +3475,8 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
}
} else {
size = ata_format_sct_write_same(scmd, block, n_block);
+ if (size < 0)
+ goto comm_fail;
if (size != len)
goto invalid_param_len;
@@ -3569,6 +3515,10 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
/* "Invalid command operation code" */
ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST, 0x20, 0x0);
return 1;
+comm_fail:
+ /* "Logical unit communication failure" */
+ ata_scsi_set_sense(dev, scmd, NOT_READY, 0x08, 0);
+ return 1;
}
/**
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v2] libata: switch to dynamic allocation insted of ata_scsi_rbuf
From: Tejun Heo @ 2017-01-10 16:06 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: geert, linux-ide
In-Reply-To: <1484064272-24129-1-git-send-email-hch@lst.de>
On Tue, Jan 10, 2017 at 05:04:32PM +0100, Christoph Hellwig wrote:
> Note of the emulated commands in the pageout/pagein path, so just do
> a GFP_NOIO dynamic allocation.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Applied to libata/for-4.11.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
From: Tejun Heo @ 2017-01-10 16:09 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide@vger.kernel.org,
linux-m68k, linux-kernel@vger.kernel.org
In-Reply-To: <3908219.8toGAOhnv9@amdc3058>
Hello,
On Mon, Jan 09, 2017 at 05:11:12PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > Disabling CONFIG_ATA_VERBOSE_ERROR saved 1380 bytes, which is less than the
> > value advertised by Kconfig (6KB).
>
> If it is only ~1kB nowadays I would vote for making the error logging always
> verbose and just removing CONFIG_ATA_VERBOSE_ERROR, Tejun?
No objection from me.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
From: Tejun Heo @ 2017-01-10 16:11 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k,
linux-kernel
In-Reply-To: <1483106478-1382-1-git-send-email-b.zolnierkie@samsung.com>
On Fri, Dec 30, 2016 at 03:01:15PM +0100, Bartlomiej Zolnierkiewicz wrote:
> Hi,
>
> This patchset adds m68k/Atari Falcon PATA support to libata.
> The major difference in the new libata's pata_falcon host
> driver when compared to legacy IDE's falconide host driver is
> that we are using polled PIO mode and thus avoiding the need
> for STDMA locking magic altogether.
>
> Tested under ARAnyM emulator.
Applied 1-3 to libata/for-4.11.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
From: Michael Schmitz @ 2017-01-10 20:02 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Tejun Heo, Geert Uytterhoeven, linux-ide, Linux/m68k,
Linux Kernel Development, Andreas Schwab
In-Reply-To: <3939493.HXZ6C33pKV@amdc3058>
Bartlomiej,
>> How is polling implemented in libata? Sleeping for something
>> approximating the average seek latency shouldn't hurt but spinning
>> wont be acceptable for a low performance single CPU architecture like
>> the Falcon.
>
> You can find actual implementation in libata-sff.c.
>
> Please see ata_sff_pio_task() for the main polling logic:
>
> fsm_start:
> WARN_ON_ONCE(ap->hsm_task_state == HSM_ST_IDLE);
>
> /*
> * This is purely heuristic. This is a fast path.
> * Sometimes when we enter, BSY will be cleared in
> * a chk-status or two. If not, the drive is probably seeking
> * or something. Snooze for a couple msecs, then
> * chk-status again. If still busy, queue delayed work.
> */
> status = ata_sff_busy_wait(ap, ATA_BUSY, 5);
> if (status & ATA_BUSY) {
> spin_unlock_irq(ap->lock);
> ata_msleep(ap, 2);
> spin_lock_irq(ap->lock);
>
> status = ata_sff_busy_wait(ap, ATA_BUSY, 10);
> if (status & ATA_BUSY) {
> ata_sff_queue_pio_task(link, ATA_SHORT_PAUSE);
> goto out_unlock;
> }
> }
>
> /*
> * hsm_move() may trigger another command to be processed.
> * clean the link beforehand.
> */
> ap->sff_pio_task_link = NULL;
> /* move the HSM */
> poll_next = ata_sff_hsm_move(ap, qc, status, 1);
>
> /* another command or interrupt handler
> * may be running at this point.
> */
> if (poll_next)
> goto fsm_start;
> out_unlock:
>
>
> ata_sff_busy_wait()'s last argument is number of 10uS waits so
> first check (50uS) should be quite quick. If device is still
> busy we sleep for 2ms. Then we quickly (100uS) busy wait and
> if needed queue delayed work (with ATA_SHORT_PAUSE == 16 ms
> delay).
>
> Overall the current heuristic looks fine and spinning should be
> minimal.
Thanks, that looks entirely reasonable.
>> > Tested under ARAnyM emulator.
>>
>> Not sure that the emulator is really feature complete enough - I'll
>> get this tested on my Falcon in the next few weeks. I'm a bit worried
>
> Great!
>
>> about IDE still generating interrupts on seek completion (did you spot
>> anything like that, Geert?).
>>
>> Cheers,
>>
>> Michael
>
> BTW according to comment in arch/m68k/atari/stdma.c:
>
> /* On the Falcon, the IDE bus uses just the ACSI/Floppy interrupt, but */
> /* not the ST-DMA chip itself. So falhd.c needs not to lock the */
> /* chip. The interrupt is routed to falhd.c if IDE is configured, the */
> /* model is a Falcon and the interrupt was caused by the HD controller */
> /* (can be determined by looking at its status register). */
That comment is probably incorrect in part. Blame Linus :-)
Seriously though - that comment dates back to the dark ages (when m68k
was an entirely separate port and the IDE driver was indeed named
falhd.c). That would have been even before 2.2 or 2.4 times. The
comment just never got updated.
What is still correct is that the IDE driver does use the interrupt
only, not the ST-DMA chip. And a single IDE interrupt can be correctly
assigned to IDE by looking at the status register.
With the SCSI (and IIRC also floppy) interrupts, we don't have direct
access to the status registers without disturbing the state of the DMA
though. Unless we know for definite that either chips have raised the
interrupt (and DMA ops are in flight), we must not touch the DMA chip
at all.
The case I'm worried about is both IDE and SCSI raising an interrupt.
We don't currently mask the IDE/ST-DMA interrupt so a stacked
interrupt must be processed in the same pass as the initial interrupt
or it will get dropped. We'd have to peek at the DMA registers to
check the SCSI or floppy interrupt status, and we just can't safely do
that. So races of this kind are currently prevented by including IDE
in the IRQ locking process.
Whether it's possible to mask the interrupt, do one pass, unmask and
process the second interrupt I don't know. Maybe Andreas does?
Cheers,
Michael
> it should be okay to use IDE at the same time as SCSI/Floppy which
> is what the new driver does (the old one is serialized operations by
> ST-DMA related IRQ handling magic).
>
> Also the comment itself may need some fixups as on Falcon it is SCSI
> not ACSI (according to the earlier comment in same file) and the old
> IDE host driver name is not falhd.c but falconide.c.
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
^ permalink raw reply
* Re: [LFS/MM TOPIC][LFS/MM ATTEND]: - Storage Stack and Driver Testing methodology.
From: Chaitanya Kulkarni @ 2017-01-10 22:40 UTC (permalink / raw)
To: lsf-pc@lists.linux-foundation.org
Cc: linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org,
linux-nvme@lists.infradead.org, linux-scsi@vger.kernel.org,
linux-ide@vger.kernel.org
In-Reply-To: <CO2PR04MB218427BF42159B20FB26F74186670@CO2PR04MB2184.namprd04.prod.outlook.com>
Resending it at as a plain text.
From: Chaitanya Kulkarni
Sent: Tuesday, January 10, 2017 2:37 PM
To: lsf-pc@lists.linux-foundation.org
Cc: linux-fsdevel@vger.kernel.org; linux-block@vger.kernel.org; linux-nvme@lists.infradead.org; linux-scsi@vger.kernel.org; linux-ide@vger.kernel.org
Subject: [LFS/MM TOPIC][LFS/MM ATTEND]: - Storage Stack and Driver Testing methodology.
Hi Folks,
I would like to propose a general discussion on Storage stack and device driver testing.
Purpose:-
-------------
The main objective of this discussion is to address the need for
a Unified Test Automation Framework which can be used by different subsystems
in the kernel in order to improve the overall development and stability
of the storage stack.
For Example:-
>From my previous experience, I've worked on the NVMe driver testing last year and we
have developed simple unit test framework
(https://github.com/linux-nvme/nvme-cli/tree/master/tests).
In current implementation Upstream NVMe Driver supports following subsystems:-
1. PCI Host.
2. RDMA Target.
3. Fiber Channel Target (in progress).
Today due to lack of centralized automated test framework NVMe Driver testing is
scattered and performed using the combination of various utilities like nvme-cli/tests,
nvmet-cli, shell scripts (git://git.infradead.org/nvme-fabrics.git nvmf-selftests) etc.
In order to improve overall driver stability with various subsystems, it will be beneficial
to have a Unified Test Automation Framework (UTAF) which will centralize overall
testing.
This topic will allow developers from various subsystem engage in the discussion about
how to collaborate efficiently instead of having discussions on lengthy email threads.
Participants:-
------------------
I'd like to invite developers from different subsystems to discuss an approach towards
a unified testing methodology for storage stack and device drivers belongs to
different subsystems.
Topics for Discussion:-
------------------------------
As a part of discussion following are some of the key points which we can focus on:-
1. What are the common components of the kernel used by the various subsystems?
2. What are the potential target drivers which can benefit from this approach?
(e.g. NVMe, NVMe Over Fabric, Open Channel Solid State Drives etc.)
3. What are the desired features that can be implemented in this Framework?
(code coverage, unit tests, stress testings, regression, generating Coccinelle reports etc.)
4. Desirable Report generation mechanism?
5. Basic performance validation?
6. Whether QEMU can be used to emulate some of the H/W functionality to create a test
platform? (Optional subsystem specific)
Some background about myself I'm Chaitanya Kulkarni, I worked as a team lead
which was responsible for delivering scalable multiplatform Automated Test
Framework for device drivers testing at HGST. It's been used for more than 1 year on
Linux/Windows for unit testing/regression/performance validation of the NVMe Linux and
Windows driver successfully. I've also recently started contributing to the
NVMe Host and NVMe over Fabrics Target driver.
Regards,
-Chaitanya
^ permalink raw reply
* Re: [LFS/MM TOPIC][LFS/MM ATTEND]: - Storage Stack and Driver Testing methodology.
From: Hannes Reinecke @ 2017-01-11 7:42 UTC (permalink / raw)
To: Chaitanya Kulkarni, lsf-pc@lists.linux-foundation.org
Cc: linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org,
linux-nvme@lists.infradead.org, linux-scsi@vger.kernel.org,
linux-ide@vger.kernel.org
In-Reply-To: <CO2PR04MB2184DB653C04FB620B41435486670@CO2PR04MB2184.namprd04.prod.outlook.com>
On 01/10/2017 11:40 PM, Chaitanya Kulkarni wrote:
> Resending it at as a plain text.
>
> From: Chaitanya Kulkarni
> Sent: Tuesday, January 10, 2017 2:37 PM
> To: lsf-pc@lists.linux-foundation.org
> Cc: linux-fsdevel@vger.kernel.org; linux-block@vger.kernel.org; linux-nvme@lists.infradead.org; linux-scsi@vger.kernel.org; linux-ide@vger.kernel.org
> Subject: [LFS/MM TOPIC][LFS/MM ATTEND]: - Storage Stack and Driver Testing methodology.
>
>
> Hi Folks,
>
> I would like to propose a general discussion on Storage stack and device driver testing.
>
> Purpose:-
> -------------
> The main objective of this discussion is to address the need for
> a Unified Test Automation Framework which can be used by different subsystems
> in the kernel in order to improve the overall development and stability
> of the storage stack.
>
> For Example:-
> From my previous experience, I've worked on the NVMe driver testing last year and we
> have developed simple unit test framework
> (https://github.com/linux-nvme/nvme-cli/tree/master/tests).
> In current implementation Upstream NVMe Driver supports following subsystems:-
> 1. PCI Host.
> 2. RDMA Target.
> 3. Fiber Channel Target (in progress).
> Today due to lack of centralized automated test framework NVMe Driver testing is
> scattered and performed using the combination of various utilities like nvme-cli/tests,
> nvmet-cli, shell scripts (git://git.infradead.org/nvme-fabrics.git nvmf-selftests) etc.
>
> In order to improve overall driver stability with various subsystems, it will be beneficial
> to have a Unified Test Automation Framework (UTAF) which will centralize overall
> testing.
>
> This topic will allow developers from various subsystem engage in the discussion about
> how to collaborate efficiently instead of having discussions on lengthy email threads.
>
> Participants:-
> ------------------
> I'd like to invite developers from different subsystems to discuss an approach towards
> a unified testing methodology for storage stack and device drivers belongs to
> different subsystems.
>
> Topics for Discussion:-
> ------------------------------
> As a part of discussion following are some of the key points which we can focus on:-
> 1. What are the common components of the kernel used by the various subsystems?
> 2. What are the potential target drivers which can benefit from this approach?
> (e.g. NVMe, NVMe Over Fabric, Open Channel Solid State Drives etc.)
> 3. What are the desired features that can be implemented in this Framework?
> (code coverage, unit tests, stress testings, regression, generating Coccinelle reports etc.)
> 4. Desirable Report generation mechanism?
> 5. Basic performance validation?
> 6. Whether QEMU can be used to emulate some of the H/W functionality to create a test
> platform? (Optional subsystem specific)
>
> Some background about myself I'm Chaitanya Kulkarni, I worked as a team lead
> which was responsible for delivering scalable multiplatform Automated Test
> Framework for device drivers testing at HGST. It's been used for more than 1 year on
> Linux/Windows for unit testing/regression/performance validation of the NVMe Linux and
> Windows driver successfully. I've also recently started contributing to the
>
> NVMe Host and NVMe over Fabrics Target driver.
>
Oh, yes, please.
That's a discussion I'd like to have, too.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@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)
^ permalink raw reply
* Re: [LFS/MM TOPIC][LFS/MM ATTEND]: - Storage Stack and Driver Testing methodology.
From: Johannes Thumshirn @ 2017-01-11 9:19 UTC (permalink / raw)
To: Chaitanya Kulkarni
Cc: lsf-pc@lists.linux-foundation.org, linux-fsdevel@vger.kernel.org,
linux-block@vger.kernel.org, linux-nvme@lists.infradead.org,
linux-scsi@vger.kernel.org, linux-ide@vger.kernel.org
In-Reply-To: <CO2PR04MB2184DB653C04FB620B41435486670@CO2PR04MB2184.namprd04.prod.outlook.com>
On Tue, Jan 10, 2017 at 10:40:53PM +0000, Chaitanya Kulkarni wrote:
> Resending it at as a plain text.
>
> From: Chaitanya Kulkarni
> Sent: Tuesday, January 10, 2017 2:37 PM
> To: lsf-pc@lists.linux-foundation.org
> Cc: linux-fsdevel@vger.kernel.org; linux-block@vger.kernel.org; linux-nvme@lists.infradead.org; linux-scsi@vger.kernel.org; linux-ide@vger.kernel.org
> Subject: [LFS/MM TOPIC][LFS/MM ATTEND]: - Storage Stack and Driver Testing methodology.
>
>
> Hi Folks,
>
> I would like to propose a general discussion on Storage stack and device driver testing.
>
> Purpose:-
> -------------
> The main objective of this discussion is to address the need for
> a Unified Test Automation Framework which can be used by different subsystems
> in the kernel in order to improve the overall development and stability
> of the storage stack.
>
> For Example:-
> From my previous experience, I've worked on the NVMe driver testing last year and we
> have developed simple unit test framework
> (https://github.com/linux-nvme/nvme-cli/tree/master/tests).
> In current implementation Upstream NVMe Driver supports following subsystems:-
> 1. PCI Host.
> 2. RDMA Target.
> 3. Fiber Channel Target (in progress).
> Today due to lack of centralized automated test framework NVMe Driver testing is
> scattered and performed using the combination of various utilities like nvme-cli/tests,
> nvmet-cli, shell scripts (git://git.infradead.org/nvme-fabrics.git nvmf-selftests) etc.
>
> In order to improve overall driver stability with various subsystems, it will be beneficial
> to have a Unified Test Automation Framework (UTAF) which will centralize overall
> testing.
>
> This topic will allow developers from various subsystem engage in the discussion about
> how to collaborate efficiently instead of having discussions on lengthy email threads.
>
> Participants:-
> ------------------
> I'd like to invite developers from different subsystems to discuss an approach towards
> a unified testing methodology for storage stack and device drivers belongs to
> different subsystems.
>
> Topics for Discussion:-
> ------------------------------
> As a part of discussion following are some of the key points which we can focus on:-
> 1. What are the common components of the kernel used by the various subsystems?
> 2. What are the potential target drivers which can benefit from this approach?
> (e.g. NVMe, NVMe Over Fabric, Open Channel Solid State Drives etc.)
> 3. What are the desired features that can be implemented in this Framework?
> (code coverage, unit tests, stress testings, regression, generating Coccinelle reports etc.)
> 4. Desirable Report generation mechanism?
> 5. Basic performance validation?
> 6. Whether QEMU can be used to emulate some of the H/W functionality to create a test
> platform? (Optional subsystem specific)
Well, something I was thinking about but didn't find enough time to actually
implement is making a xfstestes like test suite written using sg3_utils for
SCSI. This idea could very well be extented to NVMe, AHCI, blk, etc...
Byte,
Johannes
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply
* Re: [LFS/MM TOPIC][LFS/MM ATTEND]: - Storage Stack and Driver Testing methodology.
From: Christoph Hellwig @ 2017-01-11 9:24 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Chaitanya Kulkarni, linux-scsi@vger.kernel.org,
linux-nvme@lists.infradead.org, linux-block@vger.kernel.org,
linux-ide@vger.kernel.org, linux-fsdevel@vger.kernel.org,
lsf-pc@lists.linux-foundation.org
In-Reply-To: <20170111091945.GD6286@linux-x5ow.site>
On Wed, Jan 11, 2017 at 10:19:45AM +0100, Johannes Thumshirn wrote:
> Well, something I was thinking about but didn't find enough time to actually
> implement is making a xfstestes like test suite written using sg3_utils for
> SCSI.
Ronnie's libiscsi testsuite can use SG_IO for a new years now:
https://github.com/sahlberg/libiscsi/tree/master/test-tool
and has been very useful to find bus in various protocol
implementations.
> This idea could very well be extented to NVMe
Chaitanya suite is doing something similar for NVMe, although the
coverage is still much more limited so far.
^ permalink raw reply
* Re: [LFS/MM TOPIC][LFS/MM ATTEND]: - Storage Stack and Driver Testing methodology.
From: Hannes Reinecke @ 2017-01-11 9:40 UTC (permalink / raw)
To: Christoph Hellwig, Johannes Thumshirn
Cc: Chaitanya Kulkarni, linux-scsi@vger.kernel.org,
linux-nvme@lists.infradead.org, linux-block@vger.kernel.org,
linux-ide@vger.kernel.org, linux-fsdevel@vger.kernel.org,
lsf-pc@lists.linux-foundation.org
In-Reply-To: <20170111092447.GA13600@infradead.org>
On 01/11/2017 10:24 AM, Christoph Hellwig wrote:
> On Wed, Jan 11, 2017 at 10:19:45AM +0100, Johannes Thumshirn wrote:
>> Well, something I was thinking about but didn't find enough time to actually
>> implement is making a xfstestes like test suite written using sg3_utils for
>> SCSI.
>
> Ronnie's libiscsi testsuite can use SG_IO for a new years now:
>
> https://github.com/sahlberg/libiscsi/tree/master/test-tool
>
> and has been very useful to find bus in various protocol
> implementations.
>
>> This idea could very well be extented to NVMe
>
> Chaitanya suite is doing something similar for NVMe, although the
> coverage is still much more limited so far.
>
One of the discussion points here indeed would be if we want to go in
the direction of a protocol-specific testsuites (of which we already
have several) or if it makes sense to move to functional testing.
And if we can have a common interface / documentation on how these
things should run.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@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)
^ permalink raw reply
* [PATCH] ahci: imx: fix building without hwmon or thermal
From: Arnd Bergmann @ 2017-01-11 13:36 UTC (permalink / raw)
To: Tejun Heo
Cc: Arnd Bergmann, Bartlomiej Zolnierkiewicz, Fabien Lahoudere,
linux-ide, linux-kernel
When CONFIG_HWMON is disabled, we now get a link failure:
ERROR: "devm_hwmon_device_register_with_groups" [drivers/ata/ahci_imx.ko] undefined!
drivers/ata/ahci_imx.o: In function `imx_ahci_probe':
ahci_imx.c:(.text.imx_ahci_probe+0x304): undefined reference to `devm_thermal_zone_of_sensor_register'
This makes the code calling into the hwmon subsystem compile-time
conditional, and adds a Kconfig dependency to avoid the corner
case of having HWMON=m and AHCI_IMX=y, by forcing AHCI_IMX=m in this
case. The thermal subsystem already has a check in its header, but
that also doesn't cover the THERMAL=m case, so we need a somewhat
complex Kconfig expression to handle all cases.
Fixes: 54643a83b41a ("ahci: imx: Add imx53 SATA temperature sensor support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/ata/Kconfig | 1 +
drivers/ata/ahci_imx.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 78c002021029..70b57d2229d6 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -129,6 +129,7 @@ config AHCI_ST
config AHCI_IMX
tristate "Freescale i.MX AHCI SATA support"
depends on MFD_SYSCON && (ARCH_MXC || COMPILE_TEST)
+ depends on (HWMON && (THERMAL || !THERMAL_OF)) || !HWMON
help
This option enables support for the Freescale i.MX SoC's
onboard AHCI SATA.
diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index 420f065978dc..787567e840bd 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -774,7 +774,8 @@ static int imx_ahci_probe(struct platform_device *pdev)
if (ret)
return ret;
- if (imxpriv->type == AHCI_IMX53) {
+ if (imxpriv->type == AHCI_IMX53 &&
+ IS_ENABLED(CONFIG_HWMON)) {
/* Add the temperature monitor */
struct device *hwmon_dev;
--
2.9.0
^ permalink raw reply related
* Re: [PATCH] ahci: imx: fix building without hwmon or thermal
From: Bartlomiej Zolnierkiewicz @ 2017-01-11 14:16 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Tejun Heo, Fabien Lahoudere, linux-ide, linux-kernel
In-Reply-To: <20170111133652.3715437-1-arnd@arndb.de>
Hi,
On Wednesday, January 11, 2017 02:36:16 PM Arnd Bergmann wrote:
> When CONFIG_HWMON is disabled, we now get a link failure:
>
> ERROR: "devm_hwmon_device_register_with_groups" [drivers/ata/ahci_imx.ko] undefined!
> drivers/ata/ahci_imx.o: In function `imx_ahci_probe':
> ahci_imx.c:(.text.imx_ahci_probe+0x304): undefined reference to `devm_thermal_zone_of_sensor_register'
>
> This makes the code calling into the hwmon subsystem compile-time
> conditional, and adds a Kconfig dependency to avoid the corner
> case of having HWMON=m and AHCI_IMX=y, by forcing AHCI_IMX=m in this
> case. The thermal subsystem already has a check in its header, but
> that also doesn't cover the THERMAL=m case, so we need a somewhat
> complex Kconfig expression to handle all cases.
>
> Fixes: 54643a83b41a ("ahci: imx: Add imx53 SATA temperature sensor support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Looks fine to me (I see that this is the same solution as for
TOUCHSCREEN_SUN4I from commit 4a6155a46565 but without hard
dependency on HWMON).
Thanks for fixing this.
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> ---
> drivers/ata/Kconfig | 1 +
> drivers/ata/ahci_imx.c | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 78c002021029..70b57d2229d6 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -129,6 +129,7 @@ config AHCI_ST
> config AHCI_IMX
> tristate "Freescale i.MX AHCI SATA support"
> depends on MFD_SYSCON && (ARCH_MXC || COMPILE_TEST)
> + depends on (HWMON && (THERMAL || !THERMAL_OF)) || !HWMON
> help
> This option enables support for the Freescale i.MX SoC's
> onboard AHCI SATA.
> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> index 420f065978dc..787567e840bd 100644
> --- a/drivers/ata/ahci_imx.c
> +++ b/drivers/ata/ahci_imx.c
> @@ -774,7 +774,8 @@ static int imx_ahci_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - if (imxpriv->type == AHCI_IMX53) {
> + if (imxpriv->type == AHCI_IMX53 &&
> + IS_ENABLED(CONFIG_HWMON)) {
> /* Add the temperature monitor */
> struct device *hwmon_dev;
^ permalink raw reply
* Re: [PATCH] PCI:MSI Return -ENOSPC when requested vectors is not enough
From: Bjorn Helgaas @ 2017-01-11 18:18 UTC (permalink / raw)
To: Dennis Chen
Cc: linux-pci, linux-ide, Lorenzo Pieralisi, Steve Capper,
Marc Zyngier, Tom Long Nguyen, Bjorn Helgaas, Greg Kroah-Hartman,
Tejun Heo, nd, Christoph Hellwig, linux-arm-kernel
In-Reply-To: <1480558504-18691-1-git-send-email-dennis.chen@arm.com>
On Thu, Dec 01, 2016 at 10:15:04AM +0800, Dennis Chen wrote:
> The __pci_enable_msi_range() should return -ENOSPC instead of -EINVAL
> when the device doesn't have enough vectors as required, just as the
> MSI-X vector allocator does in __pci_enable_msix_range(). Otherwise,
> some drivers depending on that return value will probably fallback to
> the legacy interrupt directly, for example, in commit 17a51f12cfbd2814
> ("ahci: only try to use multi-MSI mode if there is more than 1 port"), the
> ahci driver will fallback to single MSI mode only when the return value
> is -ENOSPC in case of required vectors is not enough, else the driver will
> use legacy interrupt which has been observed on a x86 box with 6-port SATA
> controller.
Unless Christoph objects, I'll apply this, but I don't understand the
situation with 17a51f12cfbd. That commit doesn't check for EINVAL or
ENOSPC so I don't know what the connection with this patch is.
I know Christoph said he changed something in ahci to treat all errors
the same, but I don't know where that is, either.
If there's a revision of Linus' tree that is broken, please give the
details so I can at least describe which versions are broken, when it
got fixed by Christoph, and figure out whether we need stable
backports or anything.
Bjorn
> With this patch, when a MSI-capable device doesn't have enough MSI
> vectors as requested, it will fallback to single MSI mode while not
> legacy interrupt.
>
> Signed-off-by: Dennis Chen <dennis.chen@arm.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Tom Long Nguyen <tom.l.nguyen@intel.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Steve Capper <steve.capper@arm.com>
> Cc: linux-ide@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> drivers/pci/msi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index ad70507..da37113 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -1084,7 +1084,7 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
> if (nvec < 0)
> return nvec;
> if (nvec < minvec)
> - return -EINVAL;
> + return -ENOSPC;
>
> if (nvec > maxvec)
> nvec = maxvec;
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [Lsf-pc] [LFS/MM TOPIC][LFS/MM ATTEND]: - Storage Stack and Driver Testing methodology.
From: Sagi Grimberg @ 2017-01-12 11:01 UTC (permalink / raw)
To: Chaitanya Kulkarni, lsf-pc@lists.linux-foundation.org
Cc: linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-nvme@lists.infradead.org,
linux-ide@vger.kernel.org
In-Reply-To: <CO2PR04MB218427BF42159B20FB26F74186670@CO2PR04MB2184.namprd04.prod.outlook.com>
> Hi Folks,
>
> I would like to propose a general discussion on Storage stack and device driver testing.
I think its very useful and needed.
> Purpose:-
> -------------
> The main objective of this discussion is to address the need for
> a Unified Test Automation Framework which can be used by different subsystems
> in the kernel in order to improve the overall development and stability
> of the storage stack.
>
> For Example:-
> From my previous experience, I've worked on the NVMe driver testing last year and we
> have developed simple unit test framework
> (https://github.com/linux-nvme/nvme-cli/tree/master/tests).
> In current implementation Upstream NVMe Driver supports following subsystems:-
> 1. PCI Host.
> 2. RDMA Target.
> 3. Fiber Channel Target (in progress).
> Today due to lack of centralized automated test framework NVMe Driver testing is
> scattered and performed using the combination of various utilities like nvme-cli/tests,
> nvmet-cli, shell scripts (git://git.infradead.org/nvme-fabrics.git nvmf-selftests) etc.
>
> In order to improve overall driver stability with various subsystems, it will be beneficial
> to have a Unified Test Automation Framework (UTAF) which will centralize overall
> testing.
>
> This topic will allow developers from various subsystem engage in the discussion about
> how to collaborate efficiently instead of having discussions on lengthy email threads.
While a unified test framework for all sounds great, I suspect that the
difference might be too large. So I think that for this framework to be
maintainable, it needs to be carefully designed such that we don't have
too much code churn.
For example we should start by classifying tests and then see where
sharing is feasible:
1. basic management - I think not a lot can be shared
2. spec compliance - again, not much sharing here
3. data-verification - probably everything can be shared
4. basic performance - probably a lot can be shared
5. vectored-io - probably everything can be shared
6. error handling - I can think of some sharing that can be used.
This repository can also store some useful tracing scripts (ebpf and
friends) that are useful for performance analysis.
So I think that for this to happen, we can start with the shared
test under block/, then migrate proto specific tests into
scsi/, nvme/, and then add transport specific tests so
we can have something like:
├── block
├── lib
├── nvme
│ ├── fabrics
│ │ ├── loop
│ │ └── rdma
│ └── pci
└── scsi
├── fc
└── iscsi
Thoughts?
^ permalink raw reply
* Re: [PATCH] PCI:MSI Return -ENOSPC when requested vectors is not enough
From: Christoph Hellwig @ 2017-01-12 13:42 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Dennis Chen, linux-pci, linux-ide, Lorenzo Pieralisi,
Steve Capper, Marc Zyngier, Tom Long Nguyen, Bjorn Helgaas,
Greg Kroah-Hartman, Tejun Heo, nd, Christoph Hellwig,
linux-arm-kernel
In-Reply-To: <20170111181853.GA14532@bhelgaas-glaptop.roam.corp.google.com>
On Wed, Jan 11, 2017 at 12:18:53PM -0600, Bjorn Helgaas wrote:
> Unless Christoph objects, I'll apply this, but I don't understand the
> situation with 17a51f12cfbd. That commit doesn't check for EINVAL or
> ENOSPC so I don't know what the connection with this patch is.
I don't think that commit is the culprit.
> I know Christoph said he changed something in ahci to treat all errors
> the same, but I don't know where that is, either.
"ahci: always fall back to single-MSI mode"
^ permalink raw reply
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
From: Finn Thain @ 2017-01-13 2:33 UTC (permalink / raw)
To: Michael Schmitz
Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, Geert Uytterhoeven,
linux-ide, Linux/m68k, Linux Kernel Development, Andreas Schwab
In-Reply-To: <CAOmrzkJfQu6PuCpyyRJL8PNvSYxYO42F-k8jEYf2d1-5C28A_A@mail.gmail.com>
On Wed, 11 Jan 2017, Michael Schmitz wrote:
> What is still correct is that the IDE driver does use the interrupt
> only, not the ST-DMA chip. And a single IDE interrupt can be correctly
> assigned to IDE by looking at the status register.
>
> With the SCSI (and IIRC also floppy) interrupts, we don't have direct
> access to the status registers without disturbing the state of the DMA
> though. Unless we know for definite that either chips have raised the
> interrupt (and DMA ops are in flight), we must not touch the DMA chip at
> all.
>
> The case I'm worried about is both IDE and SCSI raising an interrupt. We
> don't currently mask the IDE/ST-DMA interrupt so a stacked interrupt
> must be processed in the same pass as the initial interrupt or it will
> get dropped. We'd have to peek at the DMA registers to check the SCSI or
> floppy interrupt status, and we just can't safely do that. So races of
> this kind are currently prevented by including IDE in the IRQ locking
> process.
>
> Whether it's possible to mask the interrupt, do one pass, unmask and
> process the second interrupt I don't know.
Would that require handling the SCSI DMA interrupt in the first pass? Or
handling IDE first, and ensuring that the IDE handler does not access
ST-DMA registers? What about FDC?
The atari_scsi handler accesses the ST-DMA registers; it can do so because
it knows that any DMA must have completed -- it can infer this because a
simultaneous pending interrupt from FDC or IDE is impossible due to
stdma_lock().
Your suggestion would seem to allow other pending interrupts, hence the
atari_scsi interrupt handler logic has to be tossed out. What logic would
replace it?
If all else fails, perhaps we could inhibit DMA entirely when the new ATA
driver is loaded. Then we can just dispatch the ST-DMA irq like a shared
irq. I'm sure that atari_scsi can work without DMA. No idea about the FDC
driver though (ataflop.c).
Another solution would be to dedicate the DMA function to atari_scsi, and
then mask the FDC and IDE interrupts during each DMA transfer. But once
again, this would mean changing the FDC driver to eliminate DMA, if that
is possible. From the schematic it looks the the FDC chip, "AJAX", is
another custom ...
http://dev-docs.atariforge.org/files/Falcon030_Schematic.pdf
Unfortunately my grasp of the ST hardware reflects my inability to read
German; those who can may want to take a look at "ATARI Profibuch
ST-STE-TT.pdf".
--
> Maybe Andreas does?
>
> Cheers,
>
> Michael
>
>
> > it should be okay to use IDE at the same time as SCSI/Floppy which is
> > what the new driver does (the old one is serialized operations by
> > ST-DMA related IRQ handling magic).
> >
> > Also the comment itself may need some fixups as on Falcon it is SCSI
> > not ACSI (according to the earlier comment in same file) and the old
> > IDE host driver name is not falhd.c but falconide.c.
> >
> > Best regards,
> > --
> > Bartlomiej Zolnierkiewicz
> > Samsung R&D Institute Poland
> > Samsung Electronics
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox