From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from zeniv.linux.org.uk ([195.92.253.2]:50608 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750711AbeEaVD2 (ORCPT ); Thu, 31 May 2018 17:03:28 -0400 Date: Thu, 31 May 2018 22:03:27 +0100 From: Al Viro To: Mike Marshall Cc: linux-fsdevel Subject: Re: [PATCH 4/4] orangefs: simplify compat ioctl handling Message-ID: <20180531210327.GI30522@ZenIV.linux.org.uk> References: <20180528175430.GC30522@ZenIV.linux.org.uk> <20180528222013.18402-1-viro@ZenIV.linux.org.uk> <20180528222013.18402-4-viro@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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;