[PATCH v2] Support for write stream IDs

[PATCH v2] Support for write stream IDs

[Jens Axboe]

Hi,

v2 of this posting. Changes since v1:

- Rebased on top of current master.

- Fix EINVAL -> -EINVAL typo.

- Cleanup up BIO_STREAM_OFFSET definition.

- Pack i_streamid and f_streamid better into struct file and struct
  inode.

- Add a separate per-file hint, FADV_FILE_STREAMID. This only sets
  the write stream on the file, not the inode. FADV_STREAMID sets
  the hint both in the file and the inode.

 block/bio.c                  |    2 ++
 block/blk-core.c             |    3 +++
 fs/btrfs/extent_io.c         |    1 +
 fs/btrfs/inode.c             |    1 +
 fs/buffer.c                  |    4 ++--
 fs/direct-io.c               |    4 ++++
 fs/ext4/page-io.c            |    1 +
 fs/inode.c                   |    1 +
 fs/mpage.c                   |    1 +
 fs/open.c                    |    1 +
 fs/xfs/xfs_aops.c            |    1 +
 include/linux/blk_types.h    |   28 +++++++++++++++++++++++++++-
 include/linux/fs.h           |   22 ++++++++++++++++++++++
 include/uapi/linux/fadvise.h |    3 +++
 mm/fadvise.c                 |   25 +++++++++++++++++++++++++
 15 files changed, 95 insertions(+), 3 deletions(-)

[PATCH 1/7] block: add support for carrying a stream ID in a bio

[Jens Axboe]

The top bits of bio->bi_flags are reserved for keeping the
allocation pool, set aside the next eight bits for carrying
a stream ID. That leaves us with support for 255 streams,
0 is reserved as a "stream not set" value.

Add helpers for setting/getting stream ID of a bio.

Signed-off-by: Jens Axboe <axboe@fb.com>
---
 block/bio.c               |  2 ++
 block/blk-core.c          |  3 +++
 include/linux/blk_types.h | 28 +++++++++++++++++++++++++++-
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/block/bio.c b/block/bio.c
index f66a4eae16ee..1cd3d745047c 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -567,6 +567,7 @@ void __bio_clone_fast(struct bio *bio, struct bio *bio_src)
 	bio->bi_rw = bio_src->bi_rw;
 	bio->bi_iter = bio_src->bi_iter;
 	bio->bi_io_vec = bio_src->bi_io_vec;
+	bio_set_streamid(bio, bio_get_streamid(bio_src));
 }
 EXPORT_SYMBOL(__bio_clone_fast);
 
@@ -672,6 +673,7 @@ integrity_clone:
 		}
 	}
 
+	bio_set_streamid(bio, bio_get_streamid(bio_src));
 	return bio;
 }
 EXPORT_SYMBOL(bio_clone_bioset);
