Re: [PATCH] ide write barrier support

[Jeff Garzik]

On Mon, Oct 13, 2003 at 04:08:58PM +0200, Jens Axboe wrote:
> +/*
> + * preempt pending requests, and store this cache flush for immediate
> + * execution
> + */
> +static struct request *ide_queue_flush_cmd(ide_drive_t *drive,
> +					   struct request *rq, int post)
> +{
> +	struct request *flush_rq = &HWGROUP(drive)->wrq;
> +
> +	blkdev_dequeue_request(rq);
> +
> +	memset(drive->special_buf, 0, sizeof(drive->special_buf));
> +
> +	ide_init_drive_cmd(flush_rq);
> +
> +	flush_rq->buffer = drive->special_buf;
> +	flush_rq->special = rq;
> +	flush_rq->buffer[0] = WIN_FLUSH_CACHE;
> +
> +	if (drive->id->cfs_enable_2 & 0x2400)
> +		flush_rq->buffer[0] = WIN_FLUSH_CACHE_EXT;
> +
> +	if (!post) {
> +		drive->doing_barrier = 1;
> +		flush_rq->flags |= REQ_BAR_PREFLUSH;
> +	} else
> +		flush_rq->flags |= REQ_BAR_POSTFLUSH;
> +
> +	flush_rq->flags |= REQ_STARTED;
> +	list_add(&flush_rq->queuelist, &drive->queue->queue_head);
> +	return flush_rq;
> +}

AFAICS you're missing some code that could be a major data corrupter:

FLUSH CACHE [EXT] may return before it's complete.  You need to create
an issue loop, that does FLUSH CACHE [EXT] and reads the result.  If the
result indicates the flush cache was partial, then you need to re-issue
the flush.  Lather, rinse, repeat until flush cache indicates all data
is really flushed.

	Jeff



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/