Messages in this thread | | | Date | Mon, 4 Jul 2022 23:07:54 +0900 | Subject | Re: [PATCH] char: misc: make misc_open() and misc_register() killable | From | Tetsuo Handa <> |
| |
On 2022/07/04 22:57, Wedson Almeida Filho wrote: > On Mon, Jul 04, 2022 at 10:48:32PM +0900, Tetsuo Handa wrote: >> On 2022/07/04 21:59, Wedson Almeida Filho wrote: >>>> @@ -139,6 +139,10 @@ static int misc_open(struct inode *inode, struct file *file) >>>> >>>> err = 0; >>>> replace_fops(file, new_fops); >>>> + if (iter->unlocked_open && file->f_op->open) { >>>> + mutex_unlock(&misc_mtx); >>>> + return file->f_op->open(inode, file); >>>> + } >>> >>> One of the invariants of miscdev is that once misc_deregister() returns, >>> no new calls to f_op->open() are made. (Although, of course, you can >>> still have open files but that's a whole different problem.) >> >> The point of this change is that file->f_op after mutex_unlock(&misc_mtx) is >> from new_fops which is guaranteed to hold a ref on "struct file_operations" >> via new_fops = fops_get("struct miscdevice"->fops). >> That is, a module ref remains valid even after mutex_unlock(&misc_mtx). >> >> And as with major_names_lock case quoted below, this change assumes that >> misc_deregister() is called from module's __exit function, and fops_get() >> is preventing the module owning new_fops from calling __exit function. > > Your assumption is not sound. misc_deregister() can be (and is) > legitimately called from other places, for example, a driver's remove() > callback. In fact, when I grep for misc_deregister(), the second > instance is such a case.
OK, the frequency of calling misc_deregister() can be much higher than unregister_blkdev(), which means that misc_mtx is more prone to trigger hung task warnings. I'm more inclined to avoid sleeping with misc_mtx held.
| |