Re: Inotify problem [was Re: 2.6.13-rc6-mm1]

[John McCutchan]

On Fri, 2005-08-26 at 13:03 -0400, Jim Houston wrote:
> Hi Everyone,
> 
> I'm answering this from my home email. I have not heard from my
> co-workers in Florida yet, and I imagine that they are busy cleaning up
> after hurricane Katrina and waiting for the power to come back on.
> 
> It looks like we have an "off by one" problem with idr_get_new_above()
> which may be part of the inotify problem.  I'm not sure if the problem
> is the behavior or the name & comments.  The start_id parameter is the
> starting point for the idr allocation search, and if it is available, it
> will be allocated.  If you pass in the last id allocated as the start_id
> and it has already been freed (by an idr_remove call), it will be
> allocated again.  The obvious fix would be to increment start_id
> in idr_get_new_above().

Thanks for your suggestion, it has fixed the inotify problem. But where
to put the fix is turning into a bit of a mess. Some callers like
drivers/md/dm.c:682 call idr_get_new_above as if it will return >=
starting_id. The comment says that it will return > starting_id, and the
function name leads people to believe the same thing. In the patch below
I change inotify do add one to the value was pass into idr. I also
change the comment to more accurately reflect what the function does.
The function name doesn't fit, but it never did.

Signed-off-by: John McCutchan <ttb@xxxxxxxxxxxxxxxx>

Index: linux/fs/inotify.c
===================================================================
--- linux.orig/fs/inotify.c	2005-08-26 13:38:29.000000000 -0400
+++ linux/fs/inotify.c	2005-08-26 13:38:55.000000000 -0400
@@ -353,7 +353,7 @@
 	do {
 		if (unlikely(!idr_pre_get(&dev->idr, GFP_KERNEL)))
 			return -ENOSPC;
-		ret = idr_get_new_above(&dev->idr, watch, dev->last_wd, &watch->wd);
+		ret = idr_get_new_above(&dev->idr, watch, dev->last_wd+1, &watch->wd);
 	} while (ret == -EAGAIN);
 
 	return ret;
Index: linux/lib/idr.c
===================================================================
--- linux.orig/lib/idr.c	2005-08-26 13:38:22.000000000 -0400
+++ linux/lib/idr.c	2005-08-26 13:39:08.000000000 -0400
@@ -207,7 +207,7 @@
 }
 
 /**
- * idr_get_new_above - allocate new idr entry above a start id
+ * idr_get_new_above - allocate new idr entry above or equal to a start id
  * @idp: idr handle
  * @ptr: pointer you want associated with the ide
  * @start_id: id to start search at

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