diff mbox

[V5,07/10] aacraid: Created new mutex for ioctl path

Message ID 1454461088-32482-8-git-send-email-RaghavaAditya.Renukunta@pmcs.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Raghava Aditya Renukunta Feb. 3, 2016, 12:58 a.m. UTC 
aac_mutex was used to create protect the ioctl path for only the
compat path, it would be make more sense to place mutex in
aac_do_ioctl, which is the main ioctl function call that handles
all ioctl commands.

Created new mutex ioctl_mutex in struct aac_dev to protect switch
case in aac_do_ioctl and removed aac_mutex from aac_cfg_ioctl and
aac_compat_do_ioctl

Signed-off-by: Raghava Aditya Renukunta <RaghavaAditya.Renukunta@pmcs.com>
---
 drivers/scsi/aacraid/aacraid.h  |  1 +
 drivers/scsi/aacraid/commctrl.c |  4 ++++
 drivers/scsi/aacraid/linit.c    | 12 ++----------
 3 files changed, 7 insertions(+), 10 deletions(-)

Comments

kernel test robot Feb. 3, 2016, 7:35 a.m. UTC | #1 
Hi Raghava,

[auto build test ERROR on scsi/for-next]
[also build test ERROR on v4.5-rc2 next-20160203]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Raghava-Aditya-Renukunta/aacraid-Patchset-for-aacraid-driver-version-41052/20160203-085108
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: x86_64-lkp (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Raghava-Aditya-Renukunta/aacraid-Patchset-for-aacraid-driver-version-41052/20160203-085108 HEAD 6b0bf60a881075778b6548999d5bc8d04268996b builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/scsi/aacraid/linit.c: In function 'aac_cfg_ioctl':
>> drivers/scsi/aacraid/linit.c:706:33: error: 'aac' undeclared (first use in this function)
     if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
                                    ^
   drivers/scsi/aacraid/linit.c:706:33: note: each undeclared identifier is reported only once for each function it appears in

vim +/aac +706 drivers/scsi/aacraid/linit.c

^1da177e Linus Torvalds           2005-04-16  700   *	Bugs: Needs to handle hot plugging
^1da177e Linus Torvalds           2005-04-16  701   */
^1da177e Linus Torvalds           2005-04-16  702  
f4927c45 Arnd Bergmann            2010-04-27  703  static long aac_cfg_ioctl(struct file *file,
^1da177e Linus Torvalds           2005-04-16  704  		unsigned int cmd, unsigned long arg)
^1da177e Linus Torvalds           2005-04-16  705  {
f9c42596 Mahesh Rajashekhara      2015-03-26 @706  	if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
60395bb6 Alan Cox                 2007-07-23  707  		return -EPERM;
99da1769 Raghava Aditya Renukunta 2016-02-02  708  	return aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
^1da177e Linus Torvalds           2005-04-16  709  }

:::::: The code at line 706 was first introduced by commit
:::::: f9c4259678cbde854a4e94398d66ef379178fd7c aacraid: IOCTL fix

:::::: TO: Mahesh Rajashekhara <Mahesh.Rajashekhara@pmcs.com>
:::::: CC: James Bottomley <JBottomley@Odin.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Tomas Henzl Feb. 3, 2016, 10:37 a.m. UTC | #2 
On 3.2.2016 01:58, Raghava Aditya Renukunta wrote:
> aac_mutex was used to create protect the ioctl path for only the
> compat path, it would be make more sense to place mutex in
> aac_do_ioctl, which is the main ioctl function call that handles
> all ioctl commands.
>
> Created new mutex ioctl_mutex in struct aac_dev to protect switch
> case in aac_do_ioctl and removed aac_mutex from aac_cfg_ioctl and
> aac_compat_do_ioctl
>
> Signed-off-by: Raghava Aditya Renukunta <RaghavaAditya.Renukunta@pmcs.com>
> ---
>  drivers/scsi/aacraid/aacraid.h  |  1 +
>  drivers/scsi/aacraid/commctrl.c |  4 ++++
>  drivers/scsi/aacraid/linit.c    | 12 ++----------
>  3 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 2916288..75bc65e 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -1124,6 +1124,7 @@ struct aac_dev
>  	struct fib		*free_fib;
>  	spinlock_t		fib_lock;
>  
> +	struct mutex		ioctl_mutex;
>  	struct aac_queue_block *queues;
>  	/*
>  	 *	The user API will use an IOCTL to register itself to receive
> diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
> index 54195a1..4d5f4e7 100644
> --- a/drivers/scsi/aacraid/commctrl.c
> +++ b/drivers/scsi/aacraid/commctrl.c
> @@ -855,6 +855,8 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
>  {
>  	int status;
>  
> +	mutex_lock(&dev->ioctl_mutex);
> +

       status = aac_dev_ioctl(dev, cmd, arg);
        if (status != -ENOTTY)
                return status;

That return^ needs a mutex unlock .
--tms

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Raghava Aditya Renukunta Feb. 3, 2016, 10:49 p.m. UTC | #3 
> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@redhat.com]
> Sent: Wednesday, February 3, 2016 2:38 AM
> To: Raghava Aditya Renukunta; James.Bottomley@HansenPartnership.com;
> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> sierra.com; Scott Benesh; jthumshirn@suse.de; shane.seymour@hpe.com
> Subject: Re: [PATCH V5 07/10] aacraid: Created new mutex for ioctl path
> 
> On 3.2.2016 01:58, Raghava Aditya Renukunta wrote:
> > aac_mutex was used to create protect the ioctl path for only the
> > compat path, it would be make more sense to place mutex in
> > aac_do_ioctl, which is the main ioctl function call that handles
> > all ioctl commands.
> >
> > Created new mutex ioctl_mutex in struct aac_dev to protect switch
> > case in aac_do_ioctl and removed aac_mutex from aac_cfg_ioctl and
> > aac_compat_do_ioctl
> >
> > Signed-off-by: Raghava Aditya Renukunta
> <RaghavaAditya.Renukunta@pmcs.com>
> > ---
> >  drivers/scsi/aacraid/aacraid.h  |  1 +
> >  drivers/scsi/aacraid/commctrl.c |  4 ++++
> >  drivers/scsi/aacraid/linit.c    | 12 ++----------
> >  3 files changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> > index 2916288..75bc65e 100644
> > --- a/drivers/scsi/aacraid/aacraid.h
> > +++ b/drivers/scsi/aacraid/aacraid.h
> > @@ -1124,6 +1124,7 @@ struct aac_dev
> >  	struct fib		*free_fib;
> >  	spinlock_t		fib_lock;
> >
> > +	struct mutex		ioctl_mutex;
> >  	struct aac_queue_block *queues;
> >  	/*
> >  	 *	The user API will use an IOCTL to register itself to receive
> > diff --git a/drivers/scsi/aacraid/commctrl.c
> b/drivers/scsi/aacraid/commctrl.c
> > index 54195a1..4d5f4e7 100644
> > --- a/drivers/scsi/aacraid/commctrl.c
> > +++ b/drivers/scsi/aacraid/commctrl.c
> > @@ -855,6 +855,8 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void
> __user *arg)
> >  {
> >  	int status;
> >
> > +	mutex_lock(&dev->ioctl_mutex);
> > +
> 
>        status = aac_dev_ioctl(dev, cmd, arg);
>         if (status != -ENOTTY)
>                 return status;
> 
> That return^ needs a mutex unlock .
> --tms

Will do

Regards,
Raghava Aditya

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 2916288..75bc65e 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -1124,6 +1124,7 @@  struct aac_dev
 	struct fib		*free_fib;
 	spinlock_t		fib_lock;
 
+	struct mutex		ioctl_mutex;
 	struct aac_queue_block *queues;
 	/*
 	 *	The user API will use an IOCTL to register itself to receive
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 54195a1..4d5f4e7 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -855,6 +855,8 @@  int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
 {
 	int status;
 
+	mutex_lock(&dev->ioctl_mutex);
+
 	/*
 	 *	HBA gets first crack
 	 */
@@ -890,6 +892,8 @@  int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
 		status = -ENOTTY;
 		break;
 	}
