Support for write stream IDs
Subsystems (7)
RFC PATCH v2 1/1 Support for write stream IDs
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
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.
--- block/bio.c | 2 ++ block/blk-core.c | 3 +++ include/linux/blk_types.h | 28 +++++++++++++++++++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-)
block/bio.c +2 -0
@@ -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);
block/blk-core.c +3 -0
@@ -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);
include/linux/blk_types.h +27 -1
@@ -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 */ /*
Signature · 1 line
1.9.1
Re: [PATCH 1/7] block: add support for carrying a stream ID in a bio
Quoted 5 lines · Jens Axboe
The top bits of
bio->bi_flagsare 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.
Quoted 49 lines
Add helpers for setting/getting stream ID of a bio.
Signed-off-by Jens Axboe <[email protected]>---
block/bio.c | 2 ++ block/blk-core.c | 3 +++ include/linux/blk_types.h | 28 +++++++++++++++++++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-)block/bio.c +2
-0@@ -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);block/blk-core.c +3
-0@@ -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);include/linux/blk_types.h +0
-0@@ -138,9 +138,35 @@ struct bio {#define BIO_POOL_BITS (4)
include/linux/blk_types.h struct bio { · line 147
Quoted 10 lines
include/linux/blk_types.h +5
-1#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
Quoted 32 lines
include/linux/blk_types.h +22
-0+#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 */ /* Signature · 6 lines
1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected] 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
Quoted 7 lines · Andreas Dilger
Quoted 5 lines · Jens Axboe
The top bits of
bio->bi_flagsare 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.
Quoted 5 lines
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.
Quoted 11 lines
include/linux/blk_types.h struct bio { · line 147
Quoted 5 lines
include/linux/blk_types.h +5
-0+/* +* 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.
Signature · 1 line
Jens Axboe
PATCH 2/7 Add support for per-file stream ID
In reply to Jens AxboeWriting 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.
--- fs/inode.c | 1 + fs/open.c | 1 + include/linux/fs.h | 23 +++++++++++++++++++++++ include/uapi/linux/fadvise.h | 2 ++ mm/fadvise.c | 17 +++++++++++++++++ 5 files changed, 44 insertions(+)
fs/inode.c +1 -0
@@ -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 ;
fs/open.c +1 -0
@@ -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 ;
include/linux/fs.h +23 -0
@@ -631,6 +631,7 @@ struct inode { };__u32 i_generation; +unsigned int i_streamid;#ifdef CONFIG_FSNOTIFY __u32 i_fsnotify_mask;/* all events this inode cares about */
@@ -640,6 +641,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);
@@ -820,6 +829,8 @@ struct file {const struct cred *f_cred;struct file_ra_state f_ra; +unsigned int f_streamid; +u64 f_version;#ifdef CONFIG_SECURITY void *f_security;
@@ -842,6 +853,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);
include/uapi/linux/fadvise.h +2 -0
@@ -18,4 +18,6 @@#define POSIX_FADV_NOREUSE 5/* Data will be accessed once. */ #endif +#define POSIX_FADV_STREAMID 8/* associate stream ID with file */ +#endif /* FADVISE_H_INCLUDED */
mm/fadvise.c +17 -0
@@ -60,6 +60,7 @@ 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 :/* no bad return value, but ignore advice */ break ;default :
@@ -144,6 +145,22 @@ SYSCALL_DEFINE4(fadvise64_64, int, fd, loff_t, offset, loff_t, len, int, advice) } }break ; +case POSIX_FADV_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 ; +else { + f.file->f_streamid = offset; +spin_lock (&inode->i_lock); + inode->i_streamid = offset; +spin_unlock (&inode->i_lock); + } +break ;default : ret = -EINVAL ; }
Signature · 1 line
1.9.1
Re: [PATCH 2/7] Add support for per-file stream ID
Jens Axboe <[email protected]> writes:
One small question.
You states that all IDs are equals but can we reserve some IDs for internal kernel purposes. For example very short lived data (files opened with O_TEMP) and so on.
Also small nitpicking see below.
Quoted 127 lines
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.Signed-off-by Jens Axboe <[email protected]>--- fs/inode.c | 1 + fs/open.c | 1 + include/linux/fs.h | 23 +++++++++++++++++++++++ include/uapi/linux/fadvise.h | 2 ++ mm/fadvise.c | 17 +++++++++++++++++ 5 files changed, 44 insertions(+)fs/inode.c +1
-0@@ -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 ;fs/open.c +1
-0@@ -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 ;include/linux/fs.h +23
-0@@ -631,6 +631,7 @@ struct inode { };__u32 i_generation; +unsigned int i_streamid;#ifdef CONFIG_FSNOTIFY __u32 i_fsnotify_mask;/* all events this inode cares about */ @@ -640,6 +641,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);@@ -820,6 +829,8 @@ struct file {const struct cred *f_cred;struct file_ra_state f_ra; +unsigned int f_streamid; +u64 f_version;#ifdef CONFIG_SECURITY void *f_security;@@ -842,6 +853,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);include/uapi/linux/fadvise.h +2
-0@@ -18,4 +18,6 @@#define POSIX_FADV_NOREUSE 5/* Data will be accessed once. */ #endif +#define POSIX_FADV_STREAMID 8/* associate stream ID with file */ +#endif /* FADVISE_H_INCLUDED */ mm/fadvise.c +1
-0@@ -60,6 +60,7 @@ 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 :/* no bad return value, but ignore advice */ break ;default :@@ -144,6 +145,22 @@ SYSCALL_DEFINE4(fadvise64_64, int, fd, loff_t, offset, loff_t, len, int, advice) } }
mm/fadvise.c SYSCALL_DEFINE4(fadvise64_64, int, fd, loff_t, offset, loff_t, len, int, advice) · line 156
Quoted 10 lines
mm/fadvise.c +9
-0break ; +case POSIX_FADV_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 ;
Shuld be negative ret = -EINVAL;
Quoted 17 lines
mm/fadvise.c +7
-0+else { + f.file->f_streamid = offset; +spin_lock (&inode->i_lock); + inode->i_streamid = offset; +spin_unlock (&inode->i_lock); + } +break ;default : ret = -EINVAL ; }Signature · 6 lines
1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] Add support for per-file stream ID
Quoted 6 lines · Dmitry Monakhov
Jens Axboe <[email protected]> writes:
One small question.
You states that all IDs are equals but can we reserve some IDs for internal kernel purposes. For example very short lived data (files opened withO_TEMP) and so on.
Yes, we probably should end up reserving some IDs for specific internal things. O_TEMP to a specific stream would make sense. Journal writes too, for instance.
I just preferred not wiring any of that up, as it then gets closer to being a policy decision. I'm mainly interested in getting this exposed to userspace, and seeing how the hw side develops since that will influence how we actually use this (number of streams, any actions required to manage streams, etc).
Quoted 10 lines
mm/fadvise.c SYSCALL_DEFINE4(fadvise64_64, int, fd, loff_t, offset, loff_t, len, int, advice) · line 156
Quoted 9 lines
mm/fadvise.c +9
-0+case POSIX_FADV_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 ;Shuld be negative ret =
-EINVAL;
Indeed it should, thanks!
Signature · 1 line
Jens Axboe
Re: [PATCH 2/7] Add support for per-file stream ID
In reply to Jens AxboeQuoted 52 lines · 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.Signed-off-by Jens Axboe <[email protected]>---
fs/inode.c | 1 + fs/open.c | 1 + include/linux/fs.h | 23 +++++++++++++++++++++++ include/uapi/linux/fadvise.h | 2 ++ mm/fadvise.c | 17 +++++++++++++++++ 5 files changed, 44 insertions(+)fs/inode.c +1
-0@@ -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 ;fs/open.c +1
-0@@ -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 ;include/linux/fs.h +0
-0
include/linux/fs.h struct inode { · line 634
Quoted 4 lines
include/linux/fs.h +1
-0@@ -631,6 +631,7 @@ struct inode { };__u32 i_generation; +unsigned int i_streamid;
Since there are only 8 bits of streamid being passed from userspace, is it possible to declare this as a char and pack it into a hole so that it doesn't increase the inode size for a functionality that most people won't be using? Maybe after i_bytes? That could be increased to unsigned short if needed without increasing the size of the inode.
Cheers, Andreas
Quoted 97 lines
include/linux/fs.h +21
-0#ifdef CONFIG_FSNOTIFY __u32 i_fsnotify_mask;/* all events this inode cares about */ @@ -640,6 +641,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);@@ -820,6 +829,8 @@ struct file {const struct cred *f_cred;struct file_ra_state f_ra; +unsigned int f_streamid;u64 f_version;#ifdef CONFIG_SECURITY void *f_security;@@ -842,6 +853,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);include/uapi/linux/fadvise.h +2
-0@@ -18,4 +18,6 @@#define POSIX_FADV_NOREUSE 5/* Data will be accessed once. */ #endif +#define POSIX_FADV_STREAMID 8/* associate stream ID with file */ +#endif /* FADVISE_H_INCLUDED */ mm/fadvise.c +17
-0@@ -60,6 +60,7 @@ 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 :/* no bad return value, but ignore advice */ break ;default :@@ -144,6 +145,22 @@ SYSCALL_DEFINE4(fadvise64_64, int, fd, loff_t, offset, loff_t, len, int, advice) } }break ; +case POSIX_FADV_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 ; +else { + f.file->f_streamid = offset; +spin_lock (&inode->i_lock); + inode->i_streamid = offset; +spin_unlock (&inode->i_lock); + } +break ;default : ret = -EINVAL ; }Signature · 6 lines
1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers, Andreas
Re: [PATCH 2/7] Add support for per-file stream ID
Quoted 55 lines · Andreas Dilger
Quoted 54 lines · 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.Signed-off-by Jens Axboe <[email protected]>---
fs/inode.c | 1 + fs/open.c | 1 + include/linux/fs.h | 23 +++++++++++++++++++++++ include/uapi/linux/fadvise.h | 2 ++ mm/fadvise.c | 17 +++++++++++++++++ 5 files changed, 44 insertions(+)fs/inode.c +1
-0@@ -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 ;fs/open.c +1
-0@@ -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 ;include/linux/fs.h +0
-0@@ -631,6 +631,7 @@ struct inode { };
Quoted 8 lines
include/linux/fs.h struct inode { · line 634
Quoted 2 lines
include/linux/fs.h +1
-0__u32 i_generation; +unsigned int i_streamid;Since there are only 8 bits of streamid being passed from userspace, is it possible to declare this as a char and pack it into a hole so that it doesn't increase the inode size for a functionality that most people won't be using? Maybe after
i_bytes? That could be increased to unsigned short if needed without increasing the size of the inode.
In the next version, I've retained the int, but ensured that they pack better in both struct file and struct inode. Basically fill some existing holes.
Signature · 1 line
Jens Axboe
PATCH 3/7 direct-io: add support for write stream IDs
In reply to Jens AxboeGet the streamid from the file, if any, and set it on the bio.
--- fs/direct-io.c | 4 ++++ 1 file changed, 4 insertions(+)
fs/direct-io.c +4 -0
@@ -77,6 +77,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 */
@@ -372,6 +373,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); }/*
@@ -1205,6 +1208,7 @@ do_blockdev_direct_IO(int rw, 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;
Signature · 1 line
1.9.1
PATCH 4/7 Add stream ID support for buffered mpage/__block_write_full_page()
In reply to Jens AxboePass on the inode stream ID to the bio allocation.
--- fs/buffer.c | 4 ++-- fs/mpage.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
fs/buffer.c +2 -2
@@ -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;
fs/mpage.c +1 -0
@@ -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)); }/*
Signature · 1 line
1.9.1
RE: [PATCH 4/7] Add stream ID support for buffered mpage/__block_write_full_page()
Quoted 46 lines
-----Original Message-----
From: Jens Axboe [mailto:[email protected]]
Sent: Wednesday, March 25, 2015 8:08 AM
To: [email protected]; [email protected]
Cc: Ming Lin-SSI; [email protected]; Jens Axboe
Subject: [PATCH 4/7] Add stream ID support for buffered mpage/__block_write_full_page()Pass on the inode stream ID to the bio allocation.
Signed-off-by Jens Axboe <[email protected]>--- fs/buffer.c | 4 ++-- fs/mpage.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)fs/buffer.c +1
-1@@ -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++;fs/buffer.c +1
-1} 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++; }fs/buffer.c +0
-0bh = next;fs/mpage.c +0
-0
fs/mpage.c alloc_new: · line 609
Quoted 5 lines
fs/mpage.c +1
-0@@ -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));
This will not work when multiple processes write to the same raw disk. Let's say 2 process concurrently pwrite to /dev/nvme0n1 with different stream_id.
Process 1:
fd = open("/dev/nvme0n1", ...); posix_fadvise(fd, stream_id_1, 0, POSIX_FADV_STREAMID); pwrite( fd, buf1, count1, offset1);
Process 2:
fd = open("/dev/nvme0n1", ...); posix_fadvise(fd, stream_id_2, 0, POSIX_FADV_STREAMID); pwrite(fd, buf2, count2, offset2);
One stream_id will overwrite the other one because "inode" is same.
Thanks,
Ming
Re: [PATCH 4/7] Add stream ID support for buffered mpage/__block_write_full_page()
Quoted 60 lines · Ming Lin-SSI
Quoted 46 lines
-----Original Message-----
From: Jens Axboe [mailto:[email protected]]
Sent: Wednesday, March 25, 2015 8:08 AM
To: [email protected]; [email protected]
Cc: Ming Lin-SSI; [email protected]; Jens Axboe
Subject: [PATCH 4/7] Add stream ID support for buffered mpage/__block_write_full_page()Pass on the inode stream ID to the bio allocation.
Signed-off-by Jens Axboe <[email protected]>--- fs/buffer.c | 4 ++-- fs/mpage.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)fs/buffer.c +1
-1@@ -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;fs/buffer.c +1
-1@@ -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;fs/mpage.c +0
-0fs/mpage.c alloc_new: · line 609
Quoted 5 lines
fs/mpage.c +1
-0@@ -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));This will not work when multiple processes write to the same raw disk. Let's say 2 process concurrently pwrite to
/dev/nvme0n1with differentstream_id.Process 1:
fd =open("/dev/nvme0n1", ...);posix_fadvise(fd, stream_id_1, 0, POSIX_FADV_STREAMID);pwrite( fd, buf1, count1, offset1);
Quoted 6 lines
Process 2:
fd =open("/dev/nvme0n1", ...);posix_fadvise(fd, stream_id_2, 0, POSIX_FADV_STREAMID);pwrite(fd, buf2, count2, offset2);One
stream_idwill overwrite the other one because "inode" is same.
Well, that's how buffered writeback works... There's no file available at that point in time, in fact it could be long gone. So the only reliable part we have here is the inode.
If you want the above scenario to work, you have to use O_DIRECT. Then it will work.
Signature · 1 line
Jens Axboe
PATCH 5/7 btrfs: add support for write stream IDs
In reply to Jens AxboeBoth buffered and O_DIRECT.
--- fs/btrfs/extent_io.c | 1 + fs/btrfs/inode.c | 1 + 2 files changed, 2 insertions(+)
fs/btrfs/extent_io.c +1 -0
@@ -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;
fs/btrfs/inode.c +1 -0
@@ -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;
Signature · 1 line
1.9.1
Re: [PATCH 5/7] btrfs: add support for write stream IDs
Quoted 1 line · Jens Axboe
Both buffered and
O_DIRECT.
Looks good, thanks Jens.
PATCH 6/7 xfs: add support for buffered writeback stream ID
In reply to Jens Axboe--- fs/xfs/xfs_aops.c | 1 + 1 file changed, 1 insertion(+)
fs/xfs/xfs_aops.c +1 -0
@@ -377,6 +377,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); }
Signature · 1 line
1.9.1
PATCH 7/7 ext4: add support for write stream IDs
In reply to Jens Axboe--- fs/ext4/page-io.c | 1 + 1 file changed, 1 insertion(+)
fs/ext4/page-io.c +1 -0
@@ -390,6 +390,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)
Signature · 1 line
1.9.1
RE: [PATCH 7/7] ext4: add support for write stream IDs
Quoted 17 lines
-----Original Message-----
From: Jens Axboe [mailto:[email protected]]
Sent: Wednesday, March 25, 2015 8:08 AM
To: [email protected]; [email protected]
Cc: Ming Lin-SSI; [email protected]; Jens Axboe
Subject: [PATCH 7/7] ext4: add support for write stream IDsSigned-off-by Jens Axboe <[email protected]>--- fs/ext4/page-io.c | 1 + 1 file changed, 1 insertion(+)fs/ext4/page-io.c index renamed from fs/ext4/page-io.c +0
-0
b24a2541a9ba..f642d0b87a32100644
fs/ext4/page-io.c submit_and_retry: · line 393
Quoted 4 lines
fs/ext4/page-io.c +1
-0@@ -390,6 +390,7 @@ submit_and_retry: ret =io_submit_init_bio (io, bh);if (ret)return ret; +bio_set_streamid (io->io_bio,inode_streamid (inode));
For "writeback" and "ordered" journal mode, it should be OK.
But for "journal" journal mode,
Step1: data written to journal
Step2: data written from journal to fs
Do we need other code to carry stream_id in step2?
Thanks,
Ming
Quoted 6 lines
fs/ext4/page-io.c +0
-0} ret =bio_add_page (io->io_bio, bh->b_page, bh->b_size,bh_offset (bh));if (ret !=
bh->b_size)
--
1.9.1
Re: [PATCH 7/7] ext4: add support for write stream IDs
Quoted 24 lines · Ming Lin-SSI
Quoted 17 lines
-----Original Message-----
From: Jens Axboe [mailto:[email protected]]
Sent: Wednesday, March 25, 2015 8:08 AM
To: [email protected]; [email protected]
Cc: Ming Lin-SSI; [email protected]; Jens Axboe
Subject: [PATCH 7/7] ext4: add support for write stream IDsSigned-off-by Jens Axboe <[email protected]>--- fs/ext4/page-io.c | 1 + 1 file changed, 1 insertion(+)fs/ext4/page-io.c index renamed from fs/ext4/page-io.c +0
-0
b24a2541a9ba..f642d0b87a32100644fs/ext4/page-io.c submit_and_retry: · line 393
Quoted 4 lines
fs/ext4/page-io.c +1
-0@@ -390,6 +390,7 @@ submit_and_retry: ret =io_submit_init_bio (io, bh);if (ret)return ret; +bio_set_streamid (io->io_bio,inode_streamid (inode));For "writeback" and "ordered" journal mode, it should be OK.
Quoted 5 lines
But for "journal" journal mode,
Step1: data written to journal
Step2: data written from journal to fsDo we need other code to carry
stream_idin step2?
For either mode, arguably the journal should be written with its own stream ID. But that's a subject that the filesystem guys need to tackle, once the support is one.
For the case of maintaining stream ID when in data=journal mode and copying the data out, I doubt that a use case that anyone would care about. If they do, the ext4/3 guys would need to sort that out.
I'll update the commit message to reflect that, it is a bit under explained at the moment.
Signature · 1 line
Jens Axboe
Quoted 9 lines · 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'.
And presumably the device also has no knowledge of what stream ID is what, right?
Quoted 11 lines
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_STREAMIDhint. 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_DIRECTsupport, 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.
This proposal leaves lot to the reviewer's imagination. Is there any research in this area you can point to?
At a high level, are you sure you've got the right interface? I would think data lifetime would be tied to the file. If that's the case, it might be possible to not export this to userspace at all, and simply make it work under the covers. After all, what prevents multiple applications from using the same stream id at the same time?
Cheers,
Jeff
Quoted 8 lines · Jeff Moyer
Quoted 6 lines · 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.
Quoted 6 lines
Quoted 3 lines
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'.
And presumably the device also has no knowledge of what stream ID is what, right?
Right, the point is that the device need not now. As long as it knows that lifetime of objects in stream ID X is similar, that's enough.
Quoted 6 lines
Quoted 6 lines
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_STREAMIDhint. The 'offset' field is used to pass the relevant stream ID. Switching to fcntl (with a SET/GET_STREAMID) would be trivial.
Quoted 8 lines
Quoted 5 lines
The patchset wires up the block parts, adds buffered and
O_DIRECTsupport, 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.
This proposal leaves lot to the reviewer's imagination. Is there any research in this area you can point to?
Samsung had a paper for HotStorage 14 here:
https://www.usenix.org/system/files/conference/hotstorage14/hotstorage14-paper-kang.pdf
Additionally, we're internally at FB know doing our own analysis of how this will impact write amplification for certain workloads. Hopefully I should have some info there in a few weeks.
Quoted 1 line
At a high level, are you sure you've got the right interface? I would
Not at all :-)
Quoted 4 lines
think data lifetime would be tied to the file. If that's the case, it might be possible to not export this to userspace at all, and simply make it work under the covers. After all, what prevents multiple applications from using the same stream id at the same time?
Yes, it'll be tied to a file, that's also what the interface works on. But you need something to tell the kernel what stream a given file belongs to. And of course multiple files can belong to the same stream ID.
More than one application is definitely more tricky, because you'd have to coordinate handing out stream IDs. Given that I believe we'll have a fairly limited number of streams available, some applications might share stream IDs with others. And that's perfectly fine, assuming that the objects stored under the same stream ID does have similar lifetimes. Basically it's punting that configuration to the admin.
The current interface also doesn't have any knowledge of what streams a device supports. It's done that way on purpose. The stream ID is a hint. It'll never be worse off than writing everything under the same stream. And I don't want to make this topology aware so that dm etc will have to stack and handle these limits. I just want a simple hint that we pass down.
Signature · 1 line
Jens Axboe