| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 09 Apr 2015 12:30:40 +0300
From: Dmitry Monakhov <dmonlist@...il.com>
To: Jens Axboe <axboe@...com>, linux-kernel@...r.kernel.org,
linux-fsdevel@...r.kernel.org
Cc: ming.l@....samsung.com, david@...morbit.com,
Jens Axboe <axboe@...com>
Subject: Re: [PATCH 2/7] Add support for per-file stream ID
Jens Axboe <axboe@...com> 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.
> 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 <axboe@...com>
> ---
> 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(+)
>
> 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 33f9cbf2610b..4a9b2be1a674 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 b4d71b5e1ff2..43dde70c1d0d 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -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);
> diff --git a/include/uapi/linux/fadvise.h b/include/uapi/linux/fadvise.h
> index e8e747139b9a..3dc8a1ff1422 100644
> --- a/include/uapi/linux/fadvise.h
> +++ b/include/uapi/linux/fadvise.h
> @@ -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 */
> diff --git a/mm/fadvise.c b/mm/fadvise.c
> index 4a3907cf79f8..b111a8899fb7 100644
> --- a/mm/fadvise.c
> +++ b/mm/fadvise.c
> @@ -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;
Shuld be negative 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;
> }
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Download attachment "signature.asc" of type "application/pgp-signature" (473 bytes)
Powered by blists - more mailing lists