diff --git a/block/blk-core.c b/block/blk-core.c
index fd154b94447a..6276ce1ad46b 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1941,6 +1941,9 @@ void generic_make_request(struct bio *bio)
 	do {
 		struct request_queue *q = bdev_get_queue(bio->bi_bdev);
 
+		if (bio_streamid_valid(bio))
+			blk_add_trace_msg(q, "StreamID=%u", bio_get_streamid(bio));
+
 		q->make_request_fn(q, bio);
 
 		bio = bio_list_pop(current->bio_list);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index a1b25e35ea5f..0678c7baa7b1 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -138,9 +138,35 @@ struct bio {
 #define BIO_POOL_BITS		(4)
 #define BIO_POOL_NONE		((1UL << BIO_POOL_BITS) - 1)
 #define BIO_POOL_OFFSET		(BITS_PER_LONG - BIO_POOL_BITS)
-#define BIO_POOL_MASK		(1UL << BIO_POOL_OFFSET)
 #define BIO_POOL_IDX(bio)	((bio)->bi_flags >> BIO_POOL_OFFSET)
 
+/*
+ * after the pool bits, next 8 bits are for the stream id
+ */
+#define BIO_STREAM_BITS		(8)
+#define BIO_STREAM_OFFSET	(BIO_POOL_OFFSET - BIO_STREAM_BITS)
+#define BIO_STREAM_MASK		((1 << BIO_STREAM_BITS) - 1)
+
+static inline unsigned long streamid_to_flags(unsigned int id)
+{
+	return (unsigned long) (id & BIO_STREAM_MASK) << BIO_STREAM_OFFSET;
+}
+
+static inline void bio_set_streamid(struct bio *bio, unsigned int id)
+{
+	bio->bi_flags |= streamid_to_flags(id);
+}
+
+static inline unsigned int bio_get_streamid(struct bio *bio)
+{
+	return (bio->bi_flags >> BIO_STREAM_OFFSET) & BIO_STREAM_MASK;
+}
+
+static inline bool bio_streamid_valid(struct bio *bio)
+{
+	return bio_get_streamid(bio) != 0;
+}
+
 #endif /* CONFIG_BLOCK */
 
 /*
-- 
2.4.0.rc2.1.g3d6bc9a

[PATCH 2/7] Add support for per-file/inode stream ID

[Jens Axboe]

Writing on flash devices can be much more efficient, if we can
inform the device what kind of data can be grouped together. If
the device is able to group data together with similar lifetimes,
then it can be more efficient in garbage collection. This, in turn,
leads to lower write amplification, which is a win on both device
wear and performance.

Add a new fadvise hint, POSIX_FADV_STREAMID, which sets the file
and inode streamid. The file streamid is used if we have the file
available at the time of the write (O_DIRECT), we use the inode
streamid if not (buffered writeback). The fadvise hint uses the
'offset' field to specify a stream ID. A second
POSIX_FADV_FILE_STREAMID sets only the stream ID on the file,
not the inode.

Signed-off-by: Jens Axboe <axboe@fb.com>
---
 fs/inode.c                   |  1 +
 fs/open.c                    |  1 +
 include/linux/fs.h           | 22 ++++++++++++++++++++++
 include/uapi/linux/fadvise.h |  3 +++
 mm/fadvise.c                 | 25 +++++++++++++++++++++++++
 5 files changed, 52 insertions(+)

diff --git a/fs/inode.c b/fs/inode.c
index f00b16f45507..41885322ba64 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -149,6 +149,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
 	inode->i_blocks = 0;
 	inode->i_bytes = 0;
 	inode->i_generation = 0;
+	inode->i_streamid = 0;
 	inode->i_pipe = NULL;
 	inode->i_bdev = NULL;
 	inode->i_cdev = NULL;
diff --git a/fs/open.c b/fs/open.c
index 6796f04d6032..979f34ff165b 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -743,6 +743,7 @@ static int do_dentry_open(struct file *f,
 	f->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);
 
 	file_ra_state_init(&f->f_ra, f->f_mapping->host->i_mapping);
+	f->f_streamid = 0;
 
 	return 0;
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c7496f263860..f43b866c3639 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -648,6 +648,7 @@ struct inode {
 #ifdef CONFIG_IMA
 	atomic_t		i_readcount; /* struct files open RO */
 #endif
+	unsigned int		i_streamid;
 	const struct file_operations	*i_fop;	/* former ->i_op->default_file_ops */
 	struct file_lock_context	*i_flctx;
 	struct address_space	i_data;
@@ -668,6 +669,14 @@ struct inode {
 	void			*i_private; /* fs or device private pointer */
 };
 
+static inline unsigned int inode_streamid(struct inode *inode)
+{
+	if (inode)
+		return inode->i_streamid;
+
+	return 0;
+}
+
 static inline int inode_unhashed(struct inode *inode)
 {
 	return hlist_unhashed(&inode->i_hash);
@@ -839,6 +848,7 @@ struct file {
 	 * Must not be taken from IRQ context.
 	 */
 	spinlock_t		f_lock;
+	unsigned int		f_streamid;
 	atomic_long_t		f_count;
 	unsigned int 		f_flags;
 	fmode_t			f_mode;
@@ -870,6 +880,18 @@ struct file_handle {
 	unsigned char f_handle[0];
 };
 
+/*
+ * If the file doesn't have a stream ID set, return the inode stream ID
+ * in case that has been set.
+ */
+static inline unsigned int file_streamid(struct file *f)
+{
+	if (f->f_streamid)
+		return f->f_streamid;
+
+	return inode_streamid(f->f_inode);
+}
+
 static inline struct file *get_file(struct file *f)
 {
 	atomic_long_inc(&f->f_count);
diff --git a/include/uapi/linux/fadvise.h b/include/uapi/linux/fadvise.h
index e8e747139b9a..e6e561db7f88 100644
--- a/include/uapi/linux/fadvise.h
+++ b/include/uapi/linux/fadvise.h
@@ -18,4 +18,7 @@
 #define POSIX_FADV_NOREUSE	5 /* Data will be accessed once.  */
 #endif
 
+#define POSIX_FADV_STREAMID	8 /* associate stream ID with file+inode */
+#define POSIX_FADV_FILE_STREAMID 9 /* associate stream ID with file */
+
 #endif	/* FADVISE_H_INCLUDED */
diff --git a/mm/fadvise.c b/mm/fadvise.c
index 4a3907cf79f8..a56e81840040 100644
--- a/mm/fadvise.c
+++ b/mm/fadvise.c
@@ -60,6 +60,8 @@ SYSCALL_DEFINE4(fadvise64_64, int, fd, loff_t, offset, loff_t, len, int, advice)
 		case POSIX_FADV_WILLNEED:
 		case POSIX_FADV_NOREUSE:
 		case POSIX_FADV_DONTNEED:
+		case POSIX_FADV_STREAMID:
+		case POSIX_FADV_FILE_STREAMID:
 			/* no bad return value, but ignore advice */
 			break;
 		default:
@@ -144,6 +146,29 @@ SYSCALL_DEFINE4(fadvise64_64, int, fd, loff_t, offset, loff_t, len, int, advice)
 			}
 		}
 		break;
+	case POSIX_FADV_STREAMID:
+	case POSIX_FADV_FILE_STREAMID:
+		/*
+		 * streamid is stored in offset... we don't limit or check
+		 * if the device supports streams, or if it does, if the
+		 * stream nr is within the limits. 1 is the lowest valid
+		 * stream id, 0 is "don't care/know".
+		 */
+		if (offset != (unsigned int) offset) {
+			ret = -EINVAL;
+			break;
+		}
+		/*
+		 * FILE_STREAMID stores only in the file, STREAMID stores
+		 * the stream hint in both the file and the inode.
+		 */
+		f.file->f_streamid = offset;
+		if (advice == POSIX_FADV_STREAMID) {
+			spin_lock(&inode->i_lock);
+			inode->i_streamid = offset;
+			spin_unlock(&inode->i_lock);
+		}
+		break;
 	default:
 		ret = -EINVAL;
 	}
-- 
2.4.0.rc2.1.g3d6bc9a

[PATCH 3/7] direct-io: add support for write stream IDs

[Jens Axboe]

Get the streamid from the file, if any, and set it on the bio.

Signed-off-by: Jens Axboe <axboe@fb.com>
---
 fs/direct-io.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index c3b560b24a46..d318a143b186 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -76,6 +76,7 @@ struct dio_submit {
 	int reap_counter;		/* rate limit reaping */
 	sector_t final_block_in_request;/* doesn't change */
 	int boundary;			/* prev block is at a boundary */
+	int streamid;			/* Write stream ID */
 	get_block_t *get_block;		/* block mapping function */
 	dio_submit_t *submit_io;	/* IO submition function */
 
@@ -371,6 +372,8 @@ dio_bio_alloc(struct dio *dio, struct dio_submit *sdio,
 
 	sdio->bio = bio;
 	sdio->logical_offset_in_bio = sdio->cur_page_fs_offset;
+
+	bio_set_streamid(bio, sdio->streamid);
 }
 
 /*
@@ -1201,6 +1204,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
 	sdio.blkbits = blkbits;
 	sdio.blkfactor = i_blkbits - blkbits;
 	sdio.block_in_file = offset >> blkbits;
+	sdio.streamid = file_streamid(iocb->ki_filp);
 
 	sdio.get_block = get_block;
 	dio->end_io = end_io;
-- 
2.4.0.rc2.1.g3d6bc9a

[PATCH 4/7] Add stream ID support for buffered mpage/__block_write_full_page()

[Jens Axboe]

Pass on the inode stream ID to the bio allocation.

Signed-off-by: Jens Axboe <axboe@fb.com>
---
 fs/buffer.c | 4 ++--
 fs/mpage.c  | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index c7a5602d01ee..5191523cec56 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1774,7 +1774,7 @@ static int __block_write_full_page(struct inode *inode, struct page *page,
 	do {
 		struct buffer_head *next = bh->b_this_page;
 		if (buffer_async_write(bh)) {
-			submit_bh(write_op, bh);
+			_submit_bh(write_op, bh, streamid_to_flags(inode_streamid(inode)));
 			nr_underway++;
 		}
 		bh = next;
@@ -1828,7 +1828,7 @@ recover:
 		struct buffer_head *next = bh->b_this_page;
 		if (buffer_async_write(bh)) {
 			clear_buffer_dirty(bh);
-			submit_bh(write_op, bh);
+			_submit_bh(write_op, bh, streamid_to_flags(inode_streamid(inode)));
 			nr_underway++;
 		}
 		bh = next;
diff --git a/fs/mpage.c b/fs/mpage.c
index 3e79220babac..fba13f4b981d 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -605,6 +605,7 @@ alloc_new:
 				bio_get_nr_vecs(bdev), GFP_NOFS|__GFP_HIGH);
 		if (bio == NULL)
 			goto confused;
+		bio_set_streamid(bio, inode_streamid(inode));
 	}
 
 	/*
-- 
2.4.0.rc2.1.g3d6bc9a

[PATCH 5/7] btrfs: add support for write stream IDs

[Jens Axboe]

Both buffered and O_DIRECT.

Signed-off-by: Jens Axboe <axboe@fb.com>
Acked-by: Chris Mason <clm@fb.com> 
---
 fs/btrfs/extent_io.c | 1 +
 fs/btrfs/inode.c     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index d688cfe5d496..2845fae054b6 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2838,6 +2838,7 @@ static int submit_extent_page(int rw, struct extent_io_tree *tree,
 	bio_add_page(bio, page, page_size, offset);
 	bio->bi_end_io = end_io_func;
 	bio->bi_private = tree;
+	bio_set_streamid(bio, inode_streamid(page->mapping->host));
 
 	if (bio_ret)
 		*bio_ret = bio;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 43192e10cc43..8ff7527e8d05 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8046,6 +8046,7 @@ static void btrfs_submit_direct(int rw, struct bio *dio_bio,
 	atomic_set(&dip->pending_bios, 0);
 	btrfs_bio = btrfs_io_bio(io_bio);
 	btrfs_bio->logical = file_offset;
+	bio_set_streamid(io_bio, inode_streamid(inode));
 
 	if (write) {
 		io_bio->bi_end_io = btrfs_endio_direct_write;
-- 
2.4.0.rc2.1.g3d6bc9a

[PATCH 6/7] xfs: add support for buffered writeback stream ID

[Jens Axboe]

Signed-off-by: Jens Axboe <axboe@fb.com>
---
 fs/xfs/xfs_aops.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 1d8eef9cf0f5..6d166e55de9a 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -376,6 +376,7 @@ xfs_submit_ioend_bio(
 	atomic_inc(&ioend->io_remaining);
 	bio->bi_private = ioend;
 	bio->bi_end_io = xfs_end_bio;
+	bio_set_streamid(bio, ioend->io_inode->i_streamid);
 	submit_bio(wbc->sync_mode == WB_SYNC_ALL ? WRITE_SYNC : WRITE, bio);
 }
 
-- 
2.4.0.rc2.1.g3d6bc9a

[PATCH 7/7] ext4: add support for write stream IDs

[Jens Axboe]

Signed-off-by: Jens Axboe <axboe@fb.com>
---
 fs/ext4/page-io.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 464984261e69..392a82925d5f 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -389,6 +389,7 @@ submit_and_retry:
 		ret = io_submit_init_bio(io, bh);
 		if (ret)
 			return ret;
+		bio_set_streamid(io->io_bio, inode_streamid(inode));
 	}
 	ret = bio_add_page(io->io_bio, bh->b_page, bh->b_size, bh_offset(bh));
 	if (ret != bh->b_size)
-- 
2.4.0.rc2.1.g3d6bc9a

[PATCH v2] Support for write stream IDs

[Jens Axboe]

Hi,

Changes since the last posting:

- Added a specific per-file fadvise setting. POSIX_FADV_STREAMID sets
  the inode and file stream ID, POSIX_FADV_STREAMID_FILE sets just the
  file stream ID.

- Addressed review comments.

I've since run some testing with write streams. Test case was a RocksDB
overwrite benchmark, using 3 billion keys of 400B in size (numbers set
use the full size of the device). WAL/LOG was assigned to stream 1, and
each RocksDB compaction level used a separate stream. With streams
enabled, user write to device writes (write amplification) was at 2.33.
Without streams, the write amplification was 3.05. That is roughly 20%
less written NAND, and the streams test subsequently also had 20%
higher throughput.

Unless there are any grave concerns here, I'd like to merge this for
4.2.

-- 
Jens Axboe

[PATCH 1/7] block: add support for carrying a stream ID in a bio

[Jens Axboe]

The top bits of bio->bi_flags are reserved for keeping the
allocation pool, set aside the next eight bits for carrying
a stream ID. That leaves us with support for 255 streams,
0 is reserved as a "stream not set" value.

Add helpers for setting/getting stream ID of a bio.

Signed-off-by: Jens Axboe <axboe@fb.com>
---
 block/bio.c               |  2 ++
 block/blk-core.c          |  3 +++
 include/linux/blk_types.h | 28 +++++++++++++++++++++++++++-
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/block/bio.c b/block/bio.c
index f66a4eae16ee..1cd3d745047c 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -567,6 +567,7 @@ void __bio_clone_fast(struct bio *bio, struct bio *bio_src)
 	bio->bi_rw = bio_src->bi_rw;
 	bio->bi_iter = bio_src->bi_iter;
 	bio->bi_io_vec = bio_src->bi_io_vec;
+	bio_set_streamid(bio, bio_get_streamid(bio_src));
 }
 EXPORT_SYMBOL(__bio_clone_fast);
 
@@ -672,6 +673,7 @@ integrity_clone:
 		}
 	}
 
+	bio_set_streamid(bio, bio_get_streamid(bio_src));
 	return bio;
 }
 EXPORT_SYMBOL(bio_clone_bioset);
diff --git a/block/blk-core.c b/block/blk-core.c
index fd154b94447a..6276ce1ad46b 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1941,6 +1941,9 @@ void generic_make_request(struct bio *bio)
 	do {
 		struct request_queue *q = bdev_get_queue(bio->bi_bdev);
 
+		if (bio_streamid_valid(bio))
+			blk_add_trace_msg(q, "StreamID=%u", bio_get_streamid(bio));
+
 		q->make_request_fn(q, bio);
 
 		bio = bio_list_pop(current->bio_list);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index a1b25e35ea5f..0678c7baa7b1 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -138,9 +138,35 @@ struct bio {
 #define BIO_POOL_BITS		(4)
 #define BIO_POOL_NONE		((1UL << BIO_POOL_BITS) - 1)
 #define BIO_POOL_OFFSET		(BITS_PER_LONG - BIO_POOL_BITS)
-#define BIO_POOL_MASK		(1UL << BIO_POOL_OFFSET)
 #define BIO_POOL_IDX(bio)	((bio)->bi_flags >> BIO_POOL_OFFSET)
 
+/*
+ * after the pool bits, next 8 bits are for the stream id
+ */
+#define BIO_STREAM_BITS		(8)
+#define BIO_STREAM_OFFSET	(BIO_POOL_OFFSET - BIO_STREAM_BITS)
+#define BIO_STREAM_MASK		((1 << BIO_STREAM_BITS) - 1)
+
+static inline unsigned long streamid_to_flags(unsigned int id)
+{
+	return (unsigned long) (id & BIO_STREAM_MASK) << BIO_STREAM_OFFSET;
+}
+
+static inline void bio_set_streamid(struct bio *bio, unsigned int id)
+{
+	bio->bi_flags |= streamid_to_flags(id);
+}
+
+static inline unsigned int bio_get_streamid(struct bio *bio)
+{
+	return (bio->bi_flags >> BIO_STREAM_OFFSET) & BIO_STREAM_MASK;
+}
+
+static inline bool bio_streamid_valid(struct bio *bio)
+{
+	return bio_get_streamid(bio) != 0;
+}
+
 #endif /* CONFIG_BLOCK */
 
 /*
-- 
2.4.0.rc2.1.g3d6bc9a

[PATCH RFC v2] Support for write stream IDs

[Jens Axboe]

One of the things that exacerbates write amplification on flash
based devices is that fact that data with different lifetimes get
grouped together on media. Currently we have no interface that
applications can use to separate different types of writes. This
patch set adds support for that.

The kernel has no knowledge of what stream ID is what. The idea is
that writes with identical stream IDs have similar life times, not
that stream ID 'X' has a shorter lifetime than stream ID 'X+1'.

There are basically two interfaces that could be used for this. One
is fcntl, the other is fadvise. This patchset uses fadvise, with a
new POSIX_FADV_STREAMID hint. The 'offset' field is used to pass
the relevant stream ID. Switching to fcntl (with a SET/GET_STREAMID)
would be trivial.

The patchset wires up the block parts, adds buffered and O_DIRECT
support, and modifies btrfs/xfs too. It should be trivial to extend
this to all other file systems, I just used xfs and btrfs for testing.

No block drivers are wired up yet. Patches are against current -git.

Changes since v1:

- Don't add streamid to struct writeback_control, always use the one
  in the inode for buffered writeback
- Add file_streamid() helper. Returns file streamid, if set, otherwise
  it returns the inode streamid.
- Update btrfs/xfs for wbc->streamid change.
- Add streamid to ext4.
- Bump stream bits from 3 to 8, 255 streams are now supported.

[PATCH 1/7] block: add support for carrying a stream ID in a bio

[Jens Axboe]

The top bits of bio->bi_flags are reserved for keeping the
allocation pool, set aside the next eight bits for carrying
a stream ID. That leaves us with support for 255 streams,
0 is reserved as a "stream not set" value.

Add helpers for setting/getting stream ID of a bio.

Signed-off-by: Jens Axboe <axboe@fb.com>
---
 block/bio.c               |  2 ++
 block/blk-core.c          |  3 +++
 include/linux/blk_types.h | 28 +++++++++++++++++++++++++++-
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/block/bio.c b/block/bio.c
index f66a4eae16ee..1cd3d745047c 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -567,6 +567,7 @@ void __bio_clone_fast(struct bio *bio, struct bio *bio_src)
 	bio->bi_rw = bio_src->bi_rw;
 	bio->bi_iter = bio_src->bi_iter;
 	bio->bi_io_vec = bio_src->bi_io_vec;
+	bio_set_streamid(bio, bio_get_streamid(bio_src));
 }
 EXPORT_SYMBOL(__bio_clone_fast);
 
@@ -672,6 +673,7 @@ integrity_clone:
 		}
 	}
 
+	bio_set_streamid(bio, bio_get_streamid(bio_src));
 	return bio;
 }
 EXPORT_SYMBOL(bio_clone_bioset);
diff --git a/block/blk-core.c b/block/blk-core.c
index 794c3e7f01cf..6b7b8c95c4c3 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1928,6 +1928,9 @@ void generic_make_request(struct bio *bio)
 	do {
 		struct request_queue *q = bdev_get_queue(bio->bi_bdev);
 
+		if (bio_streamid_valid(bio))
+			blk_add_trace_msg(q, "StreamID=%u", bio_get_streamid(bio));
+
 		q->make_request_fn(q, bio);
 
 		bio = bio_list_pop(current->bio_list);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index c294e3e25e37..d6909007a3fe 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -138,9 +138,35 @@ struct bio {
 #define BIO_POOL_BITS		(4)
 #define BIO_POOL_NONE		((1UL << BIO_POOL_BITS) - 1)
 #define BIO_POOL_OFFSET		(BITS_PER_LONG - BIO_POOL_BITS)
-#define BIO_POOL_MASK		(1UL << BIO_POOL_OFFSET)
 #define BIO_POOL_IDX(bio)	((bio)->bi_flags >> BIO_POOL_OFFSET)
 
+/*
+ * after the pool bits, next 8 bits are for the stream id
+ */
+#define BIO_STREAM_BITS		(8)
+#define BIO_STREAM_OFFSET	(BITS_PER_LONG - 12)
+#define BIO_STREAM_MASK		((1 << BIO_STREAM_BITS) - 1)
+
+static inline unsigned long streamid_to_flags(unsigned int id)
+{
+	return (unsigned long) (id & BIO_STREAM_MASK) << BIO_STREAM_OFFSET;
+}
+
+static inline void bio_set_streamid(struct bio *bio, unsigned int id)
+{
+	bio->bi_flags |= streamid_to_flags(id);
+}
+
+static inline unsigned int bio_get_streamid(struct bio *bio)
+{
+	return (bio->bi_flags >> BIO_STREAM_OFFSET) & BIO_STREAM_MASK;
+}
+
+static inline bool bio_streamid_valid(struct bio *bio)
+{
+	return bio_get_streamid(bio) != 0;
+}
+
 #endif /* CONFIG_BLOCK */
 
 /*
-- 
1.9.1

Re: [PATCH 1/7] block: add support for carrying a stream ID in a bio

[Andreas Dilger]

On Mar 25, 2015, at 9:07 AM, Jens Axboe <axboe@fb.com> wrote:
> 
> The top bits of bio->bi_flags are reserved for keeping the
> allocation pool, set aside the next eight bits for carrying
> a stream ID. That leaves us with support for 255 streams,
> 0 is reserved as a "stream not set" value.

I understand that the stream ID is not related to specific priority
of an IO request.  However, I'm wondering how this patch series
interacts with some of the other patch series that add cache priority
hints?  Is there a danger of running out of space in the IO pipeline
for the additional cache hints if this is using 8 bits?

Some more comments inline.

> Add helpers for setting/getting stream ID of a bio.
> 
> Signed-off-by: Jens Axboe <axboe@fb.com>
> ---
> block/bio.c               |  2 ++
> block/blk-core.c          |  3 +++
> include/linux/blk_types.h | 28 +++++++++++++++++++++++++++-
> 3 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index f66a4eae16ee..1cd3d745047c 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -567,6 +567,7 @@ void __bio_clone_fast(struct bio *bio, struct bio *bio_src)
> 	bio->bi_rw = bio_src->bi_rw;
> 	bio->bi_iter = bio_src->bi_iter;
> 	bio->bi_io_vec = bio_src->bi_io_vec;
> +	bio_set_streamid(bio, bio_get_streamid(bio_src));
> }
> EXPORT_SYMBOL(__bio_clone_fast);
> 
> @@ -672,6 +673,7 @@ integrity_clone:
> 		}
> 	}
> 
> +	bio_set_streamid(bio, bio_get_streamid(bio_src));
> 	return bio;
> }
> EXPORT_SYMBOL(bio_clone_bioset);
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 794c3e7f01cf..6b7b8c95c4c3 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -1928,6 +1928,9 @@ void generic_make_request(struct bio *bio)
> 	do {
> 		struct request_queue *q = bdev_get_queue(bio->bi_bdev);
> 
> +		if (bio_streamid_valid(bio))
> +			blk_add_trace_msg(q, "StreamID=%u", bio_get_streamid(bio));
> +
> 		q->make_request_fn(q, bio);
> 
> 		bio = bio_list_pop(current->bio_list);
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index c294e3e25e37..d6909007a3fe 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -138,9 +138,35 @@ struct bio {
> #define BIO_POOL_BITS		(4)
> #define BIO_POOL_NONE		((1UL << BIO_POOL_BITS) - 1)
> #define BIO_POOL_OFFSET		(BITS_PER_LONG - BIO_POOL_BITS)
> -#define BIO_POOL_MASK		(1UL << BIO_POOL_OFFSET)
> #define BIO_POOL_IDX(bio)	((bio)->bi_flags >> BIO_POOL_OFFSET)
> 
> +/*
> + * after the pool bits, next 8 bits are for the stream id
> + */
> +#define BIO_STREAM_BITS		(8)
> +#define BIO_STREAM_OFFSET	(BITS_PER_LONG - 12)

Should this really be:

#define BIO_STREAM_OFFSET	(BIO_POOL_OFFSET - BIO_STREAM_BITS)

Otherwise there is a risk of conflict if someone changes BIO_POOL_BITS.

Cheers, Andreas

> +#define BIO_STREAM_MASK		((1 << BIO_STREAM_BITS) - 1)
> +
> +static inline unsigned long streamid_to_flags(unsigned int id)
> +{
> +	return (unsigned long) (id & BIO_STREAM_MASK) << BIO_STREAM_OFFSET;
> +}
> +
> +static inline void bio_set_streamid(struct bio *bio, unsigned int id)
> +{
> +	bio->bi_flags |= streamid_to_flags(id);
> +}
> +
> +static inline unsigned int bio_get_streamid(struct bio *bio)
> +{
> +	return (bio->bi_flags >> BIO_STREAM_OFFSET) & BIO_STREAM_MASK;
> +}
> +
> +static inline bool bio_streamid_valid(struct bio *bio)
> +{
> +	return bio_get_streamid(bio) != 0;
> +}
> +
> #endif /* CONFIG_BLOCK */
> 
> /*
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas

Re: [PATCH 1/7] block: add support for carrying a stream ID in a bio

[Jens Axboe]

On 04/09/2015 04:46 PM, Andreas Dilger wrote:
> On Mar 25, 2015, at 9:07 AM, Jens Axboe <axboe@fb.com> wrote:
>>
>> The top bits of bio->bi_flags are reserved for keeping the
>> allocation pool, set aside the next eight bits for carrying
>> a stream ID. That leaves us with support for 255 streams,
>> 0 is reserved as a "stream not set" value.
>
> I understand that the stream ID is not related to specific priority
> of an IO request.  However, I'm wondering how this patch series
> interacts with some of the other patch series that add cache priority
> hints?  Is there a danger of running out of space in the IO pipeline
> for the additional cache hints if this is using 8 bits?

That's always a risk, of course, but that goes for most features that 
need to carry more data in struct bio (or elsewhere). Otherwise we'll 
have to bite the bullet and add a new field.

>> +/*
>> + * after the pool bits, next 8 bits are for the stream id
>> + */
>> +#define BIO_STREAM_BITS		(8)
>> +#define BIO_STREAM_OFFSET	(BITS_PER_LONG - 12)
>
> Should this really be:
>
> #define BIO_STREAM_OFFSET	(BIO_POOL_OFFSET - BIO_STREAM_BITS)
>
> Otherwise there is a risk of conflict if someone changes BIO_POOL_BITS.

Good point, that would be cleaner. I'll make that change.

-- 
Jens Axboe