From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Date: Wed, 3 Feb 2016 14:40:35 -0800 Subject: [lustre-devel] [PATCH 5/7] staging:lustre: simplify libcfs_psdev_[open|release] In-Reply-To: <1452022519-6716-6-git-send-email-jsimmons@infradead.org> References: <1452022519-6716-1-git-send-email-jsimmons@infradead.org> <1452022519-6716-6-git-send-email-jsimmons@infradead.org> Message-ID: <20160203224035.GA10529@kroah.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: James Simmons Cc: devel@driverdev.osuosl.org, Oleg Drokin , Andreas Dilger , Parinay Kondekar , James Simmons , Linux Kernel Mailing List , lustre-devel@lists.lustre.org On Tue, Jan 05, 2016 at 02:35:17PM -0500, James Simmons wrote: > From: Parinay Kondekar > > With struct libcfs_device_userstate gone we can move > the remaining code of libcfs_psdev_ops.p_[open|close] > into the libcfs_psdev_[open|release] functions directly. > > Signed-off-by: Parinay Kondekar > Signed-off-by: James Simmons > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-5844 > Reviewed-on: http://review.whamcloud.com/17492 > Reviewed-by: Andreas Dilger > Reviewed-by: Dmitry Eremin > Reviewed-by: John L. Hammond > Reviewed-by: Oleg Drokin > --- > .../staging/lustre/include/linux/libcfs/libcfs.h | 2 -- > .../lustre/lustre/libcfs/linux/linux-module.c | 20 ++++++-------------- > drivers/staging/lustre/lustre/libcfs/module.c | 16 ---------------- > 3 files changed, 6 insertions(+), 32 deletions(-) > > diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs.h b/drivers/staging/lustre/include/linux/libcfs/libcfs.h > index 0d8a91e..06bb676 100644 > --- a/drivers/staging/lustre/include/linux/libcfs/libcfs.h > +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs.h > @@ -73,8 +73,6 @@ struct cfs_psdev_file { > }; > > struct cfs_psdev_ops { > - int (*p_open)(unsigned long, void *); > - int (*p_close)(unsigned long, void *); > int (*p_read)(struct cfs_psdev_file *, char *, unsigned long); > int (*p_write)(struct cfs_psdev_file *, char *, unsigned long); > int (*p_ioctl)(struct cfs_psdev_file *, unsigned long, void *); > diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c b/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c > index 33f6036..64f0fbf 100644 > --- a/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c > +++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c > @@ -98,30 +98,22 @@ int libcfs_ioctl_popdata(void *arg, void *data, int size) > static int > libcfs_psdev_open(struct inode *inode, struct file *file) > { > - int rc = 0; > - > if (!inode) > return -EINVAL; > - if (libcfs_psdev_ops.p_open != NULL) > - rc = libcfs_psdev_ops.p_open(0, NULL); > - else > - return -EPERM; > - return rc; > + > + try_module_get(THIS_MODULE); Note, code like this is racy and incorrect and never needed, please fix this up properly (hint, set the module in the file operations.) Again, if you ever see code with that line, it is incorrect. thanks, greg k-h From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757894AbcBCWkk (ORCPT ); Wed, 3 Feb 2016 17:40:40 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:46135 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754458AbcBCWkg (ORCPT ); Wed, 3 Feb 2016 17:40:36 -0500 Date: Wed, 3 Feb 2016 14:40:35 -0800 From: Greg Kroah-Hartman To: James Simmons Cc: devel@driverdev.osuosl.org, Oleg Drokin , Andreas Dilger , Parinay Kondekar , James Simmons , Linux Kernel Mailing List , lustre-devel@lists.lustre.org Subject: Re: [PATCH 5/7] staging:lustre: simplify libcfs_psdev_[open|release] Message-ID: <20160203224035.GA10529@kroah.com> References: <1452022519-6716-1-git-send-email-jsimmons@infradead.org> <1452022519-6716-6-git-send-email-jsimmons@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1452022519-6716-6-git-send-email-jsimmons@infradead.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 05, 2016 at 02:35:17PM -0500, James Simmons wrote: > From: Parinay Kondekar > > With struct libcfs_device_userstate gone we can move > the remaining code of libcfs_psdev_ops.p_[open|close] > into the libcfs_psdev_[open|release] functions directly. > > Signed-off-by: Parinay Kondekar > Signed-off-by: James Simmons > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-5844 > Reviewed-on: http://review.whamcloud.com/17492 > Reviewed-by: Andreas Dilger > Reviewed-by: Dmitry Eremin > Reviewed-by: John L. Hammond > Reviewed-by: Oleg Drokin > --- > .../staging/lustre/include/linux/libcfs/libcfs.h | 2 -- > .../lustre/lustre/libcfs/linux/linux-module.c | 20 ++++++-------------- > drivers/staging/lustre/lustre/libcfs/module.c | 16 ---------------- > 3 files changed, 6 insertions(+), 32 deletions(-) > > diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs.h b/drivers/staging/lustre/include/linux/libcfs/libcfs.h > index 0d8a91e..06bb676 100644 > --- a/drivers/staging/lustre/include/linux/libcfs/libcfs.h > +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs.h > @@ -73,8 +73,6 @@ struct cfs_psdev_file { > }; > > struct cfs_psdev_ops { > - int (*p_open)(unsigned long, void *); > - int (*p_close)(unsigned long, void *); > int (*p_read)(struct cfs_psdev_file *, char *, unsigned long); > int (*p_write)(struct cfs_psdev_file *, char *, unsigned long); > int (*p_ioctl)(struct cfs_psdev_file *, unsigned long, void *); > diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c b/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c > index 33f6036..64f0fbf 100644 > --- a/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c > +++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c > @@ -98,30 +98,22 @@ int libcfs_ioctl_popdata(void *arg, void *data, int size) > static int > libcfs_psdev_open(struct inode *inode, struct file *file) > { > - int rc = 0; > - > if (!inode) > return -EINVAL; > - if (libcfs_psdev_ops.p_open != NULL) > - rc = libcfs_psdev_ops.p_open(0, NULL); > - else > - return -EPERM; > - return rc; > + > + try_module_get(THIS_MODULE); Note, code like this is racy and incorrect and never needed, please fix this up properly (hint, set the module in the file operations.) Again, if you ever see code with that line, it is incorrect. thanks, greg k-h