Messages in this thread | | | Date | Wed, 29 Apr 2009 08:15:58 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH][RFC] Handle improbable possibility of io_context->refcount overflow |
| |
On Wed, 29 Apr 2009 15:33:06 +0530 Nikanth Karthikesan <knikanth@novell.com> wrote:
> On Wednesday 29 April 2009 13:29:30 Andrew Morton wrote: > > On Wed, 29 Apr 2009 12:21:39 +0530 Nikanth Karthikesan <knikanth@novell.com> wrote: > > > Hi Jens > > > > > > Currently io_context has an atomic_t(int) as refcount. In case of cfq, > > > for each device a task does I/O, a reference to the io_context would be > > > taken. And when there are multiple process sharing io_contexts(CLONE_IO) > > > would also have a reference to the same io_context. Theoretically the > > > possible maximum number of processes sharing the same io_context + the > > > number of disks/cfq_data referring to the same io_context can overflow > > > the 32-bit counter on a very high-end machine. Even though it is an > > > improbable case, let us make it difficult by changing the refcount to > > > atomic64_t(long). > > > > Sorry, atomic64_t isn't implemented on 32 bit architectures. > > > > Perhaps it should be, but I expect it'd be pretty slow. > > Oh! Sorry, I didn't notice the #ifdef earlier. I guess thats why there is only > a single in-tree user for atomic64_t!
Yes, it's a bit irritating.
> In this case, could we make it atomic64_t only on 64-bit architectures and > keep it as atomic_t on 32-bit machines?
Sure.
> Something like the attached patch.
Check out atomic_long_t ;)
> I wonder whether we should also add BUG_ON's whenever the refcount is about to > wrap? Or try to handle it gracefully. Another approach would be to impose an > artificial limit on the no of tasks that could share an io_context. Or resort > to lock protection. The problem is not very serious/common. >
For a long time there was a debug patch in -mm which would warn if atomic_dec() ever took any atomic_t from zero to -1. I don't think it ever triggered false positives and it did find a couple of bugs.
I forget what happened to the patch - probably it died when the atomic code got altered.
It could well be that a similar kernel-wide check for atomic_inc() overflows would be similarly useful.
| |