Re: [PATCH] notify block layer when using temporary change to cache_type

[Christoph Hellwig]

On Wed, May 28, 2014 at 12:58:27PM +0800, Vaughan Cao wrote:
> No, I just want to keep the modification as small as possible. Checked
> the actions of sd_revalidate_disk() again, it seems no harm to call it
> from here. Also actual actions are skipped in sd_read_cache_type() if
> cache_override!=0, so I suppose your original plan is to jump to
> sd_revalidate_disk() from here.

I think the right fix is to factor out a helper like:

static void sd_set_flush_flag(struct scsi_disk *sdkp)
{
	unsigned flush = 0;

	if (sdkp->WCE) {
		flush |= REQ_FLUSH;
		if (sdkp->DPOFUA)
			flush |= REQ_FUA;
	}

	blk_queue_flush(sdkp->disk->queue, flush);
}

which can be called from sd_revalidate_disk and cache_type_store.

> 
> However, that way may not be acceptable after further code review.
> Changing the sdkp->WCE,RCD will cause these real parameters of the
> underlying device *lost*. In sd_shutdown() and sd_suspend_common(), we
> need them to call sd_sync_cache() if necessary. I don't think this
> action is avoidable, just for the performance issue to solve. And we
> can't change the mode just by setting q->flush_flags while leave
> sdkp->WCE,RCD untouched either, because cache_type_show() needs
> sdkp->WCE,RCD to present the temporary config to userspace.

If the user overrides the cache type he should expect data not to be
flushed, so I don't really think this is na issue.  Even if it is it
should be a separate issue as it already exists before your patch.

--
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/