+	mutex_unlock(&dev->ioctl_mutex);
+
 	return status;
 }
 
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 48e2a79..5f08bcf 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -703,23 +703,15 @@  static int aac_cfg_open(struct inode *inode, struct file *file)
 static long aac_cfg_ioctl(struct file *file,
 		unsigned int cmd, unsigned long arg)
 {
-	int ret;
-	struct aac_dev *aac;
-	aac = (struct aac_dev *)file->private_data;
 	if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
 		return -EPERM;
-	mutex_lock(&aac_mutex);
-	ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
-	mutex_unlock(&aac_mutex);
-
-	return ret;
+	return aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
 }
 
 #ifdef CONFIG_COMPAT
 static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long arg)
 {
 	long ret;
-	mutex_lock(&aac_mutex);
 	switch (cmd) {
 	case FSACTL_MINIPORT_REV_CHECK:
 	case FSACTL_SENDFIB:
@@ -753,7 +745,6 @@  static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
 		ret = -ENOIOCTLCMD;
 		break;
 	}
-	mutex_unlock(&aac_mutex);
 	return ret;
 }
 
@@ -1194,6 +1185,7 @@  static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto out_free_host;
 	spin_lock_init(&aac->fib_lock);
 
+	mutex_init(&aac->ioctl_mutex);
 	/*
 	 *	Map in the registers from the adapter.
 	 */