On 06/27/2017 09:16 AM, Christoph Hellwig wrote:

On Tue, Jun 27, 2017 at 09:09:48AM -0600, Jens Axboe wrote:

quoted

On 06/27/2017 08:42 AM, Christoph Hellwig wrote:

quoted

The API looks ok, but the code could use some cleanups.  What do
you think about the incremental patch below:

It refactors various manipulations, and stores the write hint right
in the iocb as there is a 4 byte hole (this will need some minor
adjustments in the next patches):

How's this? Fixes for compile, and also squeeze an enum rw_hint into
a hole in the inode structure.

I'll refactor around this and squeeze into bio/rq holes as well, then
re-test it.

Looks good, minor nitpick below:

quoted

index 4574121f4746..4587a181162e 100644

--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -265,6 +265,20 @@ struct page;
 struct address_space;
 struct writeback_control;
 
+#include <linux/fcntl.h>

I didn't seem to need the move.  But if you want to move it can
we keep all the includes together at the very top?

It did here, we need it for the RWH_ defines or my compile blows up.
But yeah, let's just move it to the top, not sure why it's in the
middle.

quoted

+static inline enum rw_hint __inode_write_hint(struct inode *inode)
+{
+	return inode->i_write_hint;
+}
+
+static inline enum rw_hint inode_write_hint(struct inode *inode)
+{
+	enum rw_hint ret = __inode_write_hint(inode);
+	if (ret != WRITE_LIFE_NOT_SET)
+		return ret;
+	return WRITE_LIFE_NONE;
+}
+
+static inline enum rw_hint __file_write_hint(struct file *file)
+{
+	if (file->f_write_hint != WRITE_LIFE_NOT_SET)
+		return file->f_write_hint;
+
+	return __inode_write_hint(file_inode(file));
+}
+
+static inline enum rw_hint file_write_hint(struct file *file)
+{
+	enum rw_hint ret = __file_write_hint(file);
+	if (ret != WRITE_LIFE_NOT_SET)
+		return ret;
+	return WRITE_LIFE_NONE;
+}

I'd say kill all these helpers and just treat both WRITE_LIFE_NONE
and WRITE_LIFE_NOT_SET special all the way down in NVMe.

Sure, we can do that.

-- 
Jens Axboe