* Re: [PATCH] move file->f_op->open out of mutex (misc_mtx) in misc_open
@ 2015-01-05 9:14 Hillf Danton
0 siblings, 0 replies; 4+ messages in thread
From: Hillf Danton @ 2015-01-05 9:14 UTC (permalink / raw)
To: 'xiaomin1'
Cc: 'Zhang Dongxing', arnd, gregkh, linux-kernel, Chuansheng Liu
>
> misc_mtx was used to mutex misc_list.
> But file->f_op->open may be blocked by downloading
> firmware in some devices' open.
> So move file->f_op->open out of mutex (misc_mtx)
> in misc_open to avoid this block.
> It also make code more efficiency.
>
> Signed-off-by: Zhang Dongxing <dongxing.zhang@intel.com>
> Signed-off-by: xiaoming wang <xiaoming.wang@intel.com>
> ---
> drivers/char/misc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/misc.c b/drivers/char/misc.c
> index ffa97d2..289c59e 100644
> --- a/drivers/char/misc.c
> +++ b/drivers/char/misc.c
> @@ -144,7 +144,8 @@ static int misc_open(struct inode * inode, struct file * file)
> replace_fops(file, new_fops);
> if (file->f_op->open) {
> file->private_data = c;
> - err = file->f_op->open(inode,file);
> + mutex_unlock(&misc_mtx);
> + return file->f_op->open(inode,file);
> }
What if unregister breaks in while opening?
> fail:
> mutex_unlock(&misc_mtx);
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] move file->f_op->open out of mutex (misc_mtx) in misc_open
2015-01-06 0:34 xiaomin1
2015-01-05 13:58 ` One Thousand Gnomes
@ 2015-01-09 22:47 ` Greg KH
1 sibling, 0 replies; 4+ messages in thread
From: Greg KH @ 2015-01-09 22:47 UTC (permalink / raw)
To: xiaomin1; +Cc: arnd, linux-kernel, chuansheng.liu, Zhang Dongxing
On Tue, Jan 06, 2015 at 08:34:07AM +0800, xiaomin1 wrote:
> misc_mtx was used to mutex misc_list.
> But file->f_op->open may be blocked by downloading
> firmware in some devices' open.
> So move file->f_op->open out of mutex (misc_mtx)
> in misc_open to avoid this block.
> It also make code more efficiency.
>
> Signed-off-by: Zhang Dongxing <dongxing.zhang@intel.com>
> Signed-off-by: xiaoming wang <xiaoming.wang@intel.com>
> ---
> drivers/char/misc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Please fix up your "From:" line to match your signed-off-by line, as I
can't take it the way you sent this.
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] move file->f_op->open out of mutex (misc_mtx) in misc_open
@ 2015-01-06 0:34 xiaomin1
2015-01-05 13:58 ` One Thousand Gnomes
2015-01-09 22:47 ` Greg KH
0 siblings, 2 replies; 4+ messages in thread
From: xiaomin1 @ 2015-01-06 0:34 UTC (permalink / raw)
To: arnd, gregkh, linux-kernel, chuansheng.liu; +Cc: xiaomin1, Zhang Dongxing
misc_mtx was used to mutex misc_list.
But file->f_op->open may be blocked by downloading
firmware in some devices' open.
So move file->f_op->open out of mutex (misc_mtx)
in misc_open to avoid this block.
It also make code more efficiency.
Signed-off-by: Zhang Dongxing <dongxing.zhang@intel.com>
Signed-off-by: xiaoming wang <xiaoming.wang@intel.com>
---
drivers/char/misc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index ffa97d2..289c59e 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -144,7 +144,8 @@ static int misc_open(struct inode * inode, struct file * file)
replace_fops(file, new_fops);
if (file->f_op->open) {
file->private_data = c;
- err = file->f_op->open(inode,file);
+ mutex_unlock(&misc_mtx);
+ return file->f_op->open(inode,file);
}
fail:
mutex_unlock(&misc_mtx);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] move file->f_op->open out of mutex (misc_mtx) in misc_open
2015-01-06 0:34 xiaomin1
@ 2015-01-05 13:58 ` One Thousand Gnomes
2015-01-09 22:47 ` Greg KH
1 sibling, 0 replies; 4+ messages in thread
From: One Thousand Gnomes @ 2015-01-05 13:58 UTC (permalink / raw)
To: xiaomin1; +Cc: arnd, gregkh, linux-kernel, chuansheng.liu, Zhang Dongxing
On Tue, 6 Jan 2015 08:34:07 +0800
xiaomin1 <xiaoming.wang@intel.com> wrote:
> misc_mtx was used to mutex misc_list.
> But file->f_op->open may be blocked by downloading
> firmware in some devices' open.
> So move file->f_op->open out of mutex (misc_mtx)
> in misc_open to avoid this block.
> It also make code more efficiency.
>
> Signed-off-by: Zhang Dongxing <dongxing.zhang@intel.com>
> Signed-off-by: xiaoming wang <xiaoming.wang@intel.com>
> ---
> drivers/char/misc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/misc.c b/drivers/char/misc.c
> index ffa97d2..289c59e 100644
> --- a/drivers/char/misc.c
> +++ b/drivers/char/misc.c
> @@ -144,7 +144,8 @@ static int misc_open(struct inode * inode, struct file * file)
> replace_fops(file, new_fops);
> if (file->f_op->open) {
> file->private_data = c;
> - err = file->f_op->open(inode,file);
> + mutex_unlock(&misc_mtx);
> + return file->f_op->open(inode,file);
> }
This changes all the users of misc device open from singlethreaded to
multi-threaded. Having read through the users I can find this seems safe
and they all seem to have the right checks in place and to assume they
are multi-threaded.
Reviewed-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-09 22:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-05 9:14 [PATCH] move file->f_op->open out of mutex (misc_mtx) in misc_open Hillf Danton
2015-01-06 0:34 xiaomin1
2015-01-05 13:58 ` One Thousand Gnomes
2015-01-09 22:47 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).