| Message ID | 20110126071626.GI27190@tux1.beaverton.ibm.com |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 25, 2011 at 11:16:26PM -0800, Darrick J. Wong wrote: > This patch is the first in a series to refactor the barrier= mount options out > of the filesystem code. This patch adds sysfs knobs to disable flush and FUA; > of course, the automatic default is the safe choice, i.e. to leave them > enabled. Obviously, only a seasoned administrator should ever be overriding > the defaults. Hmmm... wouldn't it be better to just export flush and fua instead of ignore_*? So that the admin can turn things on and off as [s]he seems fit? Also, it might be better to export them in a single attribute, say cache_control or something. Only subset of the combinations make sense anyway - none, flush, flush_fua. Thanks.
On Wed, Jan 26, 2011 at 10:30:51AM +0100, Tejun Heo wrote: > On Tue, Jan 25, 2011 at 11:16:26PM -0800, Darrick J. Wong wrote: > > This patch is the first in a series to refactor the barrier= mount options out > > of the filesystem code. This patch adds sysfs knobs to disable flush and FUA; > > of course, the automatic default is the safe choice, i.e. to leave them > > enabled. Obviously, only a seasoned administrator should ever be overriding > > the defaults. > > Hmmm... wouldn't it be better to just export flush and fua instead of > ignore_*? So that the admin can turn things on and off as [s]he seems I considered having a general knob to override the automatic FLUSH/FUA detection, but I thought that it wasn't a good idea to provide a mechanism to enable features that devices don't advertise. Mostly I was imagining horror scenarios like USB storage devices that claim no write cache and but then catch on fire if someone sends flush anyway. Not using advertised features seemed less risky. > fit? Also, it might be better to export them in a single attribute, > say cache_control or something. Only subset of the combinations make > sense anyway - none, flush, flush_fua. I agree. It could be simplified even further to a simple boolean that means "use neither" or "use whatever's supported". --D -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 25, 2011 at 11:16:26PM -0800, Darrick J. Wong wrote: > This patch is the first in a series to refactor the barrier= mount options out > of the filesystem code. This patch adds sysfs knobs to disable flush and FUA; If you add sysfs files, you must also add documentation for these files in Documentation/ABI/. Please do that in this patch series. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/block/blk-settings.c b/block/blk-settings.c index 36c8c1f..719c990 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -802,6 +802,7 @@ void blk_queue_flush(struct request_queue *q, unsigned int flush) flush &= ~REQ_FUA; q->flush_flags = flush & (REQ_FLUSH | REQ_FUA); + q->hw_flush_flags = q->flush_flags; } EXPORT_SYMBOL_GPL(blk_queue_flush); diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index 41fb691..af872a8 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -31,6 +31,58 @@ queue_var_store(unsigned long *var, const char *page, size_t count) return count; } +static ssize_t queue_flushflag_ignore_show(struct request_queue *q, char *page, + unsigned int which) +{ + if (!(q->hw_flush_flags & which)) + return sprintf(page, "0\n"); + + if (!(q->flush_flags & which)) + return sprintf(page, "1\n"); + + return sprintf(page, "0\n"); +} + +static ssize_t +queue_flushflag_ignore_store(struct request_queue *q, const char *page, + size_t count, unsigned int which) +{ + unsigned long ignore; + ssize_t ret = queue_var_store(&ignore, page, count); + + if (!(q->hw_flush_flags & which)) + return -EINVAL; + + if (ignore) + q->flush_flags &= ~which; + else + q->flush_flags |= which; + + return ret; +} + +static ssize_t queue_ignore_flush_show(struct request_queue *q, char *page) +{ + return queue_flushflag_ignore_show(q, page, REQ_FLUSH); +} + +static ssize_t queue_ignore_flush_store(struct request_queue *q, + const char *page, size_t count) +{ + return queue_flushflag_ignore_store(q, page, count, REQ_FLUSH); +} + +static ssize_t queue_ignore_fua_show(struct request_queue *q, char *page) +{ + return queue_flushflag_ignore_show(q, page, REQ_FUA); +} + +static ssize_t queue_ignore_fua_store(struct request_queue *q, + const char *page, size_t count) +{ + return queue_flushflag_ignore_store(q, page, count, REQ_FUA); +} + static ssize_t queue_requests_show(struct request_queue *q, char *page) { return queue_var_show(q->nr_requests, (page)); @@ -265,6 +317,18 @@ queue_rq_affinity_store(struct request_queue *q, const char *page, size_t count) return ret; } +static struct queue_sysfs_entry queue_ignore_flush_entry = { + .attr = {.name = "ignore_flush", .mode = S_IRUGO | S_IWUSR }, + .show = queue_ignore_flush_show, + .store = queue_ignore_flush_store, +}; + +static struct queue_sysfs_entry queue_ignore_fua_entry = { + .attr = {.name = "ignore_fua", .mode = S_IRUGO | S_IWUSR }, + .show = queue_ignore_fua_show, + .store = queue_ignore_fua_store, +}; + static struct queue_sysfs_entry queue_requests_entry = { .attr = {.name = "nr_requests", .mode = S_IRUGO | S_IWUSR }, .show = queue_requests_show, @@ -380,6 +444,8 @@ static struct queue_sysfs_entry queue_random_entry = { }; static struct attribute *default_attrs[] = { + &queue_ignore_flush_entry.attr, + &queue_ignore_fua_entry.attr, &queue_requests_entry.attr, &queue_ra_entry.attr, &queue_max_hw_sectors_entry.attr, diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 8a082a5..daa4e6b 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -368,6 +368,7 @@ struct request_queue * for flush operations */ unsigned int flush_flags; + unsigned int hw_flush_flags; unsigned int flush_pending_idx:1; unsigned int flush_running_idx:1; unsigned long flush_pending_since;
This patch is the first in a series to refactor the barrier= mount options out of the filesystem code. This patch adds sysfs knobs to disable flush and FUA; of course, the automatic default is the safe choice, i.e. to leave them enabled. Obviously, only a seasoned administrator should ever be overriding the defaults. Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> --- block/blk-settings.c | 1 + block/blk-sysfs.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/blkdev.h | 1 + 3 files changed, 68 insertions(+), 0 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html