Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Nick Piggin
Eric Dumazet wrote: Furthermore, a lazy sync would mean to change sysctl proc_handler for "file-nr" to perform a synchronize before calling proc_dointvec, this would be really obscure. I was only using your terminology (ie. the 'lazy' synch after the atomic is updated). Actually, a better

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Christoph Hellwig
On Thu, Aug 25, 2005 at 11:49:35PM +0530, Dipankar Sarma wrote: > On Thu, Aug 25, 2005 at 05:13:05PM +0200, Eric Dumazet wrote: > > Nick Piggin a ?crit : > > > > >OK, well I would prefer you do the proper atomic operations throughout > > >where it "really matters" in file_table.c, and do your

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Dipankar Sarma
On Thu, Aug 25, 2005 at 05:13:05PM +0200, Eric Dumazet wrote: > Nick Piggin a écrit : > > >OK, well I would prefer you do the proper atomic operations throughout > >where it "really matters" in file_table.c, and do your lazy synchronize > >with just the sysctl exported value. > > > > But... I

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
Nick Piggin a écrit : OK, well I would prefer you do the proper atomic operations throughout where it "really matters" in file_table.c, and do your lazy synchronize with just the sysctl exported value. But... I got complains about atomic_read() being 'an atomic op' (untrue), so my second

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Christoph Hellwig
On Fri, Aug 26, 2005 at 12:51:30AM +1000, Nick Piggin wrote: > Eric Dumazet wrote: > >Nick Piggin a ?crit : > > > > >>Would you just be able to add the atomic sysctl handler that > >>Christoph suggested? > >> > > > >Quite a lot of work indeed, and it would force to convert 3 int > >(nr_files,

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Nick Piggin
Eric Dumazet wrote: Nick Piggin a écrit : Would you just be able to add the atomic sysctl handler that Christoph suggested? Quite a lot of work indeed, and it would force to convert 3 int (nr_files, nr_free_files, max_files) to 3 atomic_t. I feel bad introducing a lot of sysctl rework

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
Nick Piggin a écrit : On Thu, 2005-08-25 at 12:41 +0200, Eric Dumazet wrote: OK, here is a new clean patch that address this problem (nothing assumed about atomics) Would you just be able to add the atomic sysctl handler that Christoph suggested? Quite a lot of work indeed, and it

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Nick Piggin
On Thu, 2005-08-25 at 12:41 +0200, Eric Dumazet wrote: > OK, here is a new clean patch that address this problem (nothing assumed > about > atomics) > Would you just be able to add the atomic sysctl handler that Christoph suggested? This introduces lost update problems. 2 CPUs may store to

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
Christoph Hellwig a écrit : On Thu, Aug 25, 2005 at 11:17:23AM +0200, Eric Dumazet wrote: But that's not true. You need to write you own sysctl handler for it, probably worth adding a generic atomic_t sysctl handler while you're at it. I checked linux-2.6.13-rc7 tree, and atomic_read() is

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Arjan van de Ven
> > > > this patch adds atomic ops where there were none before > > nope... a spinlock/spinunlock contains atomic ops. unlock usually doesn't but ok > > > for those architectures that need atomics for read (parisc? arm?) > > not today. No atomic needed for read. > > > > > however..

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Christoph Hellwig
On Thu, Aug 25, 2005 at 11:17:23AM +0200, Eric Dumazet wrote: > >But that's not true. You need to write you own sysctl handler for it, > >probably worth adding a generic atomic_t sysctl handler while you're > >at it. > > > > I checked linux-2.6.13-rc7 tree, and atomic_read() is just a wrapper to

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
Arjan van de Ven a écrit : On Thu, 2005-08-25 at 10:45 +0200, Eric Dumazet wrote: This patch removes filp_count_lock spinlock, used to protect files_stat.nr_files. Just use atomic_t type and atomic_inc()/atomic_dec() operations. This patch assumes that atomic_read() is a plain {return

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
Christoph Hellwig a écrit : On Thu, Aug 25, 2005 at 10:45:12AM +0200, Eric Dumazet wrote: This patch assumes that atomic_read() is a plain {return v->counter;} on all architectures. (keywords : sysctl, /proc/sys/fs/file-nr, proc_dointvec) But that's not true. You need to write you own

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Arjan van de Ven
On Thu, 2005-08-25 at 10:45 +0200, Eric Dumazet wrote: > This patch removes filp_count_lock spinlock, used to protect > files_stat.nr_files. > > Just use atomic_t type and atomic_inc()/atomic_dec() operations. > > This patch assumes that atomic_read() is a plain {return v->counter;} on all >

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Christoph Hellwig
On Thu, Aug 25, 2005 at 10:45:12AM +0200, Eric Dumazet wrote: > This patch assumes that atomic_read() is a plain {return v->counter;} on > all architectures. (keywords : sysctl, /proc/sys/fs/file-nr, proc_dointvec) But that's not true. You need to write you own sysctl handler for it, probably

[PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
This patch removes filp_count_lock spinlock, used to protect files_stat.nr_files. Just use atomic_t type and atomic_inc()/atomic_dec() operations. This patch assumes that atomic_read() is a plain {return v->counter;} on all architectures. (keywords : sysctl, /proc/sys/fs/file-nr,

[PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
This patch removes filp_count_lock spinlock, used to protect files_stat.nr_files. Just use atomic_t type and atomic_inc()/atomic_dec() operations. This patch assumes that atomic_read() is a plain {return v-counter;} on all architectures. (keywords : sysctl, /proc/sys/fs/file-nr,

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Christoph Hellwig
On Thu, Aug 25, 2005 at 10:45:12AM +0200, Eric Dumazet wrote: This patch assumes that atomic_read() is a plain {return v-counter;} on all architectures. (keywords : sysctl, /proc/sys/fs/file-nr, proc_dointvec) But that's not true. You need to write you own sysctl handler for it, probably

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Arjan van de Ven
On Thu, 2005-08-25 at 10:45 +0200, Eric Dumazet wrote: This patch removes filp_count_lock spinlock, used to protect files_stat.nr_files. Just use atomic_t type and atomic_inc()/atomic_dec() operations. This patch assumes that atomic_read() is a plain {return v-counter;} on all

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
Christoph Hellwig a écrit : On Thu, Aug 25, 2005 at 10:45:12AM +0200, Eric Dumazet wrote: This patch assumes that atomic_read() is a plain {return v-counter;} on all architectures. (keywords : sysctl, /proc/sys/fs/file-nr, proc_dointvec) But that's not true. You need to write you own

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
Arjan van de Ven a écrit : On Thu, 2005-08-25 at 10:45 +0200, Eric Dumazet wrote: This patch removes filp_count_lock spinlock, used to protect files_stat.nr_files. Just use atomic_t type and atomic_inc()/atomic_dec() operations. This patch assumes that atomic_read() is a plain {return

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Christoph Hellwig
On Thu, Aug 25, 2005 at 11:17:23AM +0200, Eric Dumazet wrote: But that's not true. You need to write you own sysctl handler for it, probably worth adding a generic atomic_t sysctl handler while you're at it. I checked linux-2.6.13-rc7 tree, and atomic_read() is just a wrapper to read

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Arjan van de Ven
this patch adds atomic ops where there were none before nope... a spinlock/spinunlock contains atomic ops. unlock usually doesn't but ok for those architectures that need atomics for read (parisc? arm?) not today. No atomic needed for read. however.. wouldn't it be better

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
Christoph Hellwig a écrit : On Thu, Aug 25, 2005 at 11:17:23AM +0200, Eric Dumazet wrote: But that's not true. You need to write you own sysctl handler for it, probably worth adding a generic atomic_t sysctl handler while you're at it. I checked linux-2.6.13-rc7 tree, and atomic_read() is

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Nick Piggin
On Thu, 2005-08-25 at 12:41 +0200, Eric Dumazet wrote: OK, here is a new clean patch that address this problem (nothing assumed about atomics) Would you just be able to add the atomic sysctl handler that Christoph suggested? This introduces lost update problems. 2 CPUs may store to

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
Nick Piggin a écrit : On Thu, 2005-08-25 at 12:41 +0200, Eric Dumazet wrote: OK, here is a new clean patch that address this problem (nothing assumed about atomics) Would you just be able to add the atomic sysctl handler that Christoph suggested? Quite a lot of work indeed, and it

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Nick Piggin
Eric Dumazet wrote: Nick Piggin a écrit : Would you just be able to add the atomic sysctl handler that Christoph suggested? Quite a lot of work indeed, and it would force to convert 3 int (nr_files, nr_free_files, max_files) to 3 atomic_t. I feel bad introducing a lot of sysctl rework

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Christoph Hellwig
On Fri, Aug 26, 2005 at 12:51:30AM +1000, Nick Piggin wrote: Eric Dumazet wrote: Nick Piggin a ?crit : Would you just be able to add the atomic sysctl handler that Christoph suggested? Quite a lot of work indeed, and it would force to convert 3 int (nr_files, nr_free_files,

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Eric Dumazet
Nick Piggin a écrit : OK, well I would prefer you do the proper atomic operations throughout where it really matters in file_table.c, and do your lazy synchronize with just the sysctl exported value. But... I got complains about atomic_read(counter) being 'an atomic op' (untrue), so my

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Dipankar Sarma
On Thu, Aug 25, 2005 at 05:13:05PM +0200, Eric Dumazet wrote: Nick Piggin a écrit : OK, well I would prefer you do the proper atomic operations throughout where it really matters in file_table.c, and do your lazy synchronize with just the sysctl exported value. But... I got complains

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Christoph Hellwig
On Thu, Aug 25, 2005 at 11:49:35PM +0530, Dipankar Sarma wrote: On Thu, Aug 25, 2005 at 05:13:05PM +0200, Eric Dumazet wrote: Nick Piggin a ?crit : OK, well I would prefer you do the proper atomic operations throughout where it really matters in file_table.c, and do your lazy synchronize

Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

2005-08-25 Thread Nick Piggin
Eric Dumazet wrote: Furthermore, a lazy sync would mean to change sysctl proc_handler for file-nr to perform a synchronize before calling proc_dointvec, this would be really obscure. I was only using your terminology (ie. the 'lazy' synch after the atomic is updated). Actually, a better