lkml.org 
[lkml]   [2009]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 4/5] ide: Add support for TRIM
Date
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... ]


\
 
 \ /
  Last update: 2009-04-02 21:41    [W:0.503 / U:0.248 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site