lkml.org 
[lkml]   [2006]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] protect remove_proc_entry
Kirill Korotaev <dev@sw.ru> wrote:
>
> Hi Andrew,
>
> I have a full patch for this.

Please don't top-post. It makes things hard...

> I don't remember the details yet, but lock was not god here, we used
> semaphore. I pointed to this problem long ago when fixed error path in
> proc with moduleget.
>
> This patch protects proc_dir_entry tree with a proc_tree_sem semaphore.
> I suppose lock_kernel() can be removed later after checking that no proc
> handlers require it.
> Also this patch remakes de refcounters a bit making it more clear and
> more similar to dentry scheme - this is required to make sure that
> everything works correctly.
>
> Patch is against 2.6.15-rcX and was tested for about a week. Also works
> half a year on 2.6.8 :)
>
> [ patch which uses an rwsem for procfs and somewhat removes lock_kernel() ]
>

I worry about replacing a spinlock with a sleeping lock. In some
circumstances it can cause a complete scalability collapse and I suspect
this could happen with /proc. Although I guess the only fastpath here is
proc_readdir(), and as the lock is taken there for reading, we'll be OK..

The patch does leave some lock_kernel() calls behind. If we're going to do
this, I think they should all be removed?

Races in /proc have been plentiful and hard to find. The patch worries me,
frankly. I'd like to see quite a bit more description of the locking
schema and some demonstration that it's actually complete before taking the
plunge.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-01-04 10:40    [W:0.105 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site