Re: [git pull] vfs.git part 3
On Fri, Jul 07, 2017 at 04:00:02PM +0200, Christoph Hellwig wrote: > On Fri, Jul 07, 2017 at 12:27:49AM +0100, Al Viro wrote: > > On Thu, Jul 06, 2017 at 11:44:49PM +0200, Christoph Hellwig wrote: > > > > Which sparse version are you using and what's your .config? > > > > > > sparse is v0.5.0-62-gce18a90, .config is attached. > > > > Arrgh... OK, I see what's going on. sparse commit affecting that > > is "Allow casting to a restricted type if !restricted_value"; it > > allows the things like (__le32)0. It's present in sparse.git, > > but not in chrisl/sparse.git, which is what you are using. > > So what's the current story on sparse versions to use and releases? > > At some point it seemed like upstream sparse was sort of dead > and the chrisl repo was the one to use. It seems the main sparse > repo on git.kernel.org is now identical to the chrisl one, and he > is doing the releases. So maybe I'll just need to upgrade.. OK, here's what I have on top of #for-linus. Handling of __bitwise in {COMPAT_,}SYSCALL_DEFINE went into the tip of vfs.git#for-linus, works both for old and for new sparse versions. I'd added annotations of sys_pwritev2(), etc., in syscalls.h and compat.h, other than that it's what you'd posted... Care to put your S-o-b on that? commit 1c0ecce04fc5747dbd0578360895fe910d3124db Author: Christoph HellwigDate: Thu Jul 6 18:58:37 2017 +0200 annotate RWF_... flags [AV: added missing annotations in syscalls.h/compat.h] Signed-off-by: Al Viro diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 38d0383dc7f9..bc69d40c4e8b 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -969,7 +969,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file, int use_wgather; loff_t pos = offset; unsigned intpflags = current->flags; - int flags = 0; + rwf_t flags = 0; if (test_bit(RQ_LOCAL, >rq_flags)) /* diff --git a/fs/read_write.c b/fs/read_write.c index a2cbc8303dae..327d4aeefca0 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -33,7 +33,7 @@ const struct file_operations generic_ro_fops = { EXPORT_SYMBOL(generic_ro_fops); -static inline int unsigned_offsets(struct file *file) +static inline bool unsigned_offsets(struct file *file) { return file->f_mode & FMODE_UNSIGNED_OFFSET; } @@ -633,7 +633,7 @@ unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to) EXPORT_SYMBOL(iov_shorten); static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { struct kiocb kiocb; ssize_t ret; @@ -655,7 +655,7 @@ static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, /* Do it by hand, with file-ops */ static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { ssize_t ret = 0; @@ -871,7 +871,7 @@ ssize_t compat_rw_copy_check_uvector(int type, #endif static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -899,7 +899,7 @@ static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->read_iter) return -EINVAL; @@ -908,7 +908,7 @@ ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_read); static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -937,7 +937,7 @@ static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->write_iter) return -EINVAL; @@ -946,7 +946,7 @@ ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_write); ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, - unsigned long vlen, loff_t *pos, int flags) + unsigned long vlen, loff_t *pos, rwf_t flags) { struct iovec iovstack[UIO_FASTIOV]; struct iovec *iov = iovstack; @@ -964,7 +964,7 @@ ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, EXPORT_SYMBOL(vfs_readv); ssize_t vfs_writev(struct file *file,
Re: [git pull] vfs.git part 3
On Fri, Jul 07, 2017 at 04:00:02PM +0200, Christoph Hellwig wrote: > On Fri, Jul 07, 2017 at 12:27:49AM +0100, Al Viro wrote: > > On Thu, Jul 06, 2017 at 11:44:49PM +0200, Christoph Hellwig wrote: > > > > Which sparse version are you using and what's your .config? > > > > > > sparse is v0.5.0-62-gce18a90, .config is attached. > > > > Arrgh... OK, I see what's going on. sparse commit affecting that > > is "Allow casting to a restricted type if !restricted_value"; it > > allows the things like (__le32)0. It's present in sparse.git, > > but not in chrisl/sparse.git, which is what you are using. > > So what's the current story on sparse versions to use and releases? > > At some point it seemed like upstream sparse was sort of dead > and the chrisl repo was the one to use. It seems the main sparse > repo on git.kernel.org is now identical to the chrisl one, and he > is doing the releases. So maybe I'll just need to upgrade.. OK, here's what I have on top of #for-linus. Handling of __bitwise in {COMPAT_,}SYSCALL_DEFINE went into the tip of vfs.git#for-linus, works both for old and for new sparse versions. I'd added annotations of sys_pwritev2(), etc., in syscalls.h and compat.h, other than that it's what you'd posted... Care to put your S-o-b on that? commit 1c0ecce04fc5747dbd0578360895fe910d3124db Author: Christoph Hellwig Date: Thu Jul 6 18:58:37 2017 +0200 annotate RWF_... flags [AV: added missing annotations in syscalls.h/compat.h] Signed-off-by: Al Viro diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 38d0383dc7f9..bc69d40c4e8b 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -969,7 +969,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file, int use_wgather; loff_t pos = offset; unsigned intpflags = current->flags; - int flags = 0; + rwf_t flags = 0; if (test_bit(RQ_LOCAL, >rq_flags)) /* diff --git a/fs/read_write.c b/fs/read_write.c index a2cbc8303dae..327d4aeefca0 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -33,7 +33,7 @@ const struct file_operations generic_ro_fops = { EXPORT_SYMBOL(generic_ro_fops); -static inline int unsigned_offsets(struct file *file) +static inline bool unsigned_offsets(struct file *file) { return file->f_mode & FMODE_UNSIGNED_OFFSET; } @@ -633,7 +633,7 @@ unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to) EXPORT_SYMBOL(iov_shorten); static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { struct kiocb kiocb; ssize_t ret; @@ -655,7 +655,7 @@ static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, /* Do it by hand, with file-ops */ static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { ssize_t ret = 0; @@ -871,7 +871,7 @@ ssize_t compat_rw_copy_check_uvector(int type, #endif static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -899,7 +899,7 @@ static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->read_iter) return -EINVAL; @@ -908,7 +908,7 @@ ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_read); static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -937,7 +937,7 @@ static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->write_iter) return -EINVAL; @@ -946,7 +946,7 @@ ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_write); ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, - unsigned long vlen, loff_t *pos, int flags) + unsigned long vlen, loff_t *pos, rwf_t flags) { struct iovec iovstack[UIO_FASTIOV]; struct iovec *iov = iovstack; @@ -964,7 +964,7 @@ ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, EXPORT_SYMBOL(vfs_readv); ssize_t vfs_writev(struct file *file, const struct iovec __user *vec, -
Re: [git pull] vfs.git part 3
On Fri, Jul 7, 2017 at 8:48 AM, Linus Torvaldswrote: > The releases are done way too seldom to be useful, but that may be > improving. There is one fairly imminent, and it's probably a good idea > to just test the current git tree. Yes guilty of too few releases. We are cutting one release pretty soon. The currently master branch of sparse git repository has gone to RC4 of of release v0.5.1. BTW, the current sparse development has move back to the official sparse repository. The sparse-next is the branch to test the bleeding edge bits. In theory sparse-next can roll back and rewrite history, there is no guarantee it will be clean pull. The master branch will never rebase, it will prove a clean pull. Chris
Re: [git pull] vfs.git part 3
On Fri, Jul 7, 2017 at 8:48 AM, Linus Torvalds wrote: > The releases are done way too seldom to be useful, but that may be > improving. There is one fairly imminent, and it's probably a good idea > to just test the current git tree. Yes guilty of too few releases. We are cutting one release pretty soon. The currently master branch of sparse git repository has gone to RC4 of of release v0.5.1. BTW, the current sparse development has move back to the official sparse repository. The sparse-next is the branch to test the bleeding edge bits. In theory sparse-next can roll back and rewrite history, there is no guarantee it will be clean pull. The master branch will never rebase, it will prove a clean pull. Chris
Re: [git pull] vfs.git part 3
On Fri, Jul 7, 2017 at 7:00 AM, Christoph Hellwigwrote: > > So what's the current story on sparse versions to use and releases? The releases are done way too seldom to be useful, but that may be improving. There is one fairly imminent, and it's probably a good idea to just test the current git tree. Linus
Re: [git pull] vfs.git part 3
On Fri, Jul 7, 2017 at 7:00 AM, Christoph Hellwig wrote: > > So what's the current story on sparse versions to use and releases? The releases are done way too seldom to be useful, but that may be improving. There is one fairly imminent, and it's probably a good idea to just test the current git tree. Linus
Re: [git pull] vfs.git part 3
On Fri, Jul 07, 2017 at 12:27:49AM +0100, Al Viro wrote: > On Thu, Jul 06, 2017 at 11:44:49PM +0200, Christoph Hellwig wrote: > > > Which sparse version are you using and what's your .config? > > > > sparse is v0.5.0-62-gce18a90, .config is attached. > > Arrgh... OK, I see what's going on. sparse commit affecting that > is "Allow casting to a restricted type if !restricted_value"; it > allows the things like (__le32)0. It's present in sparse.git, > but not in chrisl/sparse.git, which is what you are using. So what's the current story on sparse versions to use and releases? At some point it seemed like upstream sparse was sort of dead and the chrisl repo was the one to use. It seems the main sparse repo on git.kernel.org is now identical to the chrisl one, and he is doing the releases. So maybe I'll just need to upgrade..
Re: [git pull] vfs.git part 3
On Fri, Jul 07, 2017 at 12:27:49AM +0100, Al Viro wrote: > On Thu, Jul 06, 2017 at 11:44:49PM +0200, Christoph Hellwig wrote: > > > Which sparse version are you using and what's your .config? > > > > sparse is v0.5.0-62-gce18a90, .config is attached. > > Arrgh... OK, I see what's going on. sparse commit affecting that > is "Allow casting to a restricted type if !restricted_value"; it > allows the things like (__le32)0. It's present in sparse.git, > but not in chrisl/sparse.git, which is what you are using. So what's the current story on sparse versions to use and releases? At some point it seemed like upstream sparse was sort of dead and the chrisl repo was the one to use. It seems the main sparse repo on git.kernel.org is now identical to the chrisl one, and he is doing the releases. So maybe I'll just need to upgrade..
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 11:44:49PM +0200, Christoph Hellwig wrote: > > Which sparse version are you using and what's your .config? > > sparse is v0.5.0-62-gce18a90, .config is attached. Arrgh... OK, I see what's going on. sparse commit affecting that is "Allow casting to a restricted type if !restricted_value"; it allows the things like (__le32)0. It's present in sparse.git, but not in chrisl/sparse.git, which is what you are using. Anyway, the thing I'd missed kernel-side is this: #define __TYPE_IS_L(t) (__same_type((t)0, 0L)) #define __TYPE_IS_UL(t) (__same_type((t)0, 0UL)) #define __TYPE_IS_LL(t) (__same_type((t)0, 0LL) || __same_type((t)0, 0ULL)) Let's turn them into #define __TYPE_AS(t, v) __same_type((__force t)0, v) #define __TYPE_IS_L(t) (__TYPE_AS(t, 0L)) #define __TYPE_IS_UL(t) (__TYPE_AS(t, 0UL)) #define __TYPE_IS_LL(t) (__TYPE_AS(t, 0LL) || __TYPE_AS(t, 0ULL)) That should do it both for old and for new versions of sparse.
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 11:44:49PM +0200, Christoph Hellwig wrote: > > Which sparse version are you using and what's your .config? > > sparse is v0.5.0-62-gce18a90, .config is attached. Arrgh... OK, I see what's going on. sparse commit affecting that is "Allow casting to a restricted type if !restricted_value"; it allows the things like (__le32)0. It's present in sparse.git, but not in chrisl/sparse.git, which is what you are using. Anyway, the thing I'd missed kernel-side is this: #define __TYPE_IS_L(t) (__same_type((t)0, 0L)) #define __TYPE_IS_UL(t) (__same_type((t)0, 0UL)) #define __TYPE_IS_LL(t) (__same_type((t)0, 0LL) || __same_type((t)0, 0ULL)) Let's turn them into #define __TYPE_AS(t, v) __same_type((__force t)0, v) #define __TYPE_IS_L(t) (__TYPE_AS(t, 0L)) #define __TYPE_IS_UL(t) (__TYPE_AS(t, 0UL)) #define __TYPE_IS_LL(t) (__TYPE_AS(t, 0LL) || __TYPE_AS(t, 0ULL)) That should do it both for old and for new versions of sparse.
Re: [git pull] vfs.git part 3
> Which sparse version are you using and what's your .config? sparse is v0.5.0-62-gce18a90, .config is attached. .config Description: application/config
Re: [git pull] vfs.git part 3
> Which sparse version are you using and what's your .config? sparse is v0.5.0-62-gce18a90, .config is attached. .config Description: application/config
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 06:58:37PM +0200, Christoph Hellwig wrote: > On Thu, Jul 06, 2017 at 04:51:13PM +0100, Al Viro wrote: > > On Thu, Jul 06, 2017 at 04:46:02PM +0100, Al Viro wrote: > > > > > That - on #work.read_write, as in vfs.git at the moment... > > > > ... and for COMPAT_SYSCALL you need > > #define __SC_DELOUSE(t,v) ((__force t)(unsigned long)(v)) > > in linux/compat.h > > I'm still getting warnings with both these force casts. This is > the current stack: That + Linus' tree as of the end of yesterday => CHECK fs/read_write.c fs/read_write.c:38:29: warning: incorrect type in return expression (different base types) fs/read_write.c:38:29:expected int fs/read_write.c:38:29:got restricted fmode_t fs/read_write.c:38:29: warning: incorrect type in return expression (different base types) fs/read_write.c:38:29:expected int fs/read_write.c:38:29:got restricted fmode_t fs/read_write.c:38:29: warning: incorrect type in return expression (different base types) fs/read_write.c:38:29:expected int fs/read_write.c:38:29:got restricted fmode_t fs/read_write.c:38:29: warning: incorrect type in return expression (different base types) fs/read_write.c:38:29:expected int fs/read_write.c:38:29:got restricted fmode_t All of which are from unsigned_offsets() and that's one case where bool would be better than int. Switching the return type to bool yields CHECK fs/read_write.c CC fs/read_write.o - no warnings at all. Which sparse version are you using and what's your .config?
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 06:58:37PM +0200, Christoph Hellwig wrote: > On Thu, Jul 06, 2017 at 04:51:13PM +0100, Al Viro wrote: > > On Thu, Jul 06, 2017 at 04:46:02PM +0100, Al Viro wrote: > > > > > That - on #work.read_write, as in vfs.git at the moment... > > > > ... and for COMPAT_SYSCALL you need > > #define __SC_DELOUSE(t,v) ((__force t)(unsigned long)(v)) > > in linux/compat.h > > I'm still getting warnings with both these force casts. This is > the current stack: That + Linus' tree as of the end of yesterday => CHECK fs/read_write.c fs/read_write.c:38:29: warning: incorrect type in return expression (different base types) fs/read_write.c:38:29:expected int fs/read_write.c:38:29:got restricted fmode_t fs/read_write.c:38:29: warning: incorrect type in return expression (different base types) fs/read_write.c:38:29:expected int fs/read_write.c:38:29:got restricted fmode_t fs/read_write.c:38:29: warning: incorrect type in return expression (different base types) fs/read_write.c:38:29:expected int fs/read_write.c:38:29:got restricted fmode_t fs/read_write.c:38:29: warning: incorrect type in return expression (different base types) fs/read_write.c:38:29:expected int fs/read_write.c:38:29:got restricted fmode_t All of which are from unsigned_offsets() and that's one case where bool would be better than int. Switching the return type to bool yields CHECK fs/read_write.c CC fs/read_write.o - no warnings at all. Which sparse version are you using and what's your .config?
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 04:51:13PM +0100, Al Viro wrote: > On Thu, Jul 06, 2017 at 04:46:02PM +0100, Al Viro wrote: > > > That - on #work.read_write, as in vfs.git at the moment... > > ... and for COMPAT_SYSCALL you need > #define __SC_DELOUSE(t,v) ((__force t)(unsigned long)(v)) > in linux/compat.h I'm still getting warnings with both these force casts. This is the current stack: diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h index 0ddd37e6c29d..019f0de892c3 100644 --- a/arch/s390/include/asm/compat.h +++ b/arch/s390/include/asm/compat.h @@ -12,7 +12,7 @@ #define __SC_DELOUSE(t,v) ({ \ BUILD_BUG_ON(sizeof(t) > 4 && !__TYPE_IS_PTR(t)); \ - (t)(__TYPE_IS_PTR(t) ? ((v) & 0x7fff) : (v)); \ + (__force t)(__TYPE_IS_PTR(t) ? ((v) & 0x7fff) : (v)); \ }) #define PSW32_MASK_PER 0x4000UL diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 38d0383dc7f9..bc69d40c4e8b 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -969,7 +969,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file, int use_wgather; loff_t pos = offset; unsigned intpflags = current->flags; - int flags = 0; + rwf_t flags = 0; if (test_bit(RQ_LOCAL, >rq_flags)) /* diff --git a/fs/read_write.c b/fs/read_write.c index a2cbc8303dae..bc2db5e5cd19 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -633,7 +633,7 @@ unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to) EXPORT_SYMBOL(iov_shorten); static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { struct kiocb kiocb; ssize_t ret; @@ -655,7 +655,7 @@ static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, /* Do it by hand, with file-ops */ static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { ssize_t ret = 0; @@ -871,7 +871,7 @@ ssize_t compat_rw_copy_check_uvector(int type, #endif static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -899,7 +899,7 @@ static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->read_iter) return -EINVAL; @@ -908,7 +908,7 @@ ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_read); static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -937,7 +937,7 @@ static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->write_iter) return -EINVAL; @@ -946,7 +946,7 @@ ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_write); ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, - unsigned long vlen, loff_t *pos, int flags) + unsigned long vlen, loff_t *pos, rwf_t flags) { struct iovec iovstack[UIO_FASTIOV]; struct iovec *iov = iovstack; @@ -964,7 +964,7 @@ ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, EXPORT_SYMBOL(vfs_readv); ssize_t vfs_writev(struct file *file, const struct iovec __user *vec, - unsigned long vlen, loff_t *pos, int flags) + unsigned long vlen, loff_t *pos, rwf_t flags) { struct iovec iovstack[UIO_FASTIOV]; struct iovec *iov = iovstack; @@ -981,7 +981,7 @@ ssize_t vfs_writev(struct file *file, const struct iovec __user *vec, EXPORT_SYMBOL(vfs_writev); static ssize_t do_readv(unsigned long fd, const struct iovec __user *vec, - unsigned long vlen, int flags) + unsigned long vlen, rwf_t flags) { struct fd f = fdget_pos(fd); ssize_t ret = -EBADF; @@ -1001,7 +1001,7 @@ static ssize_t do_readv(unsigned long fd, const struct iovec __user *vec, } static ssize_t do_writev(unsigned long fd, const struct iovec __user *vec, -unsigned long vlen, int flags) +unsigned long vlen, rwf_t flags) { struct fd f = fdget_pos(fd);
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 04:51:13PM +0100, Al Viro wrote: > On Thu, Jul 06, 2017 at 04:46:02PM +0100, Al Viro wrote: > > > That - on #work.read_write, as in vfs.git at the moment... > > ... and for COMPAT_SYSCALL you need > #define __SC_DELOUSE(t,v) ((__force t)(unsigned long)(v)) > in linux/compat.h I'm still getting warnings with both these force casts. This is the current stack: diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h index 0ddd37e6c29d..019f0de892c3 100644 --- a/arch/s390/include/asm/compat.h +++ b/arch/s390/include/asm/compat.h @@ -12,7 +12,7 @@ #define __SC_DELOUSE(t,v) ({ \ BUILD_BUG_ON(sizeof(t) > 4 && !__TYPE_IS_PTR(t)); \ - (t)(__TYPE_IS_PTR(t) ? ((v) & 0x7fff) : (v)); \ + (__force t)(__TYPE_IS_PTR(t) ? ((v) & 0x7fff) : (v)); \ }) #define PSW32_MASK_PER 0x4000UL diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 38d0383dc7f9..bc69d40c4e8b 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -969,7 +969,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file, int use_wgather; loff_t pos = offset; unsigned intpflags = current->flags; - int flags = 0; + rwf_t flags = 0; if (test_bit(RQ_LOCAL, >rq_flags)) /* diff --git a/fs/read_write.c b/fs/read_write.c index a2cbc8303dae..bc2db5e5cd19 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -633,7 +633,7 @@ unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to) EXPORT_SYMBOL(iov_shorten); static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { struct kiocb kiocb; ssize_t ret; @@ -655,7 +655,7 @@ static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, /* Do it by hand, with file-ops */ static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { ssize_t ret = 0; @@ -871,7 +871,7 @@ ssize_t compat_rw_copy_check_uvector(int type, #endif static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -899,7 +899,7 @@ static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->read_iter) return -EINVAL; @@ -908,7 +908,7 @@ ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_read); static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -937,7 +937,7 @@ static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->write_iter) return -EINVAL; @@ -946,7 +946,7 @@ ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_write); ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, - unsigned long vlen, loff_t *pos, int flags) + unsigned long vlen, loff_t *pos, rwf_t flags) { struct iovec iovstack[UIO_FASTIOV]; struct iovec *iov = iovstack; @@ -964,7 +964,7 @@ ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, EXPORT_SYMBOL(vfs_readv); ssize_t vfs_writev(struct file *file, const struct iovec __user *vec, - unsigned long vlen, loff_t *pos, int flags) + unsigned long vlen, loff_t *pos, rwf_t flags) { struct iovec iovstack[UIO_FASTIOV]; struct iovec *iov = iovstack; @@ -981,7 +981,7 @@ ssize_t vfs_writev(struct file *file, const struct iovec __user *vec, EXPORT_SYMBOL(vfs_writev); static ssize_t do_readv(unsigned long fd, const struct iovec __user *vec, - unsigned long vlen, int flags) + unsigned long vlen, rwf_t flags) { struct fd f = fdget_pos(fd); ssize_t ret = -EBADF; @@ -1001,7 +1001,7 @@ static ssize_t do_readv(unsigned long fd, const struct iovec __user *vec, } static ssize_t do_writev(unsigned long fd, const struct iovec __user *vec, -unsigned long vlen, int flags) +unsigned long vlen, rwf_t flags) { struct fd f = fdget_pos(fd);
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 04:46:02PM +0100, Al Viro wrote: > That - on #work.read_write, as in vfs.git at the moment... ... and for COMPAT_SYSCALL you need #define __SC_DELOUSE(t,v) ((__force t)(unsigned long)(v)) in linux/compat.h
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 04:46:02PM +0100, Al Viro wrote: > That - on #work.read_write, as in vfs.git at the moment... ... and for COMPAT_SYSCALL you need #define __SC_DELOUSE(t,v) ((__force t)(unsigned long)(v)) in linux/compat.h
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 05:10:33PM +0200, Christoph Hellwig wrote: > On Thu, Jul 06, 2017 at 04:03:30PM +0100, Al Viro wrote: > > On Thu, Jul 06, 2017 at 04:48:40PM +0200, Christoph Hellwig wrote: > > > > > Just did the whole batch (patch below), but it seems like using a > > > __bitwise type in SYSCALL_DEFINE* will always give warnings like: > > > > > > fs/read_write.c:1095:1: warning: cast to restricted __kernel_rwf_t > > > > > > which I'm not sure to deal with.. > > > > #define __SC_CAST(t, a) (__force t) a > > doesn't seem to make a difference.. Works here... ; cat a.c #include #undef __SC_CAST #define __SC_CAST(t, a) (__force t)a typedef int __bitwise __foo_t; static __foo_t is_OK; static int will_warn; SYSCALL_DEFINE1(foo, __foo_t, arg) { is_OK = arg; will_warn = arg; return 0; } ; make a.o C=2 CHECK=~/local/sparse/sparse CHK include/config/kernel.release CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h CHECK arch/x86/purgatory/purgatory.c CHECK arch/x86/purgatory/sha256.c CHECK arch/x86/purgatory/string.c arch/x86/purgatory/../boot/string.c:165:6: warning: symbol 'strchr' was not declared. Should it be static? CHK include/generated/bounds.h CHK include/generated/timeconst.h CHK include/generated/asm-offsets.h CALLscripts/checksyscalls.sh DESCEND objtool CHECK scripts/mod/empty.c CHK scripts/mod/devicetable-offsets.h CHK include/generated/timeconst.h CHK include/generated/bounds.h CHECK a.c a.c:10:19: warning: incorrect type in assignment (different base types) a.c:10:19:expected int static [signed] [toplevel] will_warn a.c:10:19:got restricted __foo_t [usertype] arg CC a.o ; That - on #work.read_write, as in vfs.git at the moment...
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 05:10:33PM +0200, Christoph Hellwig wrote: > On Thu, Jul 06, 2017 at 04:03:30PM +0100, Al Viro wrote: > > On Thu, Jul 06, 2017 at 04:48:40PM +0200, Christoph Hellwig wrote: > > > > > Just did the whole batch (patch below), but it seems like using a > > > __bitwise type in SYSCALL_DEFINE* will always give warnings like: > > > > > > fs/read_write.c:1095:1: warning: cast to restricted __kernel_rwf_t > > > > > > which I'm not sure to deal with.. > > > > #define __SC_CAST(t, a) (__force t) a > > doesn't seem to make a difference.. Works here... ; cat a.c #include #undef __SC_CAST #define __SC_CAST(t, a) (__force t)a typedef int __bitwise __foo_t; static __foo_t is_OK; static int will_warn; SYSCALL_DEFINE1(foo, __foo_t, arg) { is_OK = arg; will_warn = arg; return 0; } ; make a.o C=2 CHECK=~/local/sparse/sparse CHK include/config/kernel.release CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h CHECK arch/x86/purgatory/purgatory.c CHECK arch/x86/purgatory/sha256.c CHECK arch/x86/purgatory/string.c arch/x86/purgatory/../boot/string.c:165:6: warning: symbol 'strchr' was not declared. Should it be static? CHK include/generated/bounds.h CHK include/generated/timeconst.h CHK include/generated/asm-offsets.h CALLscripts/checksyscalls.sh DESCEND objtool CHECK scripts/mod/empty.c CHK scripts/mod/devicetable-offsets.h CHK include/generated/timeconst.h CHK include/generated/bounds.h CHECK a.c a.c:10:19: warning: incorrect type in assignment (different base types) a.c:10:19:expected int static [signed] [toplevel] will_warn a.c:10:19:got restricted __foo_t [usertype] arg CC a.o ; That - on #work.read_write, as in vfs.git at the moment...
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 04:03:30PM +0100, Al Viro wrote: > On Thu, Jul 06, 2017 at 04:48:40PM +0200, Christoph Hellwig wrote: > > > Just did the whole batch (patch below), but it seems like using a > > __bitwise type in SYSCALL_DEFINE* will always give warnings like: > > > > fs/read_write.c:1095:1: warning: cast to restricted __kernel_rwf_t > > > > which I'm not sure to deal with.. > > #define __SC_CAST(t, a) (__force t) a doesn't seem to make a difference..
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 04:03:30PM +0100, Al Viro wrote: > On Thu, Jul 06, 2017 at 04:48:40PM +0200, Christoph Hellwig wrote: > > > Just did the whole batch (patch below), but it seems like using a > > __bitwise type in SYSCALL_DEFINE* will always give warnings like: > > > > fs/read_write.c:1095:1: warning: cast to restricted __kernel_rwf_t > > > > which I'm not sure to deal with.. > > #define __SC_CAST(t, a) (__force t) a doesn't seem to make a difference..
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 04:03:30PM +0100, Al Viro wrote: > #define __SC_CAST(t, a) (__force t) a > > in syscalls.h Hmm. > > > index a2d4a8ac94ca..a04adbc70ddf 100644 > > --- a/include/uapi/linux/aio_abi.h > > +++ b/include/uapi/linux/aio_abi.h > > @@ -28,6 +28,7 @@ > > #define __LINUX__AIO_ABI_H > > > > #include > > +#include > > Um... Includes of non-uapi in uapi are wrong. What do you need > fs.h for, anyway? Just put the typedef into uapi/linux/types.h > and be done with that... We automatically get the non-uapi one for userspace. And we do in fact need to write it that way as it will not show up as uapi/ in userspace. But yes, the type could be taken to types.h > > > +#defineBLK_STS_OK 0 > > +#define BLK_STS_NOTSUPP((__force blk_status_t)1) > > +#define BLK_STS_TIMEOUT((__force blk_status_t)2) > > +#define BLK_STS_NOSPC ((__force blk_status_t)3) > > +#define BLK_STS_TRANSPORT ((__force blk_status_t)4) > > +#define BLK_STS_TARGET ((__force blk_status_t)5) > > +#define BLK_STS_NEXUS ((__force blk_status_t)6) > > WTF is that doing here? If nothing else, it's a userland namespace > pollution; typedefs with such names are OK in the kernel, but not > is something that might be included from userland. And that chunk > doesn't seem to have anything to do with the rest of the patch... It doesn't that was the copy and paste of the __bitwise boilerplate I staeted with..
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 04:03:30PM +0100, Al Viro wrote: > #define __SC_CAST(t, a) (__force t) a > > in syscalls.h Hmm. > > > index a2d4a8ac94ca..a04adbc70ddf 100644 > > --- a/include/uapi/linux/aio_abi.h > > +++ b/include/uapi/linux/aio_abi.h > > @@ -28,6 +28,7 @@ > > #define __LINUX__AIO_ABI_H > > > > #include > > +#include > > Um... Includes of non-uapi in uapi are wrong. What do you need > fs.h for, anyway? Just put the typedef into uapi/linux/types.h > and be done with that... We automatically get the non-uapi one for userspace. And we do in fact need to write it that way as it will not show up as uapi/ in userspace. But yes, the type could be taken to types.h > > > +#defineBLK_STS_OK 0 > > +#define BLK_STS_NOTSUPP((__force blk_status_t)1) > > +#define BLK_STS_TIMEOUT((__force blk_status_t)2) > > +#define BLK_STS_NOSPC ((__force blk_status_t)3) > > +#define BLK_STS_TRANSPORT ((__force blk_status_t)4) > > +#define BLK_STS_TARGET ((__force blk_status_t)5) > > +#define BLK_STS_NEXUS ((__force blk_status_t)6) > > WTF is that doing here? If nothing else, it's a userland namespace > pollution; typedefs with such names are OK in the kernel, but not > is something that might be included from userland. And that chunk > doesn't seem to have anything to do with the rest of the patch... It doesn't that was the copy and paste of the __bitwise boilerplate I staeted with..
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 04:48:40PM +0200, Christoph Hellwig wrote: > Just did the whole batch (patch below), but it seems like using a > __bitwise type in SYSCALL_DEFINE* will always give warnings like: > > fs/read_write.c:1095:1: warning: cast to restricted __kernel_rwf_t > > which I'm not sure to deal with.. #define __SC_CAST(t, a) (__force t) a in syscalls.h > index a2d4a8ac94ca..a04adbc70ddf 100644 > --- a/include/uapi/linux/aio_abi.h > +++ b/include/uapi/linux/aio_abi.h > @@ -28,6 +28,7 @@ > #define __LINUX__AIO_ABI_H > > #include > +#include Um... Includes of non-uapi in uapi are wrong. What do you need fs.h for, anyway? Just put the typedef into uapi/linux/types.h and be done with that... > +#define BLK_STS_OK 0 > +#define BLK_STS_NOTSUPP ((__force blk_status_t)1) > +#define BLK_STS_TIMEOUT ((__force blk_status_t)2) > +#define BLK_STS_NOSPC((__force blk_status_t)3) > +#define BLK_STS_TRANSPORT((__force blk_status_t)4) > +#define BLK_STS_TARGET ((__force blk_status_t)5) > +#define BLK_STS_NEXUS((__force blk_status_t)6) WTF is that doing here? If nothing else, it's a userland namespace pollution; typedefs with such names are OK in the kernel, but not is something that might be included from userland. And that chunk doesn't seem to have anything to do with the rest of the patch... Please, move that on top of current #work.read_write - there's a fix for buggered vfs_write_iter() in it.
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 04:48:40PM +0200, Christoph Hellwig wrote: > Just did the whole batch (patch below), but it seems like using a > __bitwise type in SYSCALL_DEFINE* will always give warnings like: > > fs/read_write.c:1095:1: warning: cast to restricted __kernel_rwf_t > > which I'm not sure to deal with.. #define __SC_CAST(t, a) (__force t) a in syscalls.h > index a2d4a8ac94ca..a04adbc70ddf 100644 > --- a/include/uapi/linux/aio_abi.h > +++ b/include/uapi/linux/aio_abi.h > @@ -28,6 +28,7 @@ > #define __LINUX__AIO_ABI_H > > #include > +#include Um... Includes of non-uapi in uapi are wrong. What do you need fs.h for, anyway? Just put the typedef into uapi/linux/types.h and be done with that... > +#define BLK_STS_OK 0 > +#define BLK_STS_NOTSUPP ((__force blk_status_t)1) > +#define BLK_STS_TIMEOUT ((__force blk_status_t)2) > +#define BLK_STS_NOSPC((__force blk_status_t)3) > +#define BLK_STS_TRANSPORT((__force blk_status_t)4) > +#define BLK_STS_TARGET ((__force blk_status_t)5) > +#define BLK_STS_NEXUS((__force blk_status_t)6) WTF is that doing here? If nothing else, it's a userland namespace pollution; typedefs with such names are OK in the kernel, but not is something that might be included from userland. And that chunk doesn't seem to have anything to do with the rest of the patch... Please, move that on top of current #work.read_write - there's a fix for buggered vfs_write_iter() in it.
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 12:29:12AM +0100, Al Viro wrote: > On Thu, Jul 06, 2017 at 12:52:35AM +0200, Christoph Hellwig wrote: > > On Wed, Jul 05, 2017 at 11:38:21PM +0100, Al Viro wrote: > > > Sure, makes sense - especially since it's not too widely spread yet. > > > > Do you want to do that yourself, or do you want me to look into it? > > I'll do it tomorrow, unless you get to it first... Just did the whole batch (patch below), but it seems like using a __bitwise type in SYSCALL_DEFINE* will always give warnings like: fs/read_write.c:1095:1: warning: cast to restricted __kernel_rwf_t which I'm not sure to deal with.. -- diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 38d0383dc7f9..bc69d40c4e8b 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -969,7 +969,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file, int use_wgather; loff_t pos = offset; unsigned intpflags = current->flags; - int flags = 0; + rwf_t flags = 0; if (test_bit(RQ_LOCAL, >rq_flags)) /* diff --git a/fs/read_write.c b/fs/read_write.c index a2cbc8303dae..bc2db5e5cd19 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -633,7 +633,7 @@ unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to) EXPORT_SYMBOL(iov_shorten); static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { struct kiocb kiocb; ssize_t ret; @@ -655,7 +655,7 @@ static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, /* Do it by hand, with file-ops */ static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { ssize_t ret = 0; @@ -871,7 +871,7 @@ ssize_t compat_rw_copy_check_uvector(int type, #endif static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -899,7 +899,7 @@ static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->read_iter) return -EINVAL; @@ -908,7 +908,7 @@ ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_read); static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -937,7 +937,7 @@ static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->write_iter) return -EINVAL; @@ -946,7 +946,7 @@ ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_write); ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, - unsigned long vlen, loff_t *pos, int flags) + unsigned long vlen, loff_t *pos, rwf_t flags) { struct iovec iovstack[UIO_FASTIOV]; struct iovec *iov = iovstack; @@ -964,7 +964,7 @@ ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, EXPORT_SYMBOL(vfs_readv); ssize_t vfs_writev(struct file *file, const struct iovec __user *vec, - unsigned long vlen, loff_t *pos, int flags) + unsigned long vlen, loff_t *pos, rwf_t flags) { struct iovec iovstack[UIO_FASTIOV]; struct iovec *iov = iovstack; @@ -981,7 +981,7 @@ ssize_t vfs_writev(struct file *file, const struct iovec __user *vec, EXPORT_SYMBOL(vfs_writev); static ssize_t do_readv(unsigned long fd, const struct iovec __user *vec, - unsigned long vlen, int flags) + unsigned long vlen, rwf_t flags) { struct fd f = fdget_pos(fd); ssize_t ret = -EBADF; @@ -1001,7 +1001,7 @@ static ssize_t do_readv(unsigned long fd, const struct iovec __user *vec, } static ssize_t do_writev(unsigned long fd, const struct iovec __user *vec, -unsigned long vlen, int flags) +unsigned long vlen, rwf_t flags) { struct fd f = fdget_pos(fd); ssize_t ret = -EBADF; @@ -1027,7 +1027,7 @@ static inline loff_t pos_from_hilo(unsigned long high, unsigned long low) } static ssize_t do_preadv(unsigned long fd, const struct iovec __user *vec, -unsigned
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 12:29:12AM +0100, Al Viro wrote: > On Thu, Jul 06, 2017 at 12:52:35AM +0200, Christoph Hellwig wrote: > > On Wed, Jul 05, 2017 at 11:38:21PM +0100, Al Viro wrote: > > > Sure, makes sense - especially since it's not too widely spread yet. > > > > Do you want to do that yourself, or do you want me to look into it? > > I'll do it tomorrow, unless you get to it first... Just did the whole batch (patch below), but it seems like using a __bitwise type in SYSCALL_DEFINE* will always give warnings like: fs/read_write.c:1095:1: warning: cast to restricted __kernel_rwf_t which I'm not sure to deal with.. -- diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 38d0383dc7f9..bc69d40c4e8b 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -969,7 +969,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file, int use_wgather; loff_t pos = offset; unsigned intpflags = current->flags; - int flags = 0; + rwf_t flags = 0; if (test_bit(RQ_LOCAL, >rq_flags)) /* diff --git a/fs/read_write.c b/fs/read_write.c index a2cbc8303dae..bc2db5e5cd19 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -633,7 +633,7 @@ unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to) EXPORT_SYMBOL(iov_shorten); static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { struct kiocb kiocb; ssize_t ret; @@ -655,7 +655,7 @@ static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter, /* Do it by hand, with file-ops */ static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter, - loff_t *ppos, int type, int flags) + loff_t *ppos, int type, rwf_t flags) { ssize_t ret = 0; @@ -871,7 +871,7 @@ ssize_t compat_rw_copy_check_uvector(int type, #endif static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -899,7 +899,7 @@ static ssize_t do_iter_read(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->read_iter) return -EINVAL; @@ -908,7 +908,7 @@ ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_read); static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, - loff_t *pos, int flags) + loff_t *pos, rwf_t flags) { size_t tot_len; ssize_t ret = 0; @@ -937,7 +937,7 @@ static ssize_t do_iter_write(struct file *file, struct iov_iter *iter, } ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, - int flags) + rwf_t flags) { if (!file->f_op->write_iter) return -EINVAL; @@ -946,7 +946,7 @@ ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos, EXPORT_SYMBOL(vfs_iter_write); ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, - unsigned long vlen, loff_t *pos, int flags) + unsigned long vlen, loff_t *pos, rwf_t flags) { struct iovec iovstack[UIO_FASTIOV]; struct iovec *iov = iovstack; @@ -964,7 +964,7 @@ ssize_t vfs_readv(struct file *file, const struct iovec __user *vec, EXPORT_SYMBOL(vfs_readv); ssize_t vfs_writev(struct file *file, const struct iovec __user *vec, - unsigned long vlen, loff_t *pos, int flags) + unsigned long vlen, loff_t *pos, rwf_t flags) { struct iovec iovstack[UIO_FASTIOV]; struct iovec *iov = iovstack; @@ -981,7 +981,7 @@ ssize_t vfs_writev(struct file *file, const struct iovec __user *vec, EXPORT_SYMBOL(vfs_writev); static ssize_t do_readv(unsigned long fd, const struct iovec __user *vec, - unsigned long vlen, int flags) + unsigned long vlen, rwf_t flags) { struct fd f = fdget_pos(fd); ssize_t ret = -EBADF; @@ -1001,7 +1001,7 @@ static ssize_t do_readv(unsigned long fd, const struct iovec __user *vec, } static ssize_t do_writev(unsigned long fd, const struct iovec __user *vec, -unsigned long vlen, int flags) +unsigned long vlen, rwf_t flags) { struct fd f = fdget_pos(fd); ssize_t ret = -EBADF; @@ -1027,7 +1027,7 @@ static inline loff_t pos_from_hilo(unsigned long high, unsigned long low) } static ssize_t do_preadv(unsigned long fd, const struct iovec __user *vec, -unsigned
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 12:52:35AM +0200, Christoph Hellwig wrote: > On Wed, Jul 05, 2017 at 11:38:21PM +0100, Al Viro wrote: > > Sure, makes sense - especially since it's not too widely spread yet. > > Do you want to do that yourself, or do you want me to look into it? I'll do it tomorrow, unless you get to it first...
Re: [git pull] vfs.git part 3
On Thu, Jul 06, 2017 at 12:52:35AM +0200, Christoph Hellwig wrote: > On Wed, Jul 05, 2017 at 11:38:21PM +0100, Al Viro wrote: > > Sure, makes sense - especially since it's not too widely spread yet. > > Do you want to do that yourself, or do you want me to look into it? I'll do it tomorrow, unless you get to it first...
Re: [git pull] vfs.git part 3
On Wed, Jul 05, 2017 at 11:38:21PM +0100, Al Viro wrote: > Sure, makes sense - especially since it's not too widely spread yet. Do you want to do that yourself, or do you want me to look into it?
Re: [git pull] vfs.git part 3
On Wed, Jul 05, 2017 at 11:38:21PM +0100, Al Viro wrote: > Sure, makes sense - especially since it's not too widely spread yet. Do you want to do that yourself, or do you want me to look into it?
Re: [git pull] vfs.git part 3
On Wed, Jul 05, 2017 at 02:51:43PM -0700, Linus Torvalds wrote: > On Wed, Jul 5, 2017 at 12:14 AM, Al Virowrote: > > > > Christoph's fs/read_write.c series - consolidation and cleanups. > > Side note - when looking through this, it struck me how confusing that > "int flags" argument was. > > We have a ton of "flags" in the filesystem layer, and how all the > read/write helpers take them too, and it's really hard to see what > kind of flags they are. > > Could we perhaps make those RWF_xyz flags have a nice bitwise type, > and use that type in the argument list, so that not only could there > be some sparse typechecking, but the functions that pass flags on to > each other would automatically have a certain amount of actual > self-documenting prototypes? > > So when you look at one of those vfs_iter_write() or whatever > functions, you just see *what* flags the flags argument is. > > Because "int flags" really is the worst. It's the wrong type anyway > (at least make it unsigned if it's a collection of bits), but it's > also very ambiguous indeed when there are so many other flags that are > often used/tested in the same functions (there's the "iter" flagsm, > there's file->f_mode, there's just a lot of different flags going on, > and the "int flags" is the least well documented of them all, > particularly since 99.9% of all users just pass in zero). Sure, makes sense - especially since it's not too widely spread yet. A side note right back at you - POLL... stuff. I'd redone the old "hunt the buggy ->poll() instances down" series (took about 12 hours total), got it to the point where all remaining sparse warnings about that type are for genuine bugs. It goes like that: define __poll_t, annotate constants Type is controlled by ifdef - it's unsigned int unless CHECK_POLL is defined and a bitwise type otherwise. ->poll() methods should return __poll_t anntotate the places where ->poll() return values go annotate poll-related wait keys annotate poll_table_struct ->_key That ends all infrastructure work. Methods declarations are annotated, instances are *not*. Due to that ifdef CHECK_POLL, normal builds, including normal sparse builds, are unaffected; with CF=-DCHECK_POLL you get __poll_t warnings. cris: annotate ->poll() instances ia64: annotate ->poll() instances mips: annotate ->poll() instances ppc: annotate ->poll() instances um: annotate ->poll() instances x86: annotate ->poll() instances block: annotate ->poll() instances crypto: annotate ->poll() instances acpi: annotate ->poll() instances sound: annotate ->poll() instances tomoyo: annotate ->poll() instances net: annotate ->poll() instances ipc, kernel, mm: annotate ->poll() instances fs: annotate ->poll() instances media: annotate ->poll() instances the rest of drivers/*: annotate ->poll() instances These can be folded and split as desired - almost up to per-instance. It's pretty much "turn unsigned int foo_poll(...) into __poll_t foo_poll(...), turn unsigned int mask; in it into __poll_t mask;" kind of stuff. Can go on per-subsystem basis just fine - again, normal builds are completely unaffected. scif: annotate scif_pollepd vhost: annotate vhost_poll dmabuf: annotate dma_buf->active Several drivers playing games of their own with POLL... bitmaps. annotate fs/select.c and fs/eventpoll.c That, of course, can move up right after the infrastructure. Again, can be reordered in front of the entire queue. Some are brainos (POLL_IN instead of POLLIN - compare the kernel definitions of those), some are "what do you mean, no returning -E... from ->poll()?". However, there's the shitty part - poll/epoll ABI mess. POLLWR... and POLLRDHUP are architecture-dependent; EPOLL counterparts are not and both are parts of ABI. Consider e.g. sparc: #define POLLWRNORM POLLOUT [4, that is] #define POLLWRBAND 256 #define POLLMSG 512 #define POLLREMOVE 1024 #define POLLRDHUP 2048 and compare with #define EPOLLWRNORM 0x0100 #define EPOLLWRBAND 0x0200 #define EPOLLMSG0x0400 #define EPOLLRDHUP 0x2000 EPOLLRDHUP is never matched. Neither is EPOLLMSG (nothing raises POLLREMOVE, but then nothing raises POLLMSG either). EPOLLWRBAND is not matched either (that would be POLLMSG). And EPOLLWRNORM is matched when we raise POLLWRBAND. sparc is the worst case in that respect; mips is somewhat better - there we have #define POLLWRNORM POLLOUT #define POLLWRBAND 0x0100 and everything else is default. IOW, EPOLLWRBAND is never matched and EPOLLWRNORM is matched when we raise POLLWRBAND. Several other architectures are like mips (m68k and even more exotic stuff). I'm not sure what to do about that. Davem is probably in the best position to tell... It
Re: [git pull] vfs.git part 3
On Wed, Jul 05, 2017 at 02:51:43PM -0700, Linus Torvalds wrote: > On Wed, Jul 5, 2017 at 12:14 AM, Al Viro wrote: > > > > Christoph's fs/read_write.c series - consolidation and cleanups. > > Side note - when looking through this, it struck me how confusing that > "int flags" argument was. > > We have a ton of "flags" in the filesystem layer, and how all the > read/write helpers take them too, and it's really hard to see what > kind of flags they are. > > Could we perhaps make those RWF_xyz flags have a nice bitwise type, > and use that type in the argument list, so that not only could there > be some sparse typechecking, but the functions that pass flags on to > each other would automatically have a certain amount of actual > self-documenting prototypes? > > So when you look at one of those vfs_iter_write() or whatever > functions, you just see *what* flags the flags argument is. > > Because "int flags" really is the worst. It's the wrong type anyway > (at least make it unsigned if it's a collection of bits), but it's > also very ambiguous indeed when there are so many other flags that are > often used/tested in the same functions (there's the "iter" flagsm, > there's file->f_mode, there's just a lot of different flags going on, > and the "int flags" is the least well documented of them all, > particularly since 99.9% of all users just pass in zero). Sure, makes sense - especially since it's not too widely spread yet. A side note right back at you - POLL... stuff. I'd redone the old "hunt the buggy ->poll() instances down" series (took about 12 hours total), got it to the point where all remaining sparse warnings about that type are for genuine bugs. It goes like that: define __poll_t, annotate constants Type is controlled by ifdef - it's unsigned int unless CHECK_POLL is defined and a bitwise type otherwise. ->poll() methods should return __poll_t anntotate the places where ->poll() return values go annotate poll-related wait keys annotate poll_table_struct ->_key That ends all infrastructure work. Methods declarations are annotated, instances are *not*. Due to that ifdef CHECK_POLL, normal builds, including normal sparse builds, are unaffected; with CF=-DCHECK_POLL you get __poll_t warnings. cris: annotate ->poll() instances ia64: annotate ->poll() instances mips: annotate ->poll() instances ppc: annotate ->poll() instances um: annotate ->poll() instances x86: annotate ->poll() instances block: annotate ->poll() instances crypto: annotate ->poll() instances acpi: annotate ->poll() instances sound: annotate ->poll() instances tomoyo: annotate ->poll() instances net: annotate ->poll() instances ipc, kernel, mm: annotate ->poll() instances fs: annotate ->poll() instances media: annotate ->poll() instances the rest of drivers/*: annotate ->poll() instances These can be folded and split as desired - almost up to per-instance. It's pretty much "turn unsigned int foo_poll(...) into __poll_t foo_poll(...), turn unsigned int mask; in it into __poll_t mask;" kind of stuff. Can go on per-subsystem basis just fine - again, normal builds are completely unaffected. scif: annotate scif_pollepd vhost: annotate vhost_poll dmabuf: annotate dma_buf->active Several drivers playing games of their own with POLL... bitmaps. annotate fs/select.c and fs/eventpoll.c That, of course, can move up right after the infrastructure. Again, can be reordered in front of the entire queue. Some are brainos (POLL_IN instead of POLLIN - compare the kernel definitions of those), some are "what do you mean, no returning -E... from ->poll()?". However, there's the shitty part - poll/epoll ABI mess. POLLWR... and POLLRDHUP are architecture-dependent; EPOLL counterparts are not and both are parts of ABI. Consider e.g. sparc: #define POLLWRNORM POLLOUT [4, that is] #define POLLWRBAND 256 #define POLLMSG 512 #define POLLREMOVE 1024 #define POLLRDHUP 2048 and compare with #define EPOLLWRNORM 0x0100 #define EPOLLWRBAND 0x0200 #define EPOLLMSG0x0400 #define EPOLLRDHUP 0x2000 EPOLLRDHUP is never matched. Neither is EPOLLMSG (nothing raises POLLREMOVE, but then nothing raises POLLMSG either). EPOLLWRBAND is not matched either (that would be POLLMSG). And EPOLLWRNORM is matched when we raise POLLWRBAND. sparc is the worst case in that respect; mips is somewhat better - there we have #define POLLWRNORM POLLOUT #define POLLWRBAND 0x0100 and everything else is default. IOW, EPOLLWRBAND is never matched and EPOLLWRNORM is matched when we raise POLLWRBAND. Several other architectures are like mips (m68k and even more exotic stuff). I'm not sure what to do about that. Davem is probably in the best position to tell... It might be worth merging the
Re: [git pull] vfs.git part 3
On Wed, Jul 5, 2017 at 12:14 AM, Al Virowrote: > > Christoph's fs/read_write.c series - consolidation and cleanups. Side note - when looking through this, it struck me how confusing that "int flags" argument was. We have a ton of "flags" in the filesystem layer, and how all the read/write helpers take them too, and it's really hard to see what kind of flags they are. Could we perhaps make those RWF_xyz flags have a nice bitwise type, and use that type in the argument list, so that not only could there be some sparse typechecking, but the functions that pass flags on to each other would automatically have a certain amount of actual self-documenting prototypes? So when you look at one of those vfs_iter_write() or whatever functions, you just see *what* flags the flags argument is. Because "int flags" really is the worst. It's the wrong type anyway (at least make it unsigned if it's a collection of bits), but it's also very ambiguous indeed when there are so many other flags that are often used/tested in the same functions (there's the "iter" flagsm, there's file->f_mode, there's just a lot of different flags going on, and the "int flags" is the least well documented of them all, particularly since 99.9% of all users just pass in zero). Hmm? Linus
Re: [git pull] vfs.git part 3
On Wed, Jul 5, 2017 at 12:14 AM, Al Viro wrote: > > Christoph's fs/read_write.c series - consolidation and cleanups. Side note - when looking through this, it struck me how confusing that "int flags" argument was. We have a ton of "flags" in the filesystem layer, and how all the read/write helpers take them too, and it's really hard to see what kind of flags they are. Could we perhaps make those RWF_xyz flags have a nice bitwise type, and use that type in the argument list, so that not only could there be some sparse typechecking, but the functions that pass flags on to each other would automatically have a certain amount of actual self-documenting prototypes? So when you look at one of those vfs_iter_write() or whatever functions, you just see *what* flags the flags argument is. Because "int flags" really is the worst. It's the wrong type anyway (at least make it unsigned if it's a collection of bits), but it's also very ambiguous indeed when there are so many other flags that are often used/tested in the same functions (there's the "iter" flagsm, there's file->f_mode, there's just a lot of different flags going on, and the "int flags" is the least well documented of them all, particularly since 99.9% of all users just pass in zero). Hmm? Linus