nanosleep used unconditionally in .../source3/modules/vfs_scannedonly.c Possible (but stupid) Patch: diff -u ./source3/modules/vfs_scannedonly.c.orig ./source3/modules/vfs_scannedonly.c --- ./source3/modules/vfs_scannedonly.c.orig 2010-05-17 06:51:23.000000000 -0500 +++ ./source3/modules/vfs_scannedonly.c 2010-05-27 09:27:33.000000000 -0500 @@ -476,7 +476,9 @@ "(max %d), %d ms) for %s\n", i, recheck_tries, recheck_time, cache_smb_fname->base_name)); +#ifdef HAVE_NANOSLEEP /* ? see also lib/util.c, function smb_msleep() */ nanosleep(&req, NULL); +#endif retval = SMB_VFS_NEXT_STAT(handle, cache_smb_fname); i++; } As mentioned above the real solution might be found in ...lib/util.c, function smb_msleep() Bye, Jojo
thanks! fixed in master 3.6 with ec643df212e521fc19119820b1e4fac15986bf28. I'll close this for now. Joachim: if you need this fixed in 3.5, please reopen this bug.
Created attachment 6029 [details] patch
Yes, I'd need it for 3.5.x too. For now I've come up with the following patch, not realy sure whether it'd work though? diff -u ./source3/modules/vfs_scannedonly.c.orig ./source3/modules/vfs_scannedonly.c --- ./source3/modules/vfs_scannedonly.c.orig 2010-10-07 11:41:16.000000000 -0500 +++ ./source3/modules/vfs_scannedonly.c 2010-10-23 08:08:46.000000000 -0500 @@ -476,13 +476,21 @@ flush_sendbuffer(handle); while (retval != 0 /*&& errno == ENOENT */ && i < recheck_tries) { +#ifdef HAVE_NANOSLEEP struct timespec req = { 0, recheck_time * 10000 }; +#endif DEBUG(SCANNEDONLY_DEBUG, ("scannedonly_allow_access, wait (try=%d " "(max %d), %d ms) for %s\n", i, recheck_tries, recheck_time, cache_smb_fname->base_name)); +#ifdef HAVE_NANOSLEEP nanosleep(&req, NULL); +#elif defined HAVE_USLEEP + usleep(recheck_time/1000); +#else /* fallback, see also lib/util.c, function smb_msleep() */ + smb_msleep(recheck_time/1000000); +#endif retval = SMB_VFS_NEXT_STAT(handle, cache_smb_fname); i++; }
See also Bug 7464, which has struct timespec at the wrong place. It may not be needed at all if we don't have nanosleep()?
Sorry this was nonsense, here a correction: diff -u ./source3/modules/vfs_scannedonly.c.orig ./source3/modules/vfs_scannedonly.c --- ./source3/modules/vfs_scannedonly.c.orig 2010-10-07 11:41:16.000000000 -0500 +++ ./source3/modules/vfs_scannedonly.c 2010-10-23 08:08:46.000000000 -0500 @@ -476,13 +476,21 @@ flush_sendbuffer(handle); while (retval != 0 /*&& errno == ENOENT */ && i < recheck_tries) { +#ifdef HAVE_NANOSLEEP struct timespec req = { 0, recheck_time * 10000 }; +#endif DEBUG(SCANNEDONLY_DEBUG, ("scannedonly_allow_access, wait (try=%d " "(max %d), %d ms) for %s\n", i, recheck_tries, recheck_time, cache_smb_fname->base_name)); +#ifdef HAVE_NANOSLEEP nanosleep(&req, NULL); +#elif defined HAVE_USLEEP + usleep(recheck_time*10); +#else /* fallback, see also lib/util.c, function smb_msleep() */ + smb_msleep(recheck_time/100); +#endif retval = SMB_VFS_NEXT_STAT(handle, cache_smb_fname); i++; }
Created attachment 6030 [details] patch
Why not the same patch as commit ec643df212e521fc19119820b1e4fac15986bf28 ? Adding ifdef's is really ugly.
(In reply to comment #7) > Why not the same patch as commit ec643df212e521fc19119820b1e4fac15986bf28 ? How does that look?
so: http://git.samba.org/samba.git/?p=samba.git;a=commitdiff;h=ec643df212e521fc19119820b1e4fac15986bf28 that cherry-picks cleanly to 3.5. metze: can you give a quit ACK for a git cherry-pick to 3.5 and reassign to Karolin then?
Looks, ok if it applies to 3.5.x Karolin please pick that one.
(In reply to comment #9) > so: > http://git.samba.org/samba.git/?p=samba.git;a=commitdiff;h=ec643df212e521fc19119820b1e4fac15986bf28 Ah, thanks. Nice and simple. I searched for this but couldn't find it. I assume it doesn't matter that this makes it sleep 2 orders of magnitudes longer? 'recheck_time*10000' nanoseconds would be 'recheck_time/100' miliseconds... But then again this might be 0 and not sleep at all.
the change of factor 100 was not really intended but seemed to be reasonable as the documentation just mentions ms values. Reading the loop around the smb_msleep call (and from the man page) it also looks like smb_msleep(recheck_time) is correct. I'm reopening this bug and reassign it to Oliver, the author of that module to confirm that ec643df212e introduced the correct sleep time here.
smb_msleep() sleeps milliseconds? I'm a bit puzzled by my own code, the recheck_time is supposed to me in milliseconds, but recheck_time*10000 doesn't make milliseconds from nanoseconds, there is a factor 100 missing. So in the code it has been milliseconds/100, or microseconds*10. anyway: the default values for recheck_time are 50 and 100 100/100 = 1 50/100 might become 0 .... perhaps a MAX(1, recheck_time/100) might be better. I'll do some tests to see if this has the desired result.
as you say, the documentation says milliseconds: "...will wait recheck_tries_open times for recheck_time_open milliseconds...". I think the documentation would match the current code here now. But you should know best what the desired result is. If there is anything that should be changed, can you submit a patch for it, please? Otherwise if you think it's right now, you can close this bug again :-)
the smb_usleep() works fine, but I've improved the behavior a bit. Would you like that attached to this bug report, or as a separate bugreport?
Created attachment 6067 [details] remove nanosleep and improve the whole wait loop behavior this patch probably only applies after https://bugzilla.samba.org/show_bug.cgi?id=7789 has been committed
Olivier: which branch is this patch for? In master and v3-5-test the nanosleep call is already eliminated and the sleep time is in units of ms there. So your patch doesn't apply. If the latest changes in master and v3-5-test look sane for you then please close this bug. Any further improvement should be put into another bug report or if it's just improvemnts that is not fixing bugs, you can send it directly to samba-techical for example.
The patches are in current branches