| Message ID | 20180531210327.GI30522@ZenIV.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Thanks Al... I broke those out into individual commits, tested it all and added it to our for-next over on kernel.org. -Mike On Thu, May 31, 2018 at 5:03 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > On Thu, May 31, 2018 at 04:54:06PM -0400, Mike Marshall wrote: >> Thanks for the cleanup. This runs through xfstests with no regressions >> on 4.17-rc7. >> >> I studied what to do about the sparse warning, looked at the code, and >> looked for hints from the original authors in the pvfs svn commit messages. >> >> No luck with the old commit messages. >> >> I got the sparse warning to quit with this change, which also runs through >> xfstests with no regressions, does it seem OK? >> >> >> $ git diff >> diff --git a/fs/orangefs/protocol.h b/fs/orangefs/protocol.h >> index 61ee8d64c842..d403cf29a99b 100644 >> --- a/fs/orangefs/protocol.h >> +++ b/fs/orangefs/protocol.h >> @@ -342,7 +342,7 @@ enum { >> * that may be 32 bit! >> */ >> struct ORANGEFS_dev_map_desc { >> - void *ptr; >> + void __user *ptr; >> __s32 total_size; >> __s32 size; >> __s32 count; > > You want more than that - > --- a/fs/orangefs/orangefs-bufmap.c > +++ b/fs/orangefs/orangefs-bufmap.c > @@ -138,7 +138,7 @@ static int get(struct slot_map *m) > > /* used to describe mapped buffers */ > struct orangefs_bufmap_desc { > - void *uaddr; /* user space address pointer */ > + void __user *uaddr; /* user space address pointer */ > struct page **page_array; /* array of mapped pages */ > int array_count; /* size of above arrays */ > struct list_head list_link; > > to go with it. FWIW, the following takes care of almost all sparse > warnings in there; up to you whether to split it or not: > > diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c > index 26358efbf794..84f44365bfb3 100644 > --- a/fs/orangefs/file.c > +++ b/fs/orangefs/file.c > @@ -544,7 +544,7 @@ static int orangefs_fault(struct vm_fault *vmf) > return filemap_fault(vmf); > } > > -const struct vm_operations_struct orangefs_file_vm_ops = { > +static const struct vm_operations_struct orangefs_file_vm_ops = { > .fault = orangefs_fault, > .map_pages = filemap_map_pages, > .page_mkwrite = filemap_page_mkwrite, > diff --git a/fs/orangefs/orangefs-bufmap.c b/fs/orangefs/orangefs-bufmap.c > index 4f927023d095..0a29f57d4c8f 100644 > --- a/fs/orangefs/orangefs-bufmap.c > +++ b/fs/orangefs/orangefs-bufmap.c > @@ -138,7 +138,7 @@ static int get(struct slot_map *m) > > /* used to describe mapped buffers */ > struct orangefs_bufmap_desc { > - void *uaddr; /* user space address pointer */ > + void __user *uaddr; /* user space address pointer */ > struct page **page_array; /* array of mapped pages */ > int array_count; /* size of above arrays */ > struct list_head list_link; > @@ -215,19 +215,6 @@ int orangefs_bufmap_shift_query(void) > static DECLARE_WAIT_QUEUE_HEAD(bufmap_waitq); > static DECLARE_WAIT_QUEUE_HEAD(readdir_waitq); > > -/* > - * orangefs_get_bufmap_init > - * > - * If bufmap_init is 1, then the shared memory system, including the > - * buffer_index_array, is available. Otherwise, it is not. > - * > - * returns the value of bufmap_init > - */ > -int orangefs_get_bufmap_init(void) > -{ > - return __orangefs_bufmap ? 1 : 0; > -} > - > > static struct orangefs_bufmap * > orangefs_bufmap_alloc(struct ORANGEFS_dev_map_desc *user_desc) > diff --git a/fs/orangefs/orangefs-debugfs.c b/fs/orangefs/orangefs-debugfs.c > index 6e35f2f3c897..0732cb08173e 100644 > --- a/fs/orangefs/orangefs-debugfs.c > +++ b/fs/orangefs/orangefs-debugfs.c > @@ -114,7 +114,7 @@ static const struct seq_operations help_debug_ops = { > .show = help_show, > }; > > -const struct file_operations debug_help_fops = { > +static const struct file_operations debug_help_fops = { > .owner = THIS_MODULE, > .open = orangefs_debug_help_open, > .read = seq_read, > diff --git a/fs/orangefs/protocol.h b/fs/orangefs/protocol.h > index 61ee8d64c842..d403cf29a99b 100644 > --- a/fs/orangefs/protocol.h > +++ b/fs/orangefs/protocol.h > @@ -342,7 +342,7 @@ enum { > * that may be 32 bit! > */ > struct ORANGEFS_dev_map_desc { > - void *ptr; > + void __user *ptr; > __s32 total_size; > __s32 size; > __s32 count; > diff --git a/fs/orangefs/waitqueue.c b/fs/orangefs/waitqueue.c > index 0577d6dba8c8..3de323f17506 100644 > --- a/fs/orangefs/waitqueue.c > +++ b/fs/orangefs/waitqueue.c > @@ -17,8 +17,11 @@ > #include "orangefs-kernel.h" > #include "orangefs-bufmap.h" > > -static int wait_for_matching_downcall(struct orangefs_kernel_op_s *, long, bool); > -static void orangefs_clean_up_interrupted_operation(struct orangefs_kernel_op_s *); > +static int wait_for_matching_downcall(struct orangefs_kernel_op_s *op, > + long timeout, bool interruptible) > + __acquires(op->lock); > +static void orangefs_clean_up_interrupted_operation(struct orangefs_kernel_op_s *op) > + __releases(op->lock); > > /* > * What we do in this function is to walk the list of operations that are > @@ -245,7 +248,8 @@ bool orangefs_cancel_op_in_progress(struct orangefs_kernel_op_s *op) > * Change an op to the "given up" state and remove it from its list. > */ > static void > - orangefs_clean_up_interrupted_operation(struct orangefs_kernel_op_s *op) > +orangefs_clean_up_interrupted_operation(struct orangefs_kernel_op_s *op) > + __releases(op->lock) > { > /* > * handle interrupted cases depending on what state we were in when > @@ -315,6 +319,7 @@ static void > static int wait_for_matching_downcall(struct orangefs_kernel_op_s *op, > long timeout, > bool interruptible) > + __acquires(op->lock) > { > long n; >
--- a/fs/orangefs/orangefs-bufmap.c +++ b/fs/orangefs/orangefs-bufmap.c @@ -138,7 +138,7 @@ static int get(struct slot_map *m) /* used to describe mapped buffers */ struct orangefs_bufmap_desc { - void *uaddr; /* user space address pointer */ + void __user *uaddr; /* user space address pointer */ struct page **page_array; /* array of mapped pages */ int array_count; /* size of above arrays */ struct list_head list_link; to go with it. FWIW, the following takes care of almost all sparse warnings in there; up to you whether to split it or not: diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c index 26358efbf794..84f44365bfb3 100644 --- a/fs/orangefs/file.c +++ b/fs/orangefs/file.c @@ -544,7 +544,7 @@ static int orangefs_fault(struct vm_fault *vmf) return filemap_fault(vmf); } -const struct vm_operations_struct orangefs_file_vm_ops = { +static const struct vm_operations_struct orangefs_file_vm_ops = { .fault = orangefs_fault, .map_pages = filemap_map_pages, .page_mkwrite = filemap_page_mkwrite, diff --git a/fs/orangefs/orangefs-bufmap.c b/fs/orangefs/orangefs-bufmap.c index 4f927023d095..0a29f57d4c8f 100644 --- a/fs/orangefs/orangefs-bufmap.c +++ b/fs/orangefs/orangefs-bufmap.c @@ -138,7 +138,7 @@ static int get(struct slot_map *m) /* used to describe mapped buffers */ struct orangefs_bufmap_desc { - void *uaddr; /* user space address pointer */ + void __user *uaddr; /* user space address pointer */ struct page **page_array; /* array of mapped pages */ int array_count; /* size of above arrays */ struct list_head list_link; @@ -215,19 +215,6 @@ int orangefs_bufmap_shift_query(void) static DECLARE_WAIT_QUEUE_HEAD(bufmap_waitq); static DECLARE_WAIT_QUEUE_HEAD(readdir_waitq); -/* - * orangefs_get_bufmap_init - * - * If bufmap_init is 1, then the shared memory system, including the - * buffer_index_array, is available. Otherwise, it is not. - * - * returns the value of bufmap_init - */ -int orangefs_get_bufmap_init(void) -{ - return __orangefs_bufmap ? 1 : 0; -} - static struct orangefs_bufmap * orangefs_bufmap_alloc(struct ORANGEFS_dev_map_desc *user_desc) diff --git a/fs/orangefs/orangefs-debugfs.c b/fs/orangefs/orangefs-debugfs.c index 6e35f2f3c897..0732cb08173e 100644 --- a/fs/orangefs/orangefs-debugfs.c +++ b/fs/orangefs/orangefs-debugfs.c @@ -114,7 +114,7 @@ static const struct seq_operations help_debug_ops = { .show = help_show, }; -const struct file_operations debug_help_fops = { +static const struct file_operations debug_help_fops = { .owner = THIS_MODULE, .open = orangefs_debug_help_open, .read = seq_read, diff --git a/fs/orangefs/protocol.h b/fs/orangefs/protocol.h index 61ee8d64c842..d403cf29a99b 100644 --- a/fs/orangefs/protocol.h +++ b/fs/orangefs/protocol.h @@ -342,7 +342,7 @@ enum { * that may be 32 bit! */ struct ORANGEFS_dev_map_desc { - void *ptr; + void __user *ptr; __s32 total_size; __s32 size; __s32 count; diff --git a/fs/orangefs/waitqueue.c b/fs/orangefs/waitqueue.c index 0577d6dba8c8..3de323f17506 100644 --- a/fs/orangefs/waitqueue.c +++ b/fs/orangefs/waitqueue.c @@ -17,8 +17,11 @@ #include "orangefs-kernel.h" #include "orangefs-bufmap.h" -static int wait_for_matching_downcall(struct orangefs_kernel_op_s *, long, bool); -static void orangefs_clean_up_interrupted_operation(struct orangefs_kernel_op_s *); +static int wait_for_matching_downcall(struct orangefs_kernel_op_s *op, + long timeout, bool interruptible) + __acquires(op->lock); +static void orangefs_clean_up_interrupted_operation(struct orangefs_kernel_op_s *op) + __releases(op->lock); /* * What we do in this function is to walk the list of operations that are @@ -245,7 +248,8 @@ bool orangefs_cancel_op_in_progress(struct orangefs_kernel_op_s *op) * Change an op to the "given up" state and remove it from its list. */ static void - orangefs_clean_up_interrupted_operation(struct orangefs_kernel_op_s *op) +orangefs_clean_up_interrupted_operation(struct orangefs_kernel_op_s *op) + __releases(op->lock) { /* * handle interrupted cases depending on what state we were in when @@ -315,6 +319,7 @@ static void static int wait_for_matching_downcall(struct orangefs_kernel_op_s *op, long timeout, bool interruptible) + __acquires(op->lock) { long n;