unix_convert() and amount of stat() calls

[Stefan (metze) Metzmacher]

Am 09.09.2010 01:09, schrieb Jeremy Allison:
> On Wed, Sep 08, 2010 at 02:58:10PM -0700, Jeremy Allison wrote:
>> On Tue, Sep 07, 2010 at 09:40:17PM +0200, Stefan (metze) Metzmacher wrote:
>>> Hi Jeremy,
>>>
>>> what's the reason that we loop over the components
>>> from the root to the leaf, when checking them?
>>>
>>> Can't we start with the last component?
>>> At least if it's related to a wildcard search.
>>>
>>> Maybe we only need to check the last 2 components,
>>> unless the name is mangled?
>>>
>>> The problem I want to solve in this branch:
>>> http://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/samba-3.4.7-ctdb-10
>>> is this:
>>>
>>> FindFirst with 'path\to\some\dir\with\files\*'
>>>
>>> triggers the following stat calls
>>>
>>> path\to\some\dir\with\files\* => ENOENT (this should be already solved
>>> with my patches)
>>> path\
>>> path\to\
>>> path\to\some\
>>> path\to\some\dir\
>>> path\to\some\dir\with\
>>> path\to\some\dir\with\files\
>>> path\to\some\dir\with\files\* => ENOENT
>>
>> Ok, I have a patch for this. Give me a min and I'll post it.
> 
> Ok - here's the patch that should fix it. It passes both
> RAW-CHKPATH and RAW-SEARCH. It adds an optimization into
> the wildcard lookup code such that it checks if the wildcard
> is in the last component of the lookup, and if so tries to
> directly do a stat on the parent directory. If this succeeds
> it short-circuits the pathname walk and starts the lookup at
> the last component.
> 
> This means an incoming pathname of :
> 
> FindFirst with 'path\to\some\dir\with\files\*'
> 
> should now do the following stat calls (presuming
> the directory exists of course):
> 
> path\to\some\dir\with\files\* => ENOENT
> path\to\some\dir\with\files = OK

You mean we only do this, correct?
path\to\some\dir\with\files = OK

At least that's what I want and what the code looks like.

> It doesn't currently add "path\to\some\dir\with\files"
> into the stat cache (although that could be added) as
> I'd have to alloc and truncate the incoming path at
> the same position as the parent directory length, which
> might be a little tricky. Anyway that's not neccessary
> for correctness.
> 
> I'm going to do some more stream testing, and then commit
> if it looks good. Metze (or Volker) please comment.

Thanks!

+			if (posix_pathnames) {
+				ret = SMB_VFS_LSTAT(conn, &parent_fname);
+			} else {
+				ret = SMB_VFS_STAT(conn, &parent_fname);
+			}
+			if (ret == 0) {
+				status = check_for_dot_component(&parent_fname);
+				if (!NT_STATUS_IS_OK(status)) {
+					goto fail;
+				}
+
+				start = smb_fname->base_name +
+					strlen(parent_fname.base_name);
+
+				DEBUG(5,("unix_convert optimize2: name "
+					"= %s, dirpath = %s, "
+					"start = %s\n",
+					smb_fname->base_name,
+					dirpath,
+					start));

Don't we need a 'goto done;' here?

+			}
+		}
 	}

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100909/535470df/attachment.pgp>