|
View:
New views
20 Messages
—
Rating Filter:
Alert me
|
| < Prev | 1 - 2 | Next > |
|
|
mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6Hi,
I made a simple test with cifs in linux-2.6.31-rc1 to see if the so called kmail-problem (cifs-user-homes are totally unusable for kmail-mail-cache) still remains. Than I ran into a strange problem using "sed -i <command> <file- on-cifs>". "sed" uses mkstemp libc-funktion and fails with EEXIST, writing therefore tons of files onto the cifs-share. You can reproduce it with: strace mktemp -p . abcXXXXXX 2>&1 | more giving stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) This is the same on 2.6.31-rc1-git6. This is not the case in 2.6.26-2-vserver-686 (debian). The Samba-Server is lenny with samba-enterprise: kmux-fs:/# dpkg -l | grep samba ii sernet-samba 3.3.6-24 a LanManager-like file and printer server fo ii sernet-samba-common 3.3.6-24 Samba common files used by both the server a ii sernet-samba-keyring 1.1 GnuPG archive keys of the SerNet Samba archi If I change Samba to the lenny-version ii samba 2:3.2.5-4lenny6 a LanManager-like file and printer server fo ii samba-common 2:3.2.5-4lenny6 Samba common files used by both the server a the described test-case is fine. -- Wilhelm _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Tue, 2009-06-30 at 13:00 +0200, Wilhelm Meier wrote:
> Hi, > > I made a simple test with cifs in linux-2.6.31-rc1 to see if the so called > kmail-problem (cifs-user-homes are totally unusable for kmail-mail-cache) > still remains. Than I ran into a strange problem using "sed -i <command> <file- > on-cifs>". "sed" uses mkstemp libc-funktion and fails with EEXIST, writing > therefore tons of files onto the cifs-share. > > You can reproduce it with: > > strace mktemp -p . abcXXXXXX 2>&1 | more > > giving > > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > > This is the same on 2.6.31-rc1-git6. > > This is not the case in 2.6.26-2-vserver-686 (debian). > > The Samba-Server is lenny with samba-enterprise: > kmux-fs:/# dpkg -l | grep samba > ii sernet-samba 3.3.6-24 a LanManager-like > file and printer server fo > ii sernet-samba-common 3.3.6-24 Samba common files > used by both the server a > ii sernet-samba-keyring 1.1 GnuPG archive > keys of the SerNet Samba archi > > If I change Samba to the lenny-version > > ii samba 2:3.2.5-4lenny6 a LanManager-like > file and printer server fo > ii samba-common 2:3.2.5-4lenny6 Samba common files > used by both the server a > > the described test-case is fine. > > I can reproduce this too. Looks like another regression from the create-on-lookup patches. I commented the relevant chunk out of cifs_lookup and that fixes the issue. Shirish, thoughts? -- Jeff Layton <jlayton@...> _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Tue, Jun 30, 2009 at 8:01 AM, Jeff Layton<jlayton@...> wrote:
> On Tue, 2009-06-30 at 13:00 +0200, Wilhelm Meier wrote: >> Hi, >> >> I made a simple test with cifs in linux-2.6.31-rc1 to see if the so called >> kmail-problem (cifs-user-homes are totally unusable for kmail-mail-cache) >> still remains. Than I ran into a strange problem using "sed -i <command> <file- >> on-cifs>". "sed" uses mkstemp libc-funktion and fails with EEXIST, writing >> therefore tons of files onto the cifs-share. >> >> You can reproduce it with: >> >> strace mktemp -p . abcXXXXXX 2>&1 | more >> >> giving >> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >> open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >> open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >> open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >> open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >> >> This is the same on 2.6.31-rc1-git6. >> >> This is not the case in 2.6.26-2-vserver-686 (debian). >> >> The Samba-Server is lenny with samba-enterprise: >> kmux-fs:/# dpkg -l | grep samba >> ii sernet-samba 3.3.6-24 a LanManager-like >> file and printer server fo >> ii sernet-samba-common 3.3.6-24 Samba common files >> used by both the server a >> ii sernet-samba-keyring 1.1 GnuPG archive >> keys of the SerNet Samba archi >> >> If I change Samba to the lenny-version >> >> ii samba 2:3.2.5-4lenny6 a LanManager-like >> file and printer server fo >> ii samba-common 2:3.2.5-4lenny6 Samba common files >> used by both the server a >> >> the described test-case is fine. >> >> > > I can reproduce this too. > > Looks like another regression from the create-on-lookup patches. I > commented the relevant chunk out of cifs_lookup and that fixes the > issue. > > Shirish, thoughts? > > -- > Jeff Layton <jlayton@...> > > Jeff, Wilhelm, I will spend some time later in the day on this but one thought came to mind, Jeff, we took out a line where if there was exclusive create, we would let vfs handle it, not sure if putting it back would help! Regards, Shirish _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Tue, 2009-06-30 at 08:27 -0500, Shirish Pargaonkar wrote:
> On Tue, Jun 30, 2009 at 8:01 AM, Jeff Layton<jlayton@...> wrote: > > On Tue, 2009-06-30 at 13:00 +0200, Wilhelm Meier wrote: > >> Hi, > >> > >> I made a simple test with cifs in linux-2.6.31-rc1 to see if the so called > >> kmail-problem (cifs-user-homes are totally unusable for kmail-mail-cache) > >> still remains. Than I ran into a strange problem using "sed -i <command> <file- > >> on-cifs>". "sed" uses mkstemp libc-funktion and fails with EEXIST, writing > >> therefore tons of files onto the cifs-share. > >> > >> You can reproduce it with: > >> > >> strace mktemp -p . abcXXXXXX 2>&1 | more > >> > >> giving > >> > >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > >> open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > >> open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > >> open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > >> open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > >> > >> This is the same on 2.6.31-rc1-git6. > >> > >> This is not the case in 2.6.26-2-vserver-686 (debian). > >> > >> The Samba-Server is lenny with samba-enterprise: > >> kmux-fs:/# dpkg -l | grep samba > >> ii sernet-samba 3.3.6-24 a LanManager-like > >> file and printer server fo > >> ii sernet-samba-common 3.3.6-24 Samba common files > >> used by both the server a > >> ii sernet-samba-keyring 1.1 GnuPG archive > >> keys of the SerNet Samba archi > >> > >> If I change Samba to the lenny-version > >> > >> ii samba 2:3.2.5-4lenny6 a LanManager-like > >> file and printer server fo > >> ii samba-common 2:3.2.5-4lenny6 Samba common files > >> used by both the server a > >> > >> the described test-case is fine. > >> > >> > > > > I can reproduce this too. > > > > Looks like another regression from the create-on-lookup patches. I > > commented the relevant chunk out of cifs_lookup and that fixes the > > issue. > > > > Shirish, thoughts? > > > > -- > > Jeff Layton <jlayton@...> > > > > > > Jeff, Wilhelm, > > I will spend some time later in the day on this but one thought came to mind, > Jeff, we took out a line where if there was exclusive create, we would let > vfs handle it, not sure if putting it back would help! > Probably...it does seem to be related to O_EXCL. Rather than just blindly putting that back though, I'd like to see some understanding about why this is failing, and a consistent idea about how this is code is intended to work. Skipping create on lookup for the O_EXCL case seems ass-backwards to me. Doing it on lookup should help atomicity, and that's just what you want for O_EXCL. -- Jeff Layton <jlayton@...> _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Tue, Jun 30, 2009 at 6:00 AM, Wilhelm Meier<wilhelm.meier@...> wrote:
> Hi, > > I made a simple test with cifs in linux-2.6.31-rc1 to see if the so called > kmail-problem (cifs-user-homes are totally unusable for kmail-mail-cache) > still remains. Than I ran into a strange problem using "sed -i <command> <file- > on-cifs>". "sed" uses mkstemp libc-funktion and fails with EEXIST, writing > therefore tons of files onto the cifs-share. > > You can reproduce it with: > > strace mktemp -p . abcXXXXXX 2>&1 | more > > giving > > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > > This is the same on 2.6.31-rc1-git6. > > This is not the case in 2.6.26-2-vserver-686 (debian). > > The Samba-Server is lenny with samba-enterprise: > kmux-fs:/# dpkg -l | grep samba > ii sernet-samba 3.3.6-24 a LanManager-like > file and printer server fo > ii sernet-samba-common 3.3.6-24 Samba common files > used by both the server a > ii sernet-samba-keyring 1.1 GnuPG archive > keys of the SerNet Samba archi > > If I change Samba to the lenny-version > > ii samba 2:3.2.5-4lenny6 a LanManager-like > file and printer server fo > ii samba-common 2:3.2.5-4lenny6 Samba common files > used by both the server a > > the described test-case is fine. > > > -- > Wilhelm > > _______________________________________________ > linux-cifs-client mailing list > linux-cifs-client@... > https://lists.samba.org/mailman/listinfo/linux-cifs-client > Wilhelm, How exactly can I recreate the problem? You mount a Samba share and create files and then run mktemp -p . <list_of_files> 2>&1 | more I have not dealt with mktemp command, will spend more time later in the afternoon. Need to address Jeff's comments/concerns about exclusive create too. Regards, Shirish _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6Am Dienstag, 30. Juni 2009 15:44:36 schrieben Sie:
> On Tue, Jun 30, 2009 at 6:00 AM, Wilhelm Meier<wilhelm.meier@...> wrote: > > Hi, > > > > I made a simple test with cifs in linux-2.6.31-rc1 to see if the so > > called kmail-problem (cifs-user-homes are totally unusable for > > kmail-mail-cache) still remains. Than I ran into a strange problem using > > "sed -i <command> <file- on-cifs>". "sed" uses mkstemp libc-funktion and > > fails with EEXIST, writing therefore tons of files onto the cifs-share. > > > > You can reproduce it with: > > > > strace mktemp -p . abcXXXXXX 2>&1 | more > > > > giving > > > > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > > open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File > > exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > > open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File > > exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > > open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File > > exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > > open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File > > exists) > > > > This is the same on 2.6.31-rc1-git6. > > > > This is not the case in 2.6.26-2-vserver-686 (debian). > > > > The Samba-Server is lenny with samba-enterprise: > > kmux-fs:/# dpkg -l | grep samba > > ii sernet-samba 3.3.6-24 a > > LanManager-like file and printer server fo > > ii sernet-samba-common 3.3.6-24 Samba common > > files used by both the server a > > ii sernet-samba-keyring 1.1 GnuPG > > archive keys of the SerNet Samba archi > > > > If I change Samba to the lenny-version > > > > ii samba 2:3.2.5-4lenny6 a > > LanManager-like file and printer server fo > > ii samba-common 2:3.2.5-4lenny6 Samba common > > files used by both the server a > > > > the described test-case is fine. > > > > > > -- > > Wilhelm > > > > _______________________________________________ > > linux-cifs-client mailing list > > linux-cifs-client@... > > https://lists.samba.org/mailman/listinfo/linux-cifs-client > > Wilhelm, > > How exactly can I recreate the problem? You mount a Samba share yes, versions see above > and > create files and then run > mktemp -p . <list_of_files> 2>&1 | more excatly, but keep the 6 'X', they are replaced by other chars to form an unique filename: mktemp -p . abcXXXXXX > > I have not dealt with mktemp command, will spend more time later in > the afternoon. look at mkstemp() libc-function > > Need to address Jeff's comments/concerns about exclusive create too. > > Regards, > > Shirish -- Wilhelm _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Tue, 2009-06-30 at 08:44 -0500, Shirish Pargaonkar wrote:
> On Tue, Jun 30, 2009 at 6:00 AM, Wilhelm Meier<wilhelm.meier@...> wrote: > > Hi, > > > > I made a simple test with cifs in linux-2.6.31-rc1 to see if the so called > > kmail-problem (cifs-user-homes are totally unusable for kmail-mail-cache) > > still remains. Than I ran into a strange problem using "sed -i <command> <file- > > on-cifs>". "sed" uses mkstemp libc-funktion and fails with EEXIST, writing > > therefore tons of files onto the cifs-share. > > > > You can reproduce it with: > > > > strace mktemp -p . abcXXXXXX 2>&1 | more > > > > giving > > > > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > > open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > > open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > > open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > > stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > > open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) > > > > This is the same on 2.6.31-rc1-git6. > > > > This is not the case in 2.6.26-2-vserver-686 (debian). > > > > The Samba-Server is lenny with samba-enterprise: > > kmux-fs:/# dpkg -l | grep samba > > ii sernet-samba 3.3.6-24 a LanManager-like > > file and printer server fo > > ii sernet-samba-common 3.3.6-24 Samba common files > > used by both the server a > > ii sernet-samba-keyring 1.1 GnuPG archive > > keys of the SerNet Samba archi > > > > If I change Samba to the lenny-version > > > > ii samba 2:3.2.5-4lenny6 a LanManager-like > > file and printer server fo > > ii samba-common 2:3.2.5-4lenny6 Samba common files > > used by both the server a > > > > the described test-case is fine. > > > > > > -- > > Wilhelm > > > > _______________________________________________ > > linux-cifs-client mailing list > > linux-cifs-client@... > > https://lists.samba.org/mailman/listinfo/linux-cifs-client > > > > Wilhelm, > > How exactly can I recreate the problem? You mount a Samba share and > create files and then run > mktemp -p . <list_of_files> 2>&1 | more > > I have not dealt with mktemp command, will spend more time later in > the afternoon. > > Need to address Jeff's comments/concerns about exclusive create too. > I was able to reproduce it by just running this while cd'ed into the share: $ strace mktemp -p . abcXXXXXX You'll see it go into a near infinite loop trying to create different filenames. Each one returns -EEXIST. I'm pretty sure the O_EXCL flag is related to the problem since mktemp uses that. -- Jeff Layton <jlayton@...> _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Tue, Jun 30, 2009 at 8:40 AM, Jeff Layton<jlayton@...> wrote:
> On Tue, 2009-06-30 at 08:27 -0500, Shirish Pargaonkar wrote: >> On Tue, Jun 30, 2009 at 8:01 AM, Jeff Layton<jlayton@...> wrote: >> > On Tue, 2009-06-30 at 13:00 +0200, Wilhelm Meier wrote: >> >> Hi, >> >> >> >> I made a simple test with cifs in linux-2.6.31-rc1 to see if the so called >> >> kmail-problem (cifs-user-homes are totally unusable for kmail-mail-cache) >> >> still remains. Than I ran into a strange problem using "sed -i <command> <file- >> >> on-cifs>". "sed" uses mkstemp libc-funktion and fails with EEXIST, writing >> >> therefore tons of files onto the cifs-share. >> >> >> >> You can reproduce it with: >> >> >> >> strace mktemp -p . abcXXXXXX 2>&1 | more >> >> >> >> giving >> >> >> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >> >> open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >> >> open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >> >> open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >> >> open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >> >> >> >> This is the same on 2.6.31-rc1-git6. >> >> >> >> This is not the case in 2.6.26-2-vserver-686 (debian). >> >> >> >> The Samba-Server is lenny with samba-enterprise: >> >> kmux-fs:/# dpkg -l | grep samba >> >> ii sernet-samba 3.3.6-24 a LanManager-like >> >> file and printer server fo >> >> ii sernet-samba-common 3.3.6-24 Samba common files >> >> used by both the server a >> >> ii sernet-samba-keyring 1.1 GnuPG archive >> >> keys of the SerNet Samba archi >> >> >> >> If I change Samba to the lenny-version >> >> >> >> ii samba 2:3.2.5-4lenny6 a LanManager-like >> >> file and printer server fo >> >> ii samba-common 2:3.2.5-4lenny6 Samba common files >> >> used by both the server a >> >> >> >> the described test-case is fine. >> >> >> >> >> > >> > I can reproduce this too. >> > >> > Looks like another regression from the create-on-lookup patches. I >> > commented the relevant chunk out of cifs_lookup and that fixes the >> > issue. >> > >> > Shirish, thoughts? >> > >> > -- >> > Jeff Layton <jlayton@...> >> > >> > >> >> Jeff, Wilhelm, >> >> I will spend some time later in the day on this but one thought came to mind, >> Jeff, we took out a line where if there was exclusive create, we would let >> vfs handle it, not sure if putting it back would help! >> > > Probably...it does seem to be related to O_EXCL. Rather than just > blindly putting that back though, I'd like to see some understanding > about why this is failing, and a consistent idea about how this is code > is intended to work. > > Skipping create on lookup for the O_EXCL case seems ass-backwards to me. > Doing it on lookup should help atomicity, and that's just what you want > for O_EXCL. > > -- > Jeff Layton <jlayton@...> > > So adding this back helps, this is what I had initially. The reason I did put this was because I was not sure how to handle exclusive create then when I added lookup intent code to cifs. It sounds correct that a Samba server would handle the case of exclusive create for cifs and return an appropriate code (EEXISTS) in the response, may be cifs is not handling it. This is what I get with this patch in # mktemp -p . abcXXXXXX ./abcWpeApL I am debugging further to identify what the exact problem is. diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 7dc6b74..d6f9ecd 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -673,6 +673,8 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry, if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) && (nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open && (nd->intent.open.flags & O_CREAT)) { + if (!((nd->intent.open.flags & O_CREAT) && + (nd->intent.open.flags & O_EXCL))) { rc = cifs_posix_open(full_path, &newInode, parent_dir_inode->i_sb, nd->intent.open.create_mode, @@ -689,6 +691,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry, posix_open = true; else if ((rc == -EINVAL) || (rc != -EOPNOTSUPP)) pTcon->broken_posix_open = true; + } } if (!posix_open) rc = cifs_get_inode_info_unix(&newInode, full_path, _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Tue, Jun 30, 2009 at 1:36 PM, Shirish
Pargaonkar<shirishpargaonkar@...> wrote: > On Tue, Jun 30, 2009 at 8:40 AM, Jeff Layton<jlayton@...> wrote: >> On Tue, 2009-06-30 at 08:27 -0500, Shirish Pargaonkar wrote: >>> On Tue, Jun 30, 2009 at 8:01 AM, Jeff Layton<jlayton@...> wrote: >>> > On Tue, 2009-06-30 at 13:00 +0200, Wilhelm Meier wrote: >>> >> Hi, >>> >> >>> >> I made a simple test with cifs in linux-2.6.31-rc1 to see if the so called >>> >> kmail-problem (cifs-user-homes are totally unusable for kmail-mail-cache) >>> >> still remains. Than I ran into a strange problem using "sed -i <command> <file- >>> >> on-cifs>". "sed" uses mkstemp libc-funktion and fails with EEXIST, writing >>> >> therefore tons of files onto the cifs-share. >>> >> >>> >> You can reproduce it with: >>> >> >>> >> strace mktemp -p . abcXXXXXX 2>&1 | more >>> >> >>> >> giving >>> >> >>> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >>> >> open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >>> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >>> >> open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >>> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >>> >> open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >>> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >>> >> open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >>> >> >>> >> This is the same on 2.6.31-rc1-git6. >>> >> >>> >> This is not the case in 2.6.26-2-vserver-686 (debian). >>> >> >>> >> The Samba-Server is lenny with samba-enterprise: >>> >> kmux-fs:/# dpkg -l | grep samba >>> >> ii sernet-samba 3.3.6-24 a LanManager-like >>> >> file and printer server fo >>> >> ii sernet-samba-common 3.3.6-24 Samba common files >>> >> used by both the server a >>> >> ii sernet-samba-keyring 1.1 GnuPG archive >>> >> keys of the SerNet Samba archi >>> >> >>> >> If I change Samba to the lenny-version >>> >> >>> >> ii samba 2:3.2.5-4lenny6 a LanManager-like >>> >> file and printer server fo >>> >> ii samba-common 2:3.2.5-4lenny6 Samba common files >>> >> used by both the server a >>> >> >>> >> the described test-case is fine. >>> >> >>> >> >>> > >>> > I can reproduce this too. >>> > >>> > Looks like another regression from the create-on-lookup patches. I >>> > commented the relevant chunk out of cifs_lookup and that fixes the >>> > issue. >>> > >>> > Shirish, thoughts? >>> > >>> > -- >>> > Jeff Layton <jlayton@...> >>> > >>> > >>> >>> Jeff, Wilhelm, >>> >>> I will spend some time later in the day on this but one thought came to mind, >>> Jeff, we took out a line where if there was exclusive create, we would let >>> vfs handle it, not sure if putting it back would help! >>> >> >> Probably...it does seem to be related to O_EXCL. Rather than just >> blindly putting that back though, I'd like to see some understanding >> about why this is failing, and a consistent idea about how this is code >> is intended to work. >> >> Skipping create on lookup for the O_EXCL case seems ass-backwards to me. >> Doing it on lookup should help atomicity, and that's just what you want >> for O_EXCL. >> >> -- >> Jeff Layton <jlayton@...> >> >> > > So adding this back helps, this is what I had initially. The reason I > did put this was because I > was not sure how to handle exclusive create then when I added lookup > intent code to cifs. > It sounds correct that a Samba server would handle the case of > exclusive create for cifs > and return an appropriate code (EEXISTS) in the response, may be cifs > is not handling it. > > This is what I get with this patch in > > # mktemp -p . abcXXXXXX > ./abcWpeApL > > I am debugging further to identify what the exact problem is. > > > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c > index 7dc6b74..d6f9ecd 100644 > --- a/fs/cifs/dir.c > +++ b/fs/cifs/dir.c > @@ -673,6 +673,8 @@ cifs_lookup(struct inode *parent_dir_inode, struct > dentry *direntry, > if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) && > (nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open && > (nd->intent.open.flags & O_CREAT)) { > + if (!((nd->intent.open.flags & O_CREAT) && > + (nd->intent.open.flags & O_EXCL))) { > rc = cifs_posix_open(full_path, &newInode, > parent_dir_inode->i_sb, > nd->intent.open.create_mode, > @@ -689,6 +691,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct > dentry *direntry, > posix_open = true; > else if ((rc == -EINVAL) || (rc != -EOPNOTSUPP)) > pTcon->broken_posix_open = true; > + } > } > if (!posix_open) > rc = cifs_get_inode_info_unix(&newInode, full_path, > I think the problem is, why EEXIST since file does not exist. The looping is I guess because mktemp then attempts to create another file and receives EEXIST and it goes on forever. So the basic issues is why EEXIST since mktemp is attempting a file which does not exist on the server. _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Tue, Jun 30, 2009 at 1:36 PM, Shirish
Pargaonkar<shirishpargaonkar@...> wrote: > On Tue, Jun 30, 2009 at 8:40 AM, Jeff Layton<jlayton@...> wrote: >> On Tue, 2009-06-30 at 08:27 -0500, Shirish Pargaonkar wrote: >>> On Tue, Jun 30, 2009 at 8:01 AM, Jeff Layton<jlayton@...> wrote: >>> > On Tue, 2009-06-30 at 13:00 +0200, Wilhelm Meier wrote: >>> >> Hi, >>> >> >>> >> I made a simple test with cifs in linux-2.6.31-rc1 to see if the so called >>> >> kmail-problem (cifs-user-homes are totally unusable for kmail-mail-cache) >>> >> still remains. Than I ran into a strange problem using "sed -i <command> <file- >>> >> on-cifs>". "sed" uses mkstemp libc-funktion and fails with EEXIST, writing >>> >> therefore tons of files onto the cifs-share. >>> >> >>> >> You can reproduce it with: >>> >> >>> >> strace mktemp -p . abcXXXXXX 2>&1 | more >>> >> >>> >> giving >>> >> >>> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >>> >> open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >>> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >>> >> open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >>> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >>> >> open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >>> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 >>> >> open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File exists) >>> >> >>> >> This is the same on 2.6.31-rc1-git6. >>> >> >>> >> This is not the case in 2.6.26-2-vserver-686 (debian). >>> >> >>> >> The Samba-Server is lenny with samba-enterprise: >>> >> kmux-fs:/# dpkg -l | grep samba >>> >> ii sernet-samba 3.3.6-24 a LanManager-like >>> >> file and printer server fo >>> >> ii sernet-samba-common 3.3.6-24 Samba common files >>> >> used by both the server a >>> >> ii sernet-samba-keyring 1.1 GnuPG archive >>> >> keys of the SerNet Samba archi >>> >> >>> >> If I change Samba to the lenny-version >>> >> >>> >> ii samba 2:3.2.5-4lenny6 a LanManager-like >>> >> file and printer server fo >>> >> ii samba-common 2:3.2.5-4lenny6 Samba common files >>> >> used by both the server a >>> >> >>> >> the described test-case is fine. >>> >> >>> >> >>> > >>> > I can reproduce this too. >>> > >>> > Looks like another regression from the create-on-lookup patches. I >>> > commented the relevant chunk out of cifs_lookup and that fixes the >>> > issue. >>> > >>> > Shirish, thoughts? >>> > >>> > -- >>> > Jeff Layton <jlayton@...> >>> > >>> > >>> >>> Jeff, Wilhelm, >>> >>> I will spend some time later in the day on this but one thought came to mind, >>> Jeff, we took out a line where if there was exclusive create, we would let >>> vfs handle it, not sure if putting it back would help! >>> >> >> Probably...it does seem to be related to O_EXCL. Rather than just >> blindly putting that back though, I'd like to see some understanding >> about why this is failing, and a consistent idea about how this is code >> is intended to work. >> >> Skipping create on lookup for the O_EXCL case seems ass-backwards to me. >> Doing it on lookup should help atomicity, and that's just what you want >> for O_EXCL. >> >> -- >> Jeff Layton <jlayton@...> >> >> > > So adding this back helps, this is what I had initially. The reason I > did put this was because I > was not sure how to handle exclusive create then when I added lookup > intent code to cifs. > It sounds correct that a Samba server would handle the case of > exclusive create for cifs > and return an appropriate code (EEXISTS) in the response, may be cifs > is not handling it. > > This is what I get with this patch in > > # mktemp -p . abcXXXXXX > ./abcWpeApL > > I am debugging further to identify what the exact problem is. > > > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c > index 7dc6b74..d6f9ecd 100644 > --- a/fs/cifs/dir.c > +++ b/fs/cifs/dir.c > @@ -673,6 +673,8 @@ cifs_lookup(struct inode *parent_dir_inode, struct > dentry *direntry, > if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) && > (nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open && > (nd->intent.open.flags & O_CREAT)) { > + if (!((nd->intent.open.flags & O_CREAT) && > + (nd->intent.open.flags & O_EXCL))) { > rc = cifs_posix_open(full_path, &newInode, > parent_dir_inode->i_sb, > nd->intent.open.create_mode, > @@ -689,6 +691,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct > dentry *direntry, > posix_open = true; > else if ((rc == -EINVAL) || (rc != -EOPNOTSUPP)) > pTcon->broken_posix_open = true; > + } > } > if (!posix_open) > rc = cifs_get_inode_info_unix(&newInode, full_path, > This is a problem at vfs level/interaction. I see posix open returning without any error and wireshark trace says the same, set_path_info with info level SMB_POSIX_OPEN succeeds. _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6Am Mittwoch, 1. Juli 2009 13:01:04 schrieb Shirish Pargaonkar:
> On Tue, Jun 30, 2009 at 1:36 PM, Shirish > > Pargaonkar<shirishpargaonkar@...> wrote: > > On Tue, Jun 30, 2009 at 8:40 AM, Jeff Layton<jlayton@...> wrote: > >> On Tue, 2009-06-30 at 08:27 -0500, Shirish Pargaonkar wrote: > >>> On Tue, Jun 30, 2009 at 8:01 AM, Jeff Layton<jlayton@...> wrote: > >>> > On Tue, 2009-06-30 at 13:00 +0200, Wilhelm Meier wrote: > >>> >> Hi, > >>> >> > >>> >> I made a simple test with cifs in linux-2.6.31-rc1 to see if the so > >>> >> called kmail-problem (cifs-user-homes are totally unusable for > >>> >> kmail-mail-cache) still remains. Than I ran into a strange problem > >>> >> using "sed -i <command> <file- on-cifs>". "sed" uses mkstemp > >>> >> libc-funktion and fails with EEXIST, writing therefore tons of files > >>> >> onto the cifs-share. > >>> >> > >>> >> You can reproduce it with: > >>> >> > >>> >> strace mktemp -p . abcXXXXXX 2>&1 | more > >>> >> > >>> >> giving > >>> >> > >>> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > >>> >> open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File > >>> >> exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > >>> >> open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File > >>> >> exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > >>> >> open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File > >>> >> exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0 > >>> >> open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File > >>> >> exists) > >>> >> > >>> >> This is the same on 2.6.31-rc1-git6. > >>> >> > >>> >> This is not the case in 2.6.26-2-vserver-686 (debian). > >>> >> > >>> >> The Samba-Server is lenny with samba-enterprise: > >>> >> kmux-fs:/# dpkg -l | grep samba > >>> >> ii sernet-samba 3.3.6-24 a > >>> >> LanManager-like file and printer server fo > >>> >> ii sernet-samba-common 3.3.6-24 Samba > >>> >> common files used by both the server a > >>> >> ii sernet-samba-keyring 1.1 GnuPG > >>> >> archive keys of the SerNet Samba archi > >>> >> > >>> >> If I change Samba to the lenny-version > >>> >> > >>> >> ii samba 2:3.2.5-4lenny6 a > >>> >> LanManager-like file and printer server fo > >>> >> ii samba-common 2:3.2.5-4lenny6 Samba > >>> >> common files used by both the server a > >>> >> > >>> >> the described test-case is fine. > >>> > > >>> > I can reproduce this too. > >>> > > >>> > Looks like another regression from the create-on-lookup patches. I > >>> > commented the relevant chunk out of cifs_lookup and that fixes the > >>> > issue. > >>> > > >>> > Shirish, thoughts? > >>> > > >>> > -- > >>> > Jeff Layton <jlayton@...> > >>> > >>> Jeff, Wilhelm, > >>> > >>> I will spend some time later in the day on this but one thought came to > >>> mind, Jeff, we took out a line where if there was exclusive create, we > >>> would let vfs handle it, not sure if putting it back would help! > >> > >> Probably...it does seem to be related to O_EXCL. Rather than just > >> blindly putting that back though, I'd like to see some understanding > >> about why this is failing, and a consistent idea about how this is code > >> is intended to work. > >> > >> Skipping create on lookup for the O_EXCL case seems ass-backwards to me. > >> Doing it on lookup should help atomicity, and that's just what you want > >> for O_EXCL. > >> > >> -- > >> Jeff Layton <jlayton@...> > > > > So adding this back helps, this is what I had initially. The reason I > > did put this was because I > > was not sure how to handle exclusive create then when I added lookup > > intent code to cifs. > > It sounds correct that a Samba server would handle the case of > > exclusive create for cifs > > and return an appropriate code (EEXISTS) in the response, may be cifs > > is not handling it. > > > > This is what I get with this patch in > > > > # mktemp -p . abcXXXXXX > > ./abcWpeApL > > > > I am debugging further to identify what the exact problem is. > > > > > > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c > > index 7dc6b74..d6f9ecd 100644 > > --- a/fs/cifs/dir.c > > +++ b/fs/cifs/dir.c > > @@ -673,6 +673,8 @@ cifs_lookup(struct inode *parent_dir_inode, struct > > dentry *direntry, > > if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) && > > (nd->flags & LOOKUP_OPEN) && > > !pTcon->broken_posix_open && (nd->intent.open.flags & O_CREAT)) { > > + if (!((nd->intent.open.flags & O_CREAT) && > > + (nd->intent.open.flags & O_EXCL))) { > > rc = cifs_posix_open(full_path, &newInode, > > parent_dir_inode->i_sb, > > nd->intent.open.create_mode, > > @@ -689,6 +691,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct > > dentry *direntry, > > posix_open = true; > > else if ((rc == -EINVAL) || (rc != -EOPNOTSUPP)) > > pTcon->broken_posix_open = true; > > + } > > } > > if (!posix_open) > > rc = cifs_get_inode_info_unix(&newInode, > > full_path, > > I think the problem is, why EEXIST since file does not exist. Exactly! But the file is actualy created on the server-side, so we end up with tons of files. > The > looping is I guess > because mktemp then attempts to create another file and receives EEXIST and > it goes on forever. You may look into libc: it will try 62^3 attempts (or all comibinations of char for the six 'X'). > So the basic issues is why EEXIST since mktemp is attempting a file > which does not > exist on the server. yes > _______________________________________________ > linux-cifs-client mailing list > linux-cifs-client@... > https://lists.samba.org/mailman/listinfo/linux-cifs-client -- Wilhelm _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Wed, 2009-07-01 at 06:01 -0500, Shirish Pargaonkar wrote:
> > I think the problem is, why EEXIST since file does not exist. The > looping is I guess > because mktemp then attempts to create another file and receives EEXIST and it > goes on forever. > So the basic issues is why EEXIST since mktemp is attempting a file > which does not > exist on the server. The problem is that the lookup is returning a positive dentry since it just created the file. This makes the VFS turn around and return -EEXIST (see the second O_EXCL check in do_filp_open). So we do have to skip the create on lookup for O_EXCL or it won't work. I think what's needed is something like this patch. It seems to fix the testcase for me. That said, this is not adequately regression tested and should not be committed until it is. On that note, I'd like to reiterate my earlier sentiment that all of this create-on-lookup code was committed prematurely and should be backed out until it's more fully baked. Just my USD$.02... -- Jeff Layton <jlayton@...> [0001-cifs-fix-regression-with-O_EXCL-creates-and-optimiz.patch] >From 7edbbd8dfa17b7865a25c97f3b586fefbf2a73b3 Mon Sep 17 00:00:00 2001 From: Jeff Layton <jlayton@...> Date: Wed, 1 Jul 2009 07:12:03 -0400 Subject: [PATCH] cifs: fix regression with O_EXCL creates and optimize away lookup Signed-off-by: Jeff Layton <jlayton@...> --- fs/cifs/dir.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 7dc6b74..0ac4859 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -643,6 +643,15 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry, } } + /* + * O_EXCL: optimize away the lookup, but don't hash the dentry. Let + * the VFS handle the create. + */ + if (nd->flags & LOOKUP_EXCL) { + d_instantiate(direntry, NULL); + return 0; + } + /* can not grab the rename sem here since it would deadlock in the cases (beginning of sys_rename itself) in which we already have the sb rename sem */ -- 1.6.0.6 _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Wed, Jul 1, 2009 at 6:21 AM, Jeff Layton<jlayton@...> wrote:
> On Wed, 2009-07-01 at 06:01 -0500, Shirish Pargaonkar wrote: >> >> I think the problem is, why EEXIST since file does not exist. The >> looping is I guess >> because mktemp then attempts to create another file and receives EEXIST and it >> goes on forever. >> So the basic issues is why EEXIST since mktemp is attempting a file >> which does not >> exist on the server. > > The problem is that the lookup is returning a positive dentry since it > just created the file. This makes the VFS turn around and return -EEXIST > (see the second O_EXCL check in do_filp_open). So we do have to skip the > create on lookup for O_EXCL or it won't work. > > I think what's needed is something like this patch. It seems to fix the > testcase for me. That said, this is not adequately regression tested and > should not be committed until it is. > > On that note, I'd like to reiterate my earlier sentiment that all of > this create-on-lookup code was committed prematurely and should be > backed out until it's more fully baked. > > Just my USD$.02... > > -- > Jeff Layton <jlayton@...> > meant to send this to everybody but ended up sending only to Jeff, so here it is again... I think that is why it is not possible to exclusive create within lookup intent code. So, IMHO, we should put the check back in the cifs_lookup. That is what NFS does, and cifs should do the same _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Wed, 2009-07-01 at 06:59 -0500, Shirish Pargaonkar wrote:
> On Wed, Jul 1, 2009 at 6:21 AM, Jeff Layton<jlayton@...> wrote: > > On Wed, 2009-07-01 at 06:01 -0500, Shirish Pargaonkar wrote: > >> > >> I think the problem is, why EEXIST since file does not exist. The > >> looping is I guess > >> because mktemp then attempts to create another file and receives EEXIST and it > >> goes on forever. > >> So the basic issues is why EEXIST since mktemp is attempting a file > >> which does not > >> exist on the server. > > > > The problem is that the lookup is returning a positive dentry since it > > just created the file. This makes the VFS turn around and return -EEXIST > > (see the second O_EXCL check in do_filp_open). So we do have to skip the > > create on lookup for O_EXCL or it won't work. > > > > I think what's needed is something like this patch. It seems to fix the > > testcase for me. That said, this is not adequately regression tested and > > should not be committed until it is. > > > > On that note, I'd like to reiterate my earlier sentiment that all of > > this create-on-lookup code was committed prematurely and should be > > backed out until it's more fully baked. > > > > Just my USD$.02... > > > > -- > > Jeff Layton <jlayton@...> > > > > meant to send this to everybody but ended up sending only to Jeff, > so here it is again... > > I think that is why it is not possible to exclusive create within > lookup intent code. > So, IMHO, we should put the check back in the cifs_lookup. > That is what NFS does, and cifs should do the same Right, but do we really need to do the lookup in that case? It seems like the old check didn't optimize it away (but maybe I'm remembering incorrectly). -- Jeff Layton <jlayton@...> _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Wed, Jul 1, 2009 at 7:13 AM, Jeff Layton<jlayton@...> wrote:
> On Wed, 2009-07-01 at 06:59 -0500, Shirish Pargaonkar wrote: >> On Wed, Jul 1, 2009 at 6:21 AM, Jeff Layton<jlayton@...> wrote: >> > On Wed, 2009-07-01 at 06:01 -0500, Shirish Pargaonkar wrote: >> >> >> >> I think the problem is, why EEXIST since file does not exist. The >> >> looping is I guess >> >> because mktemp then attempts to create another file and receives EEXIST and it >> >> goes on forever. >> >> So the basic issues is why EEXIST since mktemp is attempting a file >> >> which does not >> >> exist on the server. >> > >> > The problem is that the lookup is returning a positive dentry since it >> > just created the file. This makes the VFS turn around and return -EEXIST >> > (see the second O_EXCL check in do_filp_open). So we do have to skip the >> > create on lookup for O_EXCL or it won't work. >> > >> > I think what's needed is something like this patch. It seems to fix the >> > testcase for me. That said, this is not adequately regression tested and >> > should not be committed until it is. >> > >> > On that note, I'd like to reiterate my earlier sentiment that all of >> > this create-on-lookup code was committed prematurely and should be >> > backed out until it's more fully baked. >> > >> > Just my USD$.02... >> > >> > -- >> > Jeff Layton <jlayton@...> >> > >> >> meant to send this to everybody but ended up sending only to Jeff, >> so here it is again... >> >> I think that is why it is not possible to exclusive create within >> lookup intent code. >> So, IMHO, we should put the check back in the cifs_lookup. >> That is what NFS does, and cifs should do the same > > Right, but do we really need to do the lookup in that case? It seems > like the old check didn't optimize it away (but maybe I'm remembering > incorrectly). > > -- > Jeff Layton <jlayton@...> > > With the check, we are back to what was before lookup intent for exclusive create. I think for exclusive create, a lookup can result in not create if the file exists, so for a command like mktemp, it probably does not help, there will be a lookup and there will be a create. _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6Am Mittwoch, 1. Juli 2009 15:17:20 schrieb Shirish Pargaonkar:
> On Wed, Jul 1, 2009 at 7:13 AM, Jeff Layton<jlayton@...> wrote: > > On Wed, 2009-07-01 at 06:59 -0500, Shirish Pargaonkar wrote: > >> On Wed, Jul 1, 2009 at 6:21 AM, Jeff Layton<jlayton@...> wrote: > >> > On Wed, 2009-07-01 at 06:01 -0500, Shirish Pargaonkar wrote: > >> >> I think the problem is, why EEXIST since file does not exist. The > >> >> looping is I guess > >> >> because mktemp then attempts to create another file and receives > >> >> EEXIST and it goes on forever. > >> >> So the basic issues is why EEXIST since mktemp is attempting a file > >> >> which does not > >> >> exist on the server. > >> > > >> > The problem is that the lookup is returning a positive dentry since it > >> > just created the file. This makes the VFS turn around and return > >> > -EEXIST (see the second O_EXCL check in do_filp_open). So we do have > >> > to skip the create on lookup for O_EXCL or it won't work. > >> > > >> > I think what's needed is something like this patch. It seems to fix > >> > the testcase for me. That said, this is not adequately regression > >> > tested and should not be committed until it is. > >> > > >> > On that note, I'd like to reiterate my earlier sentiment that all of > >> > this create-on-lookup code was committed prematurely and should be > >> > backed out until it's more fully baked. > >> > > >> > Just my USD$.02... > >> > > >> > -- > >> > Jeff Layton <jlayton@...> > >> > >> meant to send this to everybody but ended up sending only to Jeff, > >> so here it is again... > >> > >> I think that is why it is not possible to exclusive create within > >> lookup intent code. > >> So, IMHO, we should put the check back in the cifs_lookup. > >> That is what NFS does, and cifs should do the same > > > > Right, but do we really need to do the lookup in that case? It seems > > like the old check didn't optimize it away (but maybe I'm remembering > > incorrectly). > > > > -- > > Jeff Layton <jlayton@...> > > With the check, we are back to what was before lookup intent for > exclusive create. I think for exclusive create, a lookup can result in > not create if the file exists, so for a command like mktemp, it > probably does not help, there will be a lookup and there will be a create. Well, I don't know nothing about the internals of cifs, but in this case lookup and create must be atomic together ... > _______________________________________________ > linux-cifs-client mailing list > linux-cifs-client@... > https://lists.samba.org/mailman/listinfo/linux-cifs-client -- Wilhelm _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Wed, 2009-07-01 at 08:17 -0500, Shirish Pargaonkar wrote:
> On Wed, Jul 1, 2009 at 7:13 AM, Jeff Layton<jlayton@...> wrote: > > On Wed, 2009-07-01 at 06:59 -0500, Shirish Pargaonkar wrote: > >> On Wed, Jul 1, 2009 at 6:21 AM, Jeff Layton<jlayton@...> wrote: > >> > On Wed, 2009-07-01 at 06:01 -0500, Shirish Pargaonkar wrote: > >> >> > >> >> I think the problem is, why EEXIST since file does not exist. The > >> >> looping is I guess > >> >> because mktemp then attempts to create another file and receives EEXIST and it > >> >> goes on forever. > >> >> So the basic issues is why EEXIST since mktemp is attempting a file > >> >> which does not > >> >> exist on the server. > >> > > >> > The problem is that the lookup is returning a positive dentry since it > >> > just created the file. This makes the VFS turn around and return -EEXIST > >> > (see the second O_EXCL check in do_filp_open). So we do have to skip the > >> > create on lookup for O_EXCL or it won't work. > >> > > >> > I think what's needed is something like this patch. It seems to fix the > >> > testcase for me. That said, this is not adequately regression tested and > >> > should not be committed until it is. > >> > > >> > On that note, I'd like to reiterate my earlier sentiment that all of > >> > this create-on-lookup code was committed prematurely and should be > >> > backed out until it's more fully baked. > >> > > >> > Just my USD$.02... > >> > > >> > -- > >> > Jeff Layton <jlayton@...> > >> > > >> > >> meant to send this to everybody but ended up sending only to Jeff, > >> so here it is again... > >> > >> I think that is why it is not possible to exclusive create within > >> lookup intent code. > >> So, IMHO, we should put the check back in the cifs_lookup. > >> That is what NFS does, and cifs should do the same > > > > Right, but do we really need to do the lookup in that case? It seems > > like the old check didn't optimize it away (but maybe I'm remembering > > incorrectly). > > > > -- > > Jeff Layton <jlayton@...> > > > > > > With the check, we are back to what was before lookup intent for > exclusive create. I think for exclusive create, a lookup can result in > not create if the file exists, so for a command like mktemp, it > probably does not help, there will be a lookup and there will be a create. I'm sorry, I don't understand what you're saying here. Can you rephrase it? Or better yet, maybe post the patch that you're proposing to fix this? -- Jeff Layton <jlayton@...> _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Wed, Jul 1, 2009 at 8:59 AM, Jeff Layton<jlayton@...> wrote:
> On Wed, 2009-07-01 at 08:17 -0500, Shirish Pargaonkar wrote: >> On Wed, Jul 1, 2009 at 7:13 AM, Jeff Layton<jlayton@...> wrote: >> > On Wed, 2009-07-01 at 06:59 -0500, Shirish Pargaonkar wrote: >> >> On Wed, Jul 1, 2009 at 6:21 AM, Jeff Layton<jlayton@...> wrote: >> >> > On Wed, 2009-07-01 at 06:01 -0500, Shirish Pargaonkar wrote: >> >> >> >> >> >> I think the problem is, why EEXIST since file does not exist. The >> >> >> looping is I guess >> >> >> because mktemp then attempts to create another file and receives EEXIST and it >> >> >> goes on forever. >> >> >> So the basic issues is why EEXIST since mktemp is attempting a file >> >> >> which does not >> >> >> exist on the server. >> >> > >> >> > The problem is that the lookup is returning a positive dentry since it >> >> > just created the file. This makes the VFS turn around and return -EEXIST >> >> > (see the second O_EXCL check in do_filp_open). So we do have to skip the >> >> > create on lookup for O_EXCL or it won't work. >> >> > >> >> > I think what's needed is something like this patch. It seems to fix the >> >> > testcase for me. That said, this is not adequately regression tested and >> >> > should not be committed until it is. >> >> > >> >> > On that note, I'd like to reiterate my earlier sentiment that all of >> >> > this create-on-lookup code was committed prematurely and should be >> >> > backed out until it's more fully baked. >> >> > >> >> > Just my USD$.02... >> >> > >> >> > -- >> >> > Jeff Layton <jlayton@...> >> >> > >> >> >> >> meant to send this to everybody but ended up sending only to Jeff, >> >> so here it is again... >> >> >> >> I think that is why it is not possible to exclusive create within >> >> lookup intent code. >> >> So, IMHO, we should put the check back in the cifs_lookup. >> >> That is what NFS does, and cifs should do the same >> > >> > Right, but do we really need to do the lookup in that case? It seems >> > like the old check didn't optimize it away (but maybe I'm remembering >> > incorrectly). >> > >> > -- >> > Jeff Layton <jlayton@...> >> > >> > >> >> With the check, we are back to what was before lookup intent for >> exclusive create. I think for exclusive create, a lookup can result in >> not create if the file exists, so for a command like mktemp, it >> probably does not help, there will be a lookup and there will be a create. > > I'm sorry, I don't understand what you're saying here. Can you rephrase > it? Or better yet, maybe post the patch that you're proposing to fix > this? > > -- > Jeff Layton <jlayton@...> > > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 7dc6b74..29d5642 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -673,22 +673,26 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry, if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) && (nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open && (nd->intent.open.flags & O_CREAT)) { - rc = cifs_posix_open(full_path, &newInode, + /* skip posix open if exclusive create */ + if (!((nd->intent.open.flags & O_CREAT) && + (nd->intent.open.flags & O_EXCL))) { + rc = cifs_posix_open(full_path, &newInode, parent_dir_inode->i_sb, nd->intent.open.create_mode, nd->intent.open.flags, &oplock, &fileHandle, xid); - /* - * The check below works around a bug in POSIX - * open in samba versions 3.3.1 and earlier where - * open could incorrectly fail with invalid parameter. - * If either that or op not supported returned, follow - * the normal lookup. - */ - if ((rc == 0) || (rc == -ENOENT)) - posix_open = true; - else if ((rc == -EINVAL) || (rc != -EOPNOTSUPP)) - pTcon->broken_posix_open = true; + /* + * The check below works around a bug in POSIX + * open in samba versions 3.3.1 and earlier + * where open could incorrectly fail with + * invalid parameter. If either that or op not + * supported returned, follow the normal lookup. + */ + if ((rc == 0) || (rc == -ENOENT)) + posix_open = true; + else if ((rc == -EINVAL) || (rc != -EOPNOTSUPP)) + pTcon->broken_posix_open = true; + } } if (!posix_open) rc = cifs_get_inode_info_unix(&newInode, full_path, _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Wed, Jul 1, 2009 at 8:44 AM, Wilhelm Meier<wilhelm.meier@...> wrote:
> Am Mittwoch, 1. Juli 2009 15:17:20 schrieb Shirish Pargaonkar: >> On Wed, Jul 1, 2009 at 7:13 AM, Jeff Layton<jlayton@...> wrote: >> > On Wed, 2009-07-01 at 06:59 -0500, Shirish Pargaonkar wrote: >> >> On Wed, Jul 1, 2009 at 6:21 AM, Jeff Layton<jlayton@...> wrote: >> >> > On Wed, 2009-07-01 at 06:01 -0500, Shirish Pargaonkar wrote: >> >> >> I think the problem is, why EEXIST since file does not exist. The >> >> >> looping is I guess >> >> >> because mktemp then attempts to create another file and receives >> >> >> EEXIST and it goes on forever. >> >> >> So the basic issues is why EEXIST since mktemp is attempting a file >> >> >> which does not >> >> >> exist on the server. >> >> > >> >> > The problem is that the lookup is returning a positive dentry since it >> >> > just created the file. This makes the VFS turn around and return >> >> > -EEXIST (see the second O_EXCL check in do_filp_open). So we do have >> >> > to skip the create on lookup for O_EXCL or it won't work. >> >> > >> >> > I think what's needed is something like this patch. It seems to fix >> >> > the testcase for me. That said, this is not adequately regression >> >> > tested and should not be committed until it is. >> >> > >> >> > On that note, I'd like to reiterate my earlier sentiment that all of >> >> > this create-on-lookup code was committed prematurely and should be >> >> > backed out until it's more fully baked. >> >> > >> >> > Just my USD$.02... >> >> > >> >> > -- >> >> > Jeff Layton <jlayton@...> >> >> >> >> meant to send this to everybody but ended up sending only to Jeff, >> >> so here it is again... >> >> >> >> I think that is why it is not possible to exclusive create within >> >> lookup intent code. >> >> So, IMHO, we should put the check back in the cifs_lookup. >> >> That is what NFS does, and cifs should do the same >> > >> > Right, but do we really need to do the lookup in that case? It seems >> > like the old check didn't optimize it away (but maybe I'm remembering >> > incorrectly). >> > >> > -- >> > Jeff Layton <jlayton@...> >> >> With the check, we are back to what was before lookup intent for >> exclusive create. I think for exclusive create, a lookup can result in >> not create if the file exists, so for a command like mktemp, it >> probably does not help, there will be a lookup and there will be a create. > > Well, I don't know nothing about the internals of cifs, but in this case > lookup and create must be atomic together ... > >> _______________________________________________ >> linux-cifs-client mailing list >> linux-cifs-client@... >> https://lists.samba.org/mailman/listinfo/linux-cifs-client > > -- > Wilhelm > Wilhelm, I do not know how to maintain atomicity in an operation like exclusive creates for network file systems, I mean, between a lookup and create from client, there is a chance that file gets created on the server. With cifs_posix_open in lookup, we can gurantee that atomicity but with the current vfs implementation, that is not possible (calling cifs_posix_open within cifs_lookup for atomic operations like exclusive create) on a Samba server but not on a Windows server. _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
|
|
Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6On Wed, 2009-07-01 at 09:31 -0500, Shirish Pargaonkar wrote:
> On Wed, Jul 1, 2009 at 8:59 AM, Jeff Layton<jlayton@...> wrote: > > On Wed, 2009-07-01 at 08:17 -0500, Shirish Pargaonkar wrote: > >> On Wed, Jul 1, 2009 at 7:13 AM, Jeff Layton<jlayton@...> wrote: > >> > On Wed, 2009-07-01 at 06:59 -0500, Shirish Pargaonkar wrote: > >> >> On Wed, Jul 1, 2009 at 6:21 AM, Jeff Layton<jlayton@...> wrote: > >> >> > On Wed, 2009-07-01 at 06:01 -0500, Shirish Pargaonkar wrote: > >> >> >> > >> >> >> I think the problem is, why EEXIST since file does not exist. The > >> >> >> looping is I guess > >> >> >> because mktemp then attempts to create another file and receives EEXIST and it > >> >> >> goes on forever. > >> >> >> So the basic issues is why EEXIST since mktemp is attempting a file > >> >> >> which does not > >> >> >> exist on the server. > >> >> > > >> >> > The problem is that the lookup is returning a positive dentry since it > >> >> > just created the file. This makes the VFS turn around and return -EEXIST > >> >> > (see the second O_EXCL check in do_filp_open). So we do have to skip the > >> >> > create on lookup for O_EXCL or it won't work. > >> >> > > >> >> > I think what's needed is something like this patch. It seems to fix the > >> >> > testcase for me. That said, this is not adequately regression tested and > >> >> > should not be committed until it is. > >> >> > > >> >> > On that note, I'd like to reiterate my earlier sentiment that all of > >> >> > this create-on-lookup code was committed prematurely and should be > >> >> > backed out until it's more fully baked. > >> >> > > >> >> > Just my USD$.02... > >> >> > > >> >> > -- > >> >> > Jeff Layton <jlayton@...> > >> >> > > >> >> > >> >> meant to send this to everybody but ended up sending only to Jeff, > >> >> so here it is again... > >> >> > >> >> I think that is why it is not possible to exclusive create within > >> >> lookup intent code. > >> >> So, IMHO, we should put the check back in the cifs_lookup. > >> >> That is what NFS does, and cifs should do the same > >> > > >> > Right, but do we really need to do the lookup in that case? It seems > >> > like the old check didn't optimize it away (but maybe I'm remembering > >> > incorrectly). > >> > > >> > -- > >> > Jeff Layton <jlayton@...> > >> > > >> > > >> > >> With the check, we are back to what was before lookup intent for > >> exclusive create. I think for exclusive create, a lookup can result in > >> not create if the file exists, so for a command like mktemp, it > >> probably does not help, there will be a lookup and there will be a create. > > > > I'm sorry, I don't understand what you're saying here. Can you rephrase > > it? Or better yet, maybe post the patch that you're proposing to fix > > this? > > > > -- > > Jeff Layton <jlayton@...> > > > > > > This is the patch, this is what we had it before in dir.c in cifs_lookup() > > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c > index 7dc6b74..29d5642 100644 > --- a/fs/cifs/dir.c > +++ b/fs/cifs/dir.c > @@ -673,22 +673,26 @@ cifs_lookup(struct inode *parent_dir_inode, > struct dentry *direntry, > if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) && > (nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open && > (nd->intent.open.flags & O_CREAT)) { > - rc = cifs_posix_open(full_path, &newInode, > + /* skip posix open if exclusive create */ > + if (!((nd->intent.open.flags & O_CREAT) && > + (nd->intent.open.flags & O_EXCL))) { > + rc = cifs_posix_open(full_path, &newInode, The problem here is that when O_EXCL is set, you're doing a lookup anyway. That breaks atomicity. It's quite possible that something like this occurs: Client 1 Client 2 ======== ========= LOOKUP DELETE CREATE ...it would be much better to skip the lookup on an O_EXCL create and simply attempt to create the file. The patch I sent does this in the same way that NFS does. It returns an unhashed, negative dentry on the lookup and lets the VFS attempt the create. That said, as you no doubt understand, this code is extremely delicate and we need to take extra care to make sure that new regressions are not introduced by any further patches. This is why I've been counseling from the beginning to remove this code pending more thorough testing. If this is worth doing at all, it's worth waiting and making sure that it's done right. -- Jeff Layton <jlayton@...> _______________________________________________ linux-cifs-client mailing list linux-cifs-client@... https://lists.samba.org/mailman/listinfo/linux-cifs-client |
| < Prev | 1 - 2 | Next > |
| Free embeddable forum powered by Nabble | Forum Help |