On Thursday 02 April 2009 21:37:35 Bartlomiej Zolnierkiewicz wrote:
> On Thursday 02 April 2009, Matthew Wilcox wrote:
> > On Thu, Apr 02, 2009 at 08:38:03PM +0400, Sergei Shtylyov wrote:
> > > Hello.
> > >>>> + task->tf_flags = IDE_TFLAG_LBA48 | IDE_TFLAG_OUT_HOB |
> > >>>> + IDE_TFLAG_OUT_TF | IDE_TFLAG_OUT_DEVICE |
> > >
> > >>> The last 3 flags are going to be obsoleted too...
> > >
> > >> So if I remove them today, the command will still work?
> > >
> > > s/obsoleted/renamed and moved to another other field/ -- I'm going to
> > > submit a patchset refactoring 'struct ide_cmd and 'struct ide-taskfile'
> > > at last...
> >
> > Since I'm coding to today's kernel, not to a patch which doesn't exist
> > yet, taking them out won't work very well.
> >
> > I've not been paying much attention to drivers/ide, so I've no idea
> > whether the following patch works. It does at least compile:
> >
> > +++ b/drivers/ide/ide-disk.c
> > @@ -407,7 +407,7 @@ static void idedisk_prepare_flush(struct request_queue *q, s
> > static int idedisk_prepare_discard(struct request_queue *q, struct request *rq,
> > struct bio *bio)
> > {
> > - ide_task_t *task;
> > + struct ide_cmd *task;
> > unsigned size;
> > struct page *page = alloc_page(GFP_KERNEL);
> > if (!page)
> > @@ -432,8 +432,8 @@ static int idedisk_prepare_discard(struct request_queue *q,
> >
> > task->tf_flags = IDE_TFLAG_LBA48 | IDE_TFLAG_OUT_HOB |
> > IDE_TFLAG_OUT_TF | IDE_TFLAG_OUT_DEVICE |
> > - IDE_TFLAG_DYN;
> > - task->data_phase = TASKFILE_OUT_DMA;
> > + IDE_TFLAG_DYN | IDE_TFLAG_WRITE;
> > + task->protocol = ATA_PROT_DMA;
> >
> > rq->cmd_type = REQ_TYPE_ATA_TASKFILE;
> > rq->special = task;
> >
> > If I've understood 0dfb991c6943c810175376b58d1c29cfe532541b correctly,
> > this should be equivalent. Bart?
>
> Yep. The final patch looks fine overall (thanks for adding TRIM support
> also to drivers/ide), with the small exception for this chunk:
>
> @@ -521,6 +567,9 @@ static int set_wcache(ide_drive_t *drive, int arg)
>
> update_ordered(drive);
>
> + if (ata_id_has_trim(drive->id))
> + blk_queue_set_discard(drive->queue, idedisk_prepare_discard);
> +
> return err;
> }
>
> as it would fit better somewhere in ide_disk_setup() ISO set_wcache()...
>
> [ When it comes to CodingStyle issues I don't care that much though I think
> that applying Sergei's suggestions will make the patch more consistent with
> the rest of drivers/ide code... ]
I would like to see this merged for -rc1 so I tried to put it altogether
and then integrate it over recent IDE and block changes...
I ended up with the patch below which I'll push to Linus in a few minutes.
Once it hits Linus' tree please test that it still works. :)
Thanks.
From: David Woodhouse <David.Woodhouse@xxxxxxxxx>
Subject: [PATCH 4/5] ide: Add support for TRIM
Bart: Since this is a new feature and hasn't seen much testing with
production devices it is not enabled by default yet (requires use of
"ide_core.trim=1" kernel parameter).
Signed-off-by: David Woodhouse <David.Woodhouse@xxxxxxxxx>
[modified to reduce amount of special casing needed for discard]
Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxxxx>
Cc: Jeff Garzik <jgarzik@xxxxxxxxxx>
Cc: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
[bart: apply follow-up changes from Matthew]
[bart: apply CodingStyle fixups per Sergei's suggestion]
[bart: move blk_queue_set_discard() call to ide_disk_setup()]
[bart: port over recent IDE changes]
[bart: remove "bio" argument from idedisk_prepare_discard()]
[bart: s/PAGE_SIZE/PAGE_SIZE + 8/ + set ATA_LBA bit in tf.device]
[bart: add "ide_core.trim=" kernel parameter]
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
---
drivers/ide/ide-disk.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/ide/ide.c | 6 +++++
2 files changed, 59 insertions(+)
Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -415,6 +415,54 @@ static void idedisk_prepare_flush(struct
rq->special = cmd;
}
+static int idedisk_prepare_discard(struct request_queue *q, struct request *rq)
+{
+ struct ide_cmd *cmd;
+ struct page *page;
+ struct bio *bio = rq->bio;
+ unsigned int size;
+
+ page = alloc_page(GFP_KERNEL);
+ if (!page)
+ goto error;
+
+ cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
+ if (!cmd)
+ goto free_page;
+
+ size = ata_set_lba_range_entries(page_address(page), PAGE_SIZE / 8,
+ bio->bi_sector, bio_sectors(bio));
+ bio->bi_size = 0;
+
+ if (bio_add_pc_page(q, bio, page, size, 0) < size)
+ goto free_task;
+
+ cmd->tf.command = ATA_CMD_DSM;
+ cmd->tf.feature = ATA_DSM_TRIM;
+ cmd->tf.nsect = size / 512;
+ cmd->hob.nsect = (size / 512) >> 8;
+ cmd->tf.device = ATA_LBA;
+
+ cmd->valid.out.tf = IDE_VALID_OUT_TF | IDE_VALID_DEVICE;
+ cmd->valid.out.hob = IDE_VALID_OUT_HOB;
+
+ cmd->tf_flags = IDE_TFLAG_LBA48 | IDE_TFLAG_WRITE | IDE_TFLAG_DYN;
+ cmd->protocol = ATA_PROT_DMA;
+
+ rq->cmd_type = REQ_TYPE_ATA_TASKFILE;
+ rq->special = cmd;
+ rq->nr_sectors = size / 512;
+
+ return 0;
+
+ free_task:
+ kfree(cmd);
+ free_page:
+ __free_page(page);
+ error:
+ return -ENOMEM;
+}
+
ide_devset_get(multcount, mult_count);
/*
@@ -606,6 +654,8 @@ static int ide_disk_check(ide_drive_t *d
return 1;
}
+extern int ide_trim;
+
static void ide_disk_setup(ide_drive_t *drive)
{
struct ide_disk_obj *idkp = drive->driver_data;
@@ -693,6 +743,9 @@ static void ide_disk_setup(ide_drive_t *
set_wcache(drive, 1);
+ if (ata_id_has_trim(id) && ide_trim)
+ blk_queue_set_discard(q, idedisk_prepare_discard);
+
if ((drive->dev_flags & IDE_DFLAG_LBA) == 0 &&
(drive->head == 0 || drive->head > 16)) {
printk(KERN_ERR "%s: invalid geometry: %d physical heads?\n",
Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -178,6 +178,12 @@ EXPORT_SYMBOL_GPL(ide_pci_clk);
module_param_named(pci_clock, ide_pci_clk, int, 0);
MODULE_PARM_DESC(pci_clock, "PCI bus clock frequency (in MHz)");
+int ide_trim = 0;
+EXPORT_SYMBOL_GPL(ide_trim);
+
+module_param_named(trim, ide_trim, int, 0);
+MODULE_PARM_DESC(trim, "TRIM support (0=off, 1=on)");
+
static int ide_set_dev_param_mask(const char *s, struct kernel_param *kp)
{
int a, b, i, j = 1;
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
