All of lore.kernel.org
 help / color / mirror / Atom feed
* Race condition in autofs 4.1.4
@ 2007-01-26 16:24 Lukas Kolbe
  2007-01-26 17:12 ` Ian Kent
  0 siblings, 1 reply; 23+ messages in thread
From: Lukas Kolbe @ 2007-01-26 16:24 UTC (permalink / raw)
  To: autofs

Hello!

We're currently in the process of migrating an old Solaris 2.8
mailserver to linux (debian etch). For testing purposes, we did the
following:

an nfsv4 fileserver serving the home directories
2 automount maps on the mailserver (don't ask ... ;))



auto.master is:
/nfs4homes      /etc/auto.mailhomes		-fstype=nfs4,-nosuid,grpid,proto=tcp,port=2049
/homes		/etc/auto.bind-mailhomes	-fstype=auto,bind


auto.mailhomes is:

musr1   10.0.0.1:/users/mail01/mail/&
musr2   10.0.0.1:/users/mail01/mail/&
musr3   10.0.0.1:/users/mail01/mail/&
[...]
musr1000   10.0.0.1:/users/mail01/mail/&


auto.bind-mailhomes is:

musr1   :/nfs4homes/&
musr2   :/nfs4homes/&
musr3   :/nfs4homes/&
[...]
musr1000   :/nfs4homes/&


This works: access to /homes/musr123 results in a successful nfs4-mount
of 10.0.0.1:/users/mail01/mail/musr123 to /nfs4homes/musr123 and a
successful bind-mount of /nfs4homes/musr123 to /homes/musr123, and mail
gets delivered properly.

The reason for this bind-mount thing is that we have in production a few
nfsv3 solaris fileservers, and the homedirectories for the users come
from different fileservers, all get mounted to /homes. The problem with
nfsv3 is that linux can't mount more than approximatly 100 shares at
once, so we plan to mount every fileservers /users to somewhere, so we
have all homes available, and let automount mount them to /homes/ per a
bind-mount.

We use a stress test on the machine, that sent about 150000 random sized
mails (up to 15k) to random users at random intervals, and under high
load, autofs seams to break here.

this is the current state of the /nfs4homes and /homes directories after
stopping the mail system and waiting for a few minutes (the timeout of
the automounter is set to 5 seconds, which triggers the bug after only a
few thousand mails delivered):


/homes/:
total 60
drwxr-xr-x 22 root    root        0 2007-01-26 16:40 .
drwxr-xr-x 24 root    root     4096 2007-01-26 16:35 ..
drwxr-xr-x  4 musr174 mailtest 4096 2007-01-17 17:28 musr174
drwxr-xr-x  4 musr253 mailtest 4096 2007-01-17 17:26 musr253
dr-xr-xr-x  2 root    root        0 2007-01-26 16:37 musr33
dr-xr-xr-x  2 root    root        0 2007-01-26 16:37 musr336
drwxr-xr-x  4 musr363 mailtest 4096 2007-01-18 15:28 musr363
drwxr-xr-x  4 musr403 mailtest 4096 2007-01-18 15:33 musr403
dr-xr-xr-x  2 root    root        0 2007-01-26 16:37 musr437
drwxr-xr-x  4 musr44  mailtest 4096 2007-01-18 14:01 musr44
drwxr-xr-x  4 musr46  mailtest 4096 2007-01-17 16:59 musr46
drwxr-xr-x  4 musr493 mailtest 4096 2007-01-22 15:50 musr493
dr-xr-xr-x  2 root    root        0 2007-01-26 16:36 musr549
drwxr-xr-x  4 musr602 mailtest 4096 2007-01-22 15:50 musr602
drwxr-xr-x  4 musr603 mailtest 4096 2007-01-22 15:51 musr603
dr-xr-xr-x  2 root    root        0 2007-01-26 16:37 musr646
dr-xr-xr-x  2 root    root        0 2007-01-26 16:37 musr657
drwxr-xr-x  4 musr662 mailtest 4096 2007-01-17 17:26 musr662
drwxr-xr-x  4 musr695 mailtest 4096 2007-01-18 15:28 musr695
drwxr-xr-x  4 musr860 mailtest 4096 2007-01-22 15:51 musr860
drwxr-xr-x  4 musr879 mailtest 4096 2007-01-18 15:14 musr879
drwxr-xr-x  4 musr918 mailtest 4096 2007-01-18 08:24 musr918

/nfs4homes:
total 60
drwxr-xr-x 24 root    root        0 2007-01-26 16:40 .
drwxr-xr-x 24 root    root     4096 2007-01-26 16:35 ..
dr-xr-xr-x  2 root    root        0 2007-01-26 16:37 musr117
drwxr-xr-x  4 musr200 mailtest 4096 2007-01-17 16:59 musr200
drwxr-xr-x  4 musr26  mailtest 4096 2007-01-18 08:24 musr26
drwxr-xr-x  4 musr311 mailtest 4096 2007-01-17 17:25 musr311
dr-xr-xr-x  2 root    root        0 2007-01-26 16:38 musr314
drwxr-xr-x  4 musr321 mailtest 4096 2007-01-17 17:28 musr321
dr-xr-xr-x  2 root    root        0 2007-01-26 16:37 musr33
drwxr-xr-x  4 musr363 mailtest 4096 2007-01-18 15:28 musr363
dr-xr-xr-x  2 root    root        0 2007-01-26 16:38 musr406
dr-xr-xr-x  2 root    root        0 2007-01-26 16:37 musr459
drwxr-xr-x  4 musr46  mailtest 4096 2007-01-17 16:59 musr46
drwxr-xr-x  4 musr489 mailtest 4096 2007-01-17 17:00 musr489
drwxr-xr-x  4 musr521 mailtest 4096 2007-01-22 15:54 musr521
drwxr-xr-x  4 musr532 mailtest 4096 2007-01-22 15:49 musr532
dr-xr-xr-x  2 root    root        0 2007-01-26 16:36 musr549
drwxr-xr-x  4 musr6   mailtest 4096 2007-01-17 17:00 musr6
dr-xr-xr-x  2 root    root        0 2007-01-26 16:39 musr70
drwxr-xr-x  4 musr819 mailtest 4096 2007-01-18 13:59 musr819
drwxr-xr-x  4 musr855 mailtest 4096 2007-01-22 15:50 musr855
dr-xr-xr-x  2 root    root        0 2007-01-26 16:38 musr946
drwxr-xr-x  4 musr948 mailtest 4096 2007-01-18 08:24 musr948
drwxr-xr-x  4 musr99  mailtest 4096 2007-01-18 15:36 musr99

You can clearly see that the directories that belong to root shouldn't
be there, at least they shouldn't be kept there.
autofs can't be restarted now - and it won't umount the remaining
correctly mounted directories. Only manual umounts of /homes/*
and /nfs4homes/* and a restart of the automounter afterwards fix this
problem for a short amount of time.

In syslog I get random messages like this:
Jan 26 16:39:29 demon automount[16757]: mount(generic): warning: /homes/musr657 is already mounted
Jan 26 16:39:42 demon automount[17062]: mount(generic): warning: /nfs4homes/musr946 is already mounted
for exactly the mountpoints that belong to root afterwards.

My guess is that the following happens:
1. postfix tries to deliver a mail
2. automounts mkdirs /nfs4homes/musr123 and mounts it
3. automount mkdirs /homes/musr123 and mounts it
4. mail gets delivered
...
5. the mounts get timeouts
6. automount umounts /homes/musr123
7. hey wait, postfix has another mail for musr123!
8. automount sees that /homes/musr123 is still mounted and does nothing
   (this is where the syslog message about the already mounted dir 
    possibly comes from)
9. the umount now finishes, leaving an empty /homes/musr123 behind,
   belonging to root:root
10. no rmdir /homes/musr123 happens?
11. the mail can't be delivered

and from here on, automount seems to work only half anymore: it still
mounts and umounts directories, but only new requests - the ones listed
above stay mounted. lsof shows that no process is using that directories
anymore ...

I hope this description was good enough for you to understand the
problem? I'd really like to help debugging this, but I'm not all that
familiar with strace and stuff. The mailserver is not in production, so
I'll be happy if I can penetrate it even more to find out the cause of
the problem :)

Thanks,
Lukas

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-26 16:24 Race condition in autofs 4.1.4 Lukas Kolbe
@ 2007-01-26 17:12 ` Ian Kent
  2007-01-26 18:10   ` Lukas Kolbe
  0 siblings, 1 reply; 23+ messages in thread
From: Ian Kent @ 2007-01-26 17:12 UTC (permalink / raw)
  To: Lukas Kolbe; +Cc: autofs

On Fri, 2007-01-26 at 17:24 +0100, Lukas Kolbe wrote:
> You can clearly see that the directories that belong to root shouldn't
> be there, at least they shouldn't be kept there.
> autofs can't be restarted now - and it won't umount the remaining
> correctly mounted directories. Only manual umounts of /homes/*
> and /nfs4homes/* and a restart of the automounter afterwards fix this
> problem for a short amount of time.
> 
> In syslog I get random messages like this:
> Jan 26 16:39:29 demon automount[16757]: mount(generic): warning: /homes/musr657 is already mounted
> Jan 26 16:39:42 demon automount[17062]: mount(generic): warning: /nfs4homes/musr946 is already mounted
> for exactly the mountpoints that belong to root afterwards.

Your mtab locking is broken.
Get a mount(8) that's not broken and then re-evaluate.

Ian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-26 17:12 ` Ian Kent
@ 2007-01-26 18:10   ` Lukas Kolbe
  2007-01-27  1:30     ` Ian Kent
  0 siblings, 1 reply; 23+ messages in thread
From: Lukas Kolbe @ 2007-01-26 18:10 UTC (permalink / raw)
  To: autofs

On Sa, 2007-01-27 at 02:12 +0900, Ian Kent wrote:

> > In syslog I get random messages like this:
> > Jan 26 16:39:29 demon automount[16757]: mount(generic): warning: /homes/musr657 is already mounted
> > Jan 26 16:39:42 demon automount[17062]: mount(generic): warning: /nfs4homes/musr946 is already mounted
> > for exactly the mountpoints that belong to root afterwards.
> 
> Your mtab locking is broken.
> Get a mount(8) that's not broken and then re-evaluate.

Thanks for the pointer. I found a discussion you were involved in in the
util-linux-ng mailinglist at the beginning of january, is that the
locking you talk about (see
http://www.mail-archive.com/util-linux-ng@vger.kernel.org/msg00005.html)?

I'll have a look at it on monday and see if I can patch our mount and if
it makes a difference!

> Ian

Kind regards,
Lukas

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-26 18:10   ` Lukas Kolbe
@ 2007-01-27  1:30     ` Ian Kent
  2007-01-27  2:18       ` Ian Kent
  2007-01-29 14:33       ` Lukas Kolbe
  0 siblings, 2 replies; 23+ messages in thread
From: Ian Kent @ 2007-01-27  1:30 UTC (permalink / raw)
  To: Lukas Kolbe; +Cc: autofs

On Fri, 2007-01-26 at 19:10 +0100, Lukas Kolbe wrote:
> On Sa, 2007-01-27 at 02:12 +0900, Ian Kent wrote:
> 
> > > In syslog I get random messages like this:
> > > Jan 26 16:39:29 demon automount[16757]: mount(generic): warning: /homes/musr657 is already mounted
> > > Jan 26 16:39:42 demon automount[17062]: mount(generic): warning: /nfs4homes/musr946 is already mounted
> > > for exactly the mountpoints that belong to root afterwards.
> > 
> > Your mtab locking is broken.
> > Get a mount(8) that's not broken and then re-evaluate.
> 
> Thanks for the pointer. I found a discussion you were involved in in the
> util-linux-ng mailinglist at the beginning of january, is that the
> locking you talk about (see
> http://www.mail-archive.com/util-linux-ng@vger.kernel.org/msg00005.html)?
> 
> I'll have a look at it on monday and see if I can patch our mount and if
> it makes a difference!

No and yes.

This issue has come up fairly often over the last 3-5 years.

What I expect when posting a comment like this is that the relevant
distribution maintainer speak up and justify that mount(8) in their
distribution has been patched to fix this issue.

That of course doesn't mean that it isn't still broken but indicates
that the right people are aware of the problem, have attempted to fix
it, and will help out if needed.

All distributions should have something included to fix this by now but
it seems that may not be the case.

It may be worthwhile looking at
http://www.kernel.org/pub/linux/daemons/autofs/v4/autofs-4.1.4-configureable-locking.patch.

Ian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-27  1:30     ` Ian Kent
@ 2007-01-27  2:18       ` Ian Kent
  2007-01-27 13:10         ` Jan Christoph Nordholz
  2007-01-29 14:33       ` Lukas Kolbe
  1 sibling, 1 reply; 23+ messages in thread
From: Ian Kent @ 2007-01-27  2:18 UTC (permalink / raw)
  To: Lukas Kolbe; +Cc: autofs

On Sat, 2007-01-27 at 10:30 +0900, Ian Kent wrote:
> On Fri, 2007-01-26 at 19:10 +0100, Lukas Kolbe wrote:
> > On Sa, 2007-01-27 at 02:12 +0900, Ian Kent wrote:
> > 
> > > > In syslog I get random messages like this:
> > > > Jan 26 16:39:29 demon automount[16757]: mount(generic): warning: /homes/musr657 is already mounted
> > > > Jan 26 16:39:42 demon automount[17062]: mount(generic): warning: /nfs4homes/musr946 is already mounted
> > > > for exactly the mountpoints that belong to root afterwards.
> > > 
> > > Your mtab locking is broken.
> > > Get a mount(8) that's not broken and then re-evaluate.
> > 
> > Thanks for the pointer. I found a discussion you were involved in in the
> > util-linux-ng mailinglist at the beginning of january, is that the
> > locking you talk about (see
> > http://www.mail-archive.com/util-linux-ng@vger.kernel.org/msg00005.html)?
> > 
> > I'll have a look at it on monday and see if I can patch our mount and if
> > it makes a difference!
> 
> No and yes.
> 
> This issue has come up fairly often over the last 3-5 years.
> 
> What I expect when posting a comment like this is that the relevant
> distribution maintainer speak up and justify that mount(8) in their
> distribution has been patched to fix this issue.
> 
> That of course doesn't mean that it isn't still broken but indicates
> that the right people are aware of the problem, have attempted to fix
> it, and will help out if needed.
> 
> All distributions should have something included to fix this by now but
> it seems that may not be the case.
> 
> It may be worthwhile looking at
> http://www.kernel.org/pub/linux/daemons/autofs/v4/autofs-4.1.4-configureable-locking.patch.
> 

Having checked this to the extent that I can, as I'm not very familiar
with Debian, it looks like the above patch has been included in Debian
testing but mount(8) locking in util-linux is broken in spite of the
"warnings" given "every time" this comes up.

Ian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-27  2:18       ` Ian Kent
@ 2007-01-27 13:10         ` Jan Christoph Nordholz
  2007-01-27 13:37           ` Lukas Kolbe
  2007-01-28  6:40           ` Ian Kent
  0 siblings, 2 replies; 23+ messages in thread
From: Jan Christoph Nordholz @ 2007-01-27 13:10 UTC (permalink / raw)
  To: autofs


[-- Attachment #1.1: Type: text/plain, Size: 415 bytes --]

Hi Ian and Lukas,

(stepping up as the new maintainer of autofs in Debian, taking over the
work of Steinar Gundersson)

I'm discussing the matter with the util-linux maintainer: the patch in
question question is included in Debian's autofs package, but not in
util-linux. I've only had time for a quick glance though, because I'll
leave for a week of vacation soon. I'll keep you informed.


Regards,

Jan Nordholz

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 140 bytes --]

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-27 13:10         ` Jan Christoph Nordholz
@ 2007-01-27 13:37           ` Lukas Kolbe
  2007-01-28  6:40           ` Ian Kent
  1 sibling, 0 replies; 23+ messages in thread
From: Lukas Kolbe @ 2007-01-27 13:37 UTC (permalink / raw)
  To: autofs

On Sa, 2007-01-27 at 14:10 +0100, Jan Christoph Nordholz wrote:
> Hi Ian and Lukas,

> I'm discussing the matter with the util-linux maintainer: the patch in
> question question is included in Debian's autofs package, but not in
> util-linux. I've only had time for a quick glance though, because I'll
> leave for a week of vacation soon. I'll keep you informed.


Danke :)
On Monday I'll try to patch utli-linux too and look if it makes a
difference.


> Regards,
> 
> Jan Nordholz

-- 
Lukas

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-27 13:10         ` Jan Christoph Nordholz
  2007-01-27 13:37           ` Lukas Kolbe
@ 2007-01-28  6:40           ` Ian Kent
  1 sibling, 0 replies; 23+ messages in thread
From: Ian Kent @ 2007-01-28  6:40 UTC (permalink / raw)
  To: Jan Christoph Nordholz; +Cc: autofs

On Sat, 2007-01-27 at 14:10 +0100, Jan Christoph Nordholz wrote:
> Hi Ian and Lukas,
> 
> (stepping up as the new maintainer of autofs in Debian, taking over the
> work of Steinar Gundersson)

Cool.

> 
> I'm discussing the matter with the util-linux maintainer: the patch in
> question question is included in Debian's autofs package, but not in
> util-linux. I've only had time for a quick glance though, because I'll
> leave for a week of vacation soon. I'll keep you informed.

And that patch is also in the patches directory of the 5.0.1-rc3
tarball. It is mainly to demonstrate the bug so people know what to
check. It did seem to improve things somewhat but is not a complete
solution but is all we have atm.

It's been quite a while since I looked at this so it's difficult to
remember, but ...

I think the main problem was in the function lock_fatab().

The close at the bottom of the loop was in the wrong position which was
causing the flock to be released frequently for all waiting process.
This caused the retry count to be used up quickly leading to an
additional waiter going ahead anyway, without the lock, along with the
lock holder which often resulted in corruption in /etc/mtab.

Ian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-27  1:30     ` Ian Kent
  2007-01-27  2:18       ` Ian Kent
@ 2007-01-29 14:33       ` Lukas Kolbe
  2007-01-29 15:19         ` Lukas Kolbe
  1 sibling, 1 reply; 23+ messages in thread
From: Lukas Kolbe @ 2007-01-29 14:33 UTC (permalink / raw)
  To: autofs

Am Samstag, den 27.01.2007, 10:30 +0900 schrieb Ian Kent:

> No and yes.
> 
> This issue has come up fairly often over the last 3-5 years.

> It may be worthwhile looking at
> http://www.kernel.org/pub/linux/daemons/autofs/v4/autofs-4.1.4-configureable-locking.patch.

That patch contains a typo (we_created_lock_file vs.
we_created_lockfile), but otherwise seems to work. I'm running another
stress test currently with the modified mount(8), starting with about
30000 mails in the queue of which 5000 are delivered now and the race
hasn't happend yet (I set the timemout of autofs to 5 seconds to trigger
that bug faster).

> Ian

-- 
Lukas

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-29 14:33       ` Lukas Kolbe
@ 2007-01-29 15:19         ` Lukas Kolbe
  2007-01-29 15:33           ` Lukas Kolbe
  2007-01-29 16:55           ` Ian Kent
  0 siblings, 2 replies; 23+ messages in thread
From: Lukas Kolbe @ 2007-01-29 15:19 UTC (permalink / raw)
  To: autofs

Am Montag, den 29.01.2007, 15:33 +0100 schrieb Lukas Kolbe:

> That patch contains a typo (we_created_lock_file vs.
> we_created_lockfile), but otherwise seems to work. I'm running another
> stress test currently with the modified mount(8), starting with about
> 30000 mails in the queue of which 5000 are delivered now and the race
> hasn't happend yet (I set the timemout of autofs to 5 seconds to trigger
> that bug faster).

The system is now through more than 25000 mails, and hasn't hit the bug
yet.
When it finished delivering, I'll setup another mailbomb and stress it
over the night, let's cross fingers that it works. I'll then file a bug
with the debian util-linux package and attach the patch to it.
Not close()ing the "we_created_lockfile" in unlock_mtab() seems like a
bug to me, anyway ;)

> > Ian

-- 
Lukas

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-29 15:19         ` Lukas Kolbe
@ 2007-01-29 15:33           ` Lukas Kolbe
  2007-02-06  2:04             ` Ian Kent
  2007-01-29 16:55           ` Ian Kent
  1 sibling, 1 reply; 23+ messages in thread
From: Lukas Kolbe @ 2007-01-29 15:33 UTC (permalink / raw)
  To: autofs

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

Am Montag, den 29.01.2007, 16:19 +0100 schrieb Lukas Kolbe:

> The system is now through more than 25000 mails, and hasn't hit the bug
> yet.
> When it finished delivering, I'll setup another mailbomb and stress it
> over the night, let's cross fingers that it works. I'll then file a bug
> with the debian util-linux package and attach the patch to it.
> Not close()ing the "we_created_lockfile" in unlock_mtab() seems like a
> bug to me, anyway ;)

Okay, the race hit again. Damnit!

Out of about 30000 mails, 8(!!!) didn't get delivered because:

Every 2.0s: ls -al /homes/ /nfs4homes | grep root                                                                                      Mon Jan 29 16:27:55 2007
drwxr-xr-x  7 root    root        0 2007-01-29 16:26 .
drwxr-xr-x 24 root    root     4096 2007-01-29 15:26 ..
dr-xr-xr-x  2 root    root        0 2007-01-29 16:11 musr352
dr-xr-xr-x  2 root    root        0 2007-01-29 16:17 musr847
drwxr-xr-x  8 root    root        0 2007-01-29 16:26 .
drwxr-xr-x 24 root    root     4096 2007-01-29 15:26 ..
dr-xr-xr-x  2 root    root        0 2007-01-29 16:17 musr847

These should look like
drwxr-xr-x  4 musr123 mailtest 4096 2007-01-17 16:59 musr123

And in /var/log/syslog appears again:
Jan 29 16:20:50 demon automount[29640]: mount(generic): warning: /homes/musr352 is already mounted

This was with debian's autofs 4.1.4-13, see 
ftp://ftp.de.debian.org/debian/pool/main/a/autofs/

And mount 2.12r-15.1, see ftp://ftp.de.debian.org/debian/pool/main/u/util-linux/
with the attached patch appliend, hence version number -15.1.

So, the patch made the problem appear way less, but it won't go away
completely. Ian, I guess that's what you suspected?

I'll try the test again overnight with an autofs timeout of 300.

-- 
Lukas


[-- Attachment #2: 70-mount-fstab-locking.dpatch --]
[-- Type: application/x-shellscript, Size: 1196 bytes --]

[-- Attachment #3: Type: text/plain, Size: 140 bytes --]

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-29 15:19         ` Lukas Kolbe
  2007-01-29 15:33           ` Lukas Kolbe
@ 2007-01-29 16:55           ` Ian Kent
  2007-01-29 20:56             ` Lukas Kolbe
  1 sibling, 1 reply; 23+ messages in thread
From: Ian Kent @ 2007-01-29 16:55 UTC (permalink / raw)
  To: Lukas Kolbe; +Cc: autofs

On Mon, 2007-01-29 at 16:19 +0100, Lukas Kolbe wrote:
> Am Montag, den 29.01.2007, 15:33 +0100 schrieb Lukas Kolbe:
> 
> > That patch contains a typo (we_created_lock_file vs.
> > we_created_lockfile), but otherwise seems to work. I'm running another
> > stress test currently with the modified mount(8), starting with about
> > 30000 mails in the queue of which 5000 are delivered now and the race
> > hasn't happend yet (I set the timemout of autofs to 5 seconds to trigger
> > that bug faster).
> 
> The system is now through more than 25000 mails, and hasn't hit the bug
> yet.
> When it finished delivering, I'll setup another mailbomb and stress it
> over the night, let's cross fingers that it works. I'll then file a bug
> with the debian util-linux package and attach the patch to it.
> Not close()ing the "we_created_lockfile" in unlock_mtab() seems like a
> bug to me, anyway ;)

Ya, but it gets closed at exit anyway.

Ian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-29 16:55           ` Ian Kent
@ 2007-01-29 20:56             ` Lukas Kolbe
  2007-01-29 21:09               ` Jeff Moyer
  2007-01-30  7:38               ` Ian Kent
  0 siblings, 2 replies; 23+ messages in thread
From: Lukas Kolbe @ 2007-01-29 20:56 UTC (permalink / raw)
  To: Ian Kent; +Cc: autofs

On Di, 2007-01-30 at 01:55 +0900, Ian Kent wrote:
> > The system is now through more than 25000 mails, and hasn't hit the bug
> > yet.
> > When it finished delivering, I'll setup another mailbomb and stress it
> > over the night, let's cross fingers that it works. I'll then file a bug
> > with the debian util-linux package and attach the patch to it.
> > Not close()ing the "we_created_lockfile" in unlock_mtab() seems like a
> > bug to me, anyway ;)
> 
> Ya, but it gets closed at exit anyway.

Ah, of course!

I sent a mail earlier today, but it didn't come through to the list,
dunno why, hence the Cc: on you.

The bug/race hit again when the system delivered about 95% of the mail,
with a few directories in /homes/ and /nfs4homes/ that belonged to root
and were empty, with an error in syslog that said "/homes/musrXXX
already mounted" (I don't have the exact messages here, can post them
tomorrow when I'm in the office).

This was with the patch minus the typo applied. It seems that the patch
helps the situation, as it happens not that fast, but once it happens,
the directories in question cannot be mounted anymore by the
automounter.

Are there any other patches for mount floating around that I can try? I
only have tomorrow for that because we're now going to implement our
fallback configuration - a patched amd that understands our hesiod maps
and just symlinks the home directories in place. But that feels rather
not very correct ... and I'd much like to see mount fixed, because that
locking code really scares me (I'm not really a C developer, though).

> Ian

-- 
Lukas

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-29 20:56             ` Lukas Kolbe
@ 2007-01-29 21:09               ` Jeff Moyer
  2007-01-29 21:21                 ` Lukas Kolbe
  2007-01-30  7:38               ` Ian Kent
  1 sibling, 1 reply; 23+ messages in thread
From: Jeff Moyer @ 2007-01-29 21:09 UTC (permalink / raw)
  To: Lukas Kolbe; +Cc: autofs, Ian Kent

==> On Mon, 29 Jan 2007 21:56:25 +0100, Lukas Kolbe <lukas@einfachkaffee.de> said:

Lukas> On Di, 2007-01-30 at 01:55 +0900, Ian Kent wrote: > > The
Lukas> system is now through more than 25000 mails, and hasn't hit the
Lukas> bug > > yet.  > > When it finished delivering, I'll setup
Lukas> another mailbomb and stress it > > over the night, let's cross
Lukas> fingers that it works. I'll then file a bug > > with the debian
Lukas> util-linux package and attach the patch to it.  > > Not
Lukas> close()ing the "we_created_lockfile" in unlock_mtab() seems
Lukas> like a > > bug to me, anyway ;)
Lukas> > 
Lukas> > Ya, but it gets closed at exit anyway.

Lukas> Ah, of course!

Lukas> I sent a mail earlier today, but it didn't come through to the
Lukas> list, dunno why, hence the Cc: on you.

Lukas> The bug/race hit again when the system delivered about 95% of
Lukas> the mail, with a few directories in /homes/ and /nfs4homes/
Lukas> that belonged to root and were empty, with an error in syslog
Lukas> that said "/homes/musrXXX already mounted" (I don't have the
Lukas> exact messages here, can post them tomorrow when I'm in the
Lukas> office).

Lukas> This was with the patch minus the typo applied. It seems that
Lukas> the patch helps the situation, as it happens not that fast, but
Lukas> once it happens, the directories in question cannot be mounted
Lukas> anymore by the automounter.

Lukas> Are there any other patches for mount floating around that I
Lukas> can try? I only have tomorrow for that because we're now going
Lukas> to implement our fallback configuration - a patched amd that
Lukas> understands our hesiod maps and just symlinks the home
Lukas> directories in place. But that feels rather not very correct
Lukas> ... and I'd much like to see mount fixed, because that locking
Lukas> code really scares me (I'm not really a C developer, though).

I don't mean to muddy the waters, but you might try adding the patch
below to autofs.  I posted it a while back, but Ian's been busy with
v5 development.  This is from my git tree, so I'm not sure if it
applies to current v4 as is.

-Jeff

commit 76b606ec00c3682971697b95c1af054742c704c5
Author: Jeff Moyer <jmoyer@segfault.boston.devel.redhat.com>
Date:   Wed Nov 8 22:04:44 2006 -0500

    Here is a patch to consult /proc/mounts instead of /etc/mtab when
    deciding if a path has already been mounted.  The problem with using
    /etc/mtab, as you know, is that it sometimes does not get updated.  We
    know that /proc/mounts will always be right, so why not use it?
    
    The patch will determine if /proc/mounts is available at run-time, and
    so will continue to work in the event that proc isn't available.

diff --git a/daemon/automount.c b/daemon/automount.c
index c09eaf6..766c472 100644
--- a/daemon/automount.c
+++ b/daemon/automount.c
@@ -333,7 +333,7 @@ static int umount_multi(const char *path
 
 	debug("umount_multi: path=%s incl=%d\n", path, incl);
 
-	mntlist = get_mnt_list(_PATH_MOUNTED, path, incl);
+	mntlist = get_mnt_list(path_mounted, path, incl);
 
 	if (!mntlist) {
 		warn("umount_multi: no mounts found under %s", path);
@@ -445,7 +445,7 @@ static int mount_autofs(char *path)
 	struct stat st;
 	int len;
 
-	if ((ap.state != ST_INIT) || is_mounted(_PATH_MOUNTED, path)) {
+	if ((ap.state != ST_INIT) || is_mounted(path_mounted, path)) {
 		/* This can happen if an autofs process is already running*/
 		error("mount_autofs: already mounted");
 		return -1;
@@ -1844,6 +1844,16 @@ int handle_mounts(char *path)
 	return 0;
 }
 
+void init_path_mounted(void)
+{
+	struct stat st;
+
+	if (stat(PROC_MOUNTS, &st) == 0)
+		path_mounted = PROC_MOUNTS;
+	else
+		path_mounted = _PATH_MOUNTED;
+}
+
 int main(int argc, char *argv[])
 {
 	char *path, *map, *mapfmt;
@@ -1928,6 +1938,8 @@ int main(int argc, char *argv[])
 	if (!dumpmap)
 		become_daemon();
 
+	init_path_mounted();
+
 	path = argv[0];
 	map = argv[1];
 	mapargv = (const char **) &argv[2];
diff --git a/daemon/spawn.c b/daemon/spawn.c
index 53887f9..b7e1511 100644
--- a/daemon/spawn.c
+++ b/daemon/spawn.c
@@ -119,7 +119,7 @@ void discard_pending(int sig)
  */
 int signal_children(int sig)
 {
-	struct mnt_list *mnts = get_mnt_list(_PATH_MOUNTED, "/", 0);
+	struct mnt_list *mnts = get_mnt_list(path_mounted, "/", 0);
 	struct mnt_list *next;
 	pid_t pgrp = getpgrp();
 	int ret = -1;
diff --git a/include/automount.h b/include/automount.h
index 14ec07b..c5f4707 100644
--- a/include/automount.h
+++ b/include/automount.h
@@ -40,6 +40,7 @@ #endif
 #define AUTOFS_SUPER_MAGIC 0x00000187L
 
 #define DEFAULT_TIMEOUT (5*60)			/* 5 minutes */
+#define PROC_MOUNTS	"/proc/mounts"
 #define AUTOFS_LOCK	"/var/lock/autofs"	/* To serialize access to mount */
 #define MOUNTED_LOCK	_PATH_MOUNTED "~"	/* mounts' lock file */
 #define MTAB_NOTUPDATED 0x1000			/* mtab succeded but not updated */
@@ -121,6 +122,7 @@ struct autofs_point {
 };
 
 extern struct autofs_point ap; 
+const char *path_mounted;
 
 /* Standard function used by daemon or modules */
 
diff --git a/lib/cache.c b/lib/cache.c
index 0f71fe2..c774dc0 100644
--- a/lib/cache.c
+++ b/lib/cache.c
@@ -268,7 +268,7 @@ int cache_delete(const char *root, const
 	if (!path)
 		return CHE_FAIL;
 
-	if (is_mounted(_PATH_MOUNTED, path)) {
+	if (is_mounted(path_mounted, path)) {
 		free(path);
 		return CHE_FAIL;
 	}
@@ -439,7 +439,7 @@ int cache_ghost(const char *root, int gh
 				break;
 
 			case LKP_MOUNT:
-				if (!is_mounted(_PATH_MOUNTED, gc.direct_base)) {
+				if (!is_mounted(path_mounted, gc.direct_base)) {
 					debug("cache_ghost: attempting to mount map, "
 					      "key %s",
 					      gc.direct_base);
diff --git a/lib/mounts.c b/lib/mounts.c
index 1e40fc1..1dacf14 100644
--- a/lib/mounts.c
+++ b/lib/mounts.c
@@ -294,7 +294,7 @@ int contained_in_local_fs(const char *pa
 	if (!path || !pathlen || pathlen > PATH_MAX)
 		return 0;
 
-	mnts = get_mnt_list(_PATH_MOUNTED, "/", 1);
+	mnts = get_mnt_list(path_mounted, "/", 1);
 	if (!mnts)
 		return 0;
 
@@ -309,13 +309,15 @@ int contained_in_local_fs(const char *pa
 			rv = statfs(this->path, &fs);
 			if (rv != -1 && fs.f_type == AUTOFS_SUPER_MAGIC)
 				ret = 1;
+			/* What is the below testing?  --JEM */
 			else if (this->fs_name[0] == '/') {
 				if (strlen(this->fs_name) > 1) {
 					if (this->fs_name[1] != '/')
 						ret = 1;
 				} else
 					ret = 1;
-			}
+			} else if (!strncmp(this->fs_name, "rootfs", 6))
+				ret = 1;
 			break;
 		}
 	}
@@ -357,7 +359,7 @@ int allow_owner_mount(const char *path)
 	struct mntent ent;
 	int ret = 0;
 
-	if (getuid() || is_mounted(_PATH_MOUNTED, path))
+	if (getuid() || is_mounted(path_mounted, path))
 		return 0;
 
 	if (find_mntent(_PATH_MNTTAB, path, &ent)) {
diff --git a/modules/mount_autofs.c b/modules/mount_autofs.c
index f0edb0a..08ae5f1 100644
--- a/modules/mount_autofs.c
+++ b/modules/mount_autofs.c
@@ -71,7 +71,7 @@ int mount_mount(const char *root, const 
 	debug(MODPREFIX "fullpath=%s what=%s options=%s",
 		  fullpath, what, options);
 
-	if (is_mounted(_PATH_MOUNTED, fullpath)) {
+	if (is_mounted(path_mounted, fullpath)) {
 		error(MODPREFIX 
 		 "warning: about to mount over %s, continuing", fullpath);
 		return 0;
diff --git a/modules/mount_bind.c b/modules/mount_bind.c
index 20ab7bf..fbf1546 100644
--- a/modules/mount_bind.c
+++ b/modules/mount_bind.c
@@ -121,7 +121,7 @@ int mount_mount(const char *root, const 
 		if (!status)
 			existed = 0;
 
-		if (is_mounted(_PATH_MOUNTED, fullpath)) {
+		if (is_mounted(path_mounted, fullpath)) {
 			error(MODPREFIX 
 			  "warning: %s is already mounted", fullpath);
 			return 0;
diff --git a/modules/mount_ext2.c b/modules/mount_ext2.c
index 38d67bf..b2efb96 100644
--- a/modules/mount_ext2.c
+++ b/modules/mount_ext2.c
@@ -69,7 +69,7 @@ int mount_mount(const char *root, const 
 	if (!status)
 		existed = 0;
 
-	if (is_mounted(_PATH_MOUNTED, fullpath)) {
+	if (is_mounted(path_mounted, fullpath)) {
 		error(MODPREFIX 
 		  "warning: %s is already mounted", fullpath);
 		return 0;
diff --git a/modules/mount_generic.c b/modules/mount_generic.c
index b3179d6..0fe6a3d 100644
--- a/modules/mount_generic.c
+++ b/modules/mount_generic.c
@@ -68,7 +68,7 @@ int mount_mount(const char *root, const 
 	if (!status)
 		existed = 0;
 
-	if (is_mounted(_PATH_MOUNTED, fullpath)) {
+	if (is_mounted(path_mounted, fullpath)) {
 		error(MODPREFIX "warning: %s is already mounted", fullpath);
 		return 0;
 	}
diff --git a/modules/mount_nfs.c b/modules/mount_nfs.c
index 3f9de50..c0546b9 100644
--- a/modules/mount_nfs.c
+++ b/modules/mount_nfs.c
@@ -596,7 +596,7 @@ #endif
 	if (!status)
 		existed = 0;
 
-	if (is_mounted(_PATH_MOUNTED, fullpath)) {
+	if (is_mounted(path_mounted, fullpath)) {
 		error(MODPREFIX 
 		      "warning: %s is already mounted", fullpath);
 		return 0;

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-29 21:09               ` Jeff Moyer
@ 2007-01-29 21:21                 ` Lukas Kolbe
  2007-01-29 21:35                   ` Paul Smith
  2007-01-30  7:20                   ` Ian Kent
  0 siblings, 2 replies; 23+ messages in thread
From: Lukas Kolbe @ 2007-01-29 21:21 UTC (permalink / raw)
  To: Jeff Moyer; +Cc: autofs, Ian Kent

On Mo, 2007-01-29 at 16:09 -0500, Jeff Moyer wrote:

> I don't mean to muddy the waters, but you might try adding the patch
> below to autofs.  I posted it a while back, but Ian's been busy with
> v5 development.  This is from my git tree, so I'm not sure if it
> applies to current v4 as is.

Thanks, I'll try it tomorrow. Seems reasonable to me.

> -Jeff

-- 
Lukas

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-29 21:21                 ` Lukas Kolbe
@ 2007-01-29 21:35                   ` Paul Smith
  2007-01-30  7:21                     ` Ian Kent
  2007-01-30  7:20                   ` Ian Kent
  1 sibling, 1 reply; 23+ messages in thread
From: Paul Smith @ 2007-01-29 21:35 UTC (permalink / raw)
  To: Lukas Kolbe; +Cc: autofs, Ian Kent

On Mon, 2007-01-29 at 22:21 +0100, Lukas Kolbe wrote:
> On Mo, 2007-01-29 at 16:09 -0500, Jeff Moyer wrote:
> 
> > I don't mean to muddy the waters, but you might try adding the patch
> > below to autofs.  I posted it a while back, but Ian's been busy with
> > v5 development.  This is from my git tree, so I'm not sure if it
> > applies to current v4 as is.
> 
> Thanks, I'll try it tomorrow. Seems reasonable to me.

Apropos of not much, on my embedded system using BusyBox I wanted to use
autofs, etc. but not install a separate mount and all that jazz; I just
made /etc/mtab a symlink to /proc/mounts and that works fine for me.

I don't do the kind of serious load via autofs/NFS you are talking
about, though; I mainly use it for handling core dumps (so they get
dumped to a central system instead of onto the embedded system).

-- 
-----------------------------------------------------------------------------
 Paul D. Smith <psmith@netezza.com>                       http://netezza.com
 "Please remain calm--I may be mad, but I am a professional."--Mad Scientist
-----------------------------------------------------------------------------
      These are my opinions--Netezza takes no responsibility for them.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-29 21:21                 ` Lukas Kolbe
  2007-01-29 21:35                   ` Paul Smith
@ 2007-01-30  7:20                   ` Ian Kent
  1 sibling, 0 replies; 23+ messages in thread
From: Ian Kent @ 2007-01-30  7:20 UTC (permalink / raw)
  To: Lukas Kolbe; +Cc: autofs

On Mon, 2007-01-29 at 22:21 +0100, Lukas Kolbe wrote:
> On Mo, 2007-01-29 at 16:09 -0500, Jeff Moyer wrote:
> 
> > I don't mean to muddy the waters, but you might try adding the patch
> > below to autofs.  I posted it a while back, but Ian's been busy with
> > v5 development.  This is from my git tree, so I'm not sure if it
> > applies to current v4 as is.
> 
> Thanks, I'll try it tomorrow. Seems reasonable to me.

Yep, Jeffs patch should work fine and you may not need to patch mount(8)
when using it.

Alas, while I can do that for v5 I'm concerned about the possible
performance impact it could have.

Ian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-29 21:35                   ` Paul Smith
@ 2007-01-30  7:21                     ` Ian Kent
  2007-01-31  4:46                       ` Lukas Kolbe
  0 siblings, 1 reply; 23+ messages in thread
From: Ian Kent @ 2007-01-30  7:21 UTC (permalink / raw)
  To: psmith; +Cc: autofs

On Mon, 2007-01-29 at 16:35 -0500, Paul Smith wrote:
> On Mon, 2007-01-29 at 22:21 +0100, Lukas Kolbe wrote:
> > On Mo, 2007-01-29 at 16:09 -0500, Jeff Moyer wrote:
> > 
> > > I don't mean to muddy the waters, but you might try adding the patch
> > > below to autofs.  I posted it a while back, but Ian's been busy with
> > > v5 development.  This is from my git tree, so I'm not sure if it
> > > applies to current v4 as is.
> > 
> > Thanks, I'll try it tomorrow. Seems reasonable to me.
> 
> Apropos of not much, on my embedded system using BusyBox I wanted to use
> autofs, etc. but not install a separate mount and all that jazz; I just
> made /etc/mtab a symlink to /proc/mounts and that works fine for me.
> 
> I don't do the kind of serious load via autofs/NFS you are talking
> about, though; I mainly use it for handling core dumps (so they get
> dumped to a central system instead of onto the embedded system).

Nevertheless that should also work.

Ian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-29 20:56             ` Lukas Kolbe
  2007-01-29 21:09               ` Jeff Moyer
@ 2007-01-30  7:38               ` Ian Kent
  1 sibling, 0 replies; 23+ messages in thread
From: Ian Kent @ 2007-01-30  7:38 UTC (permalink / raw)
  To: Lukas Kolbe; +Cc: autofs

On Mon, 2007-01-29 at 21:56 +0100, Lukas Kolbe wrote:
> On Di, 2007-01-30 at 01:55 +0900, Ian Kent wrote:
> > > The system is now through more than 25000 mails, and hasn't hit the bug
> > > yet.
> > > When it finished delivering, I'll setup another mailbomb and stress it
> > > over the night, let's cross fingers that it works. I'll then file a bug
> > > with the debian util-linux package and attach the patch to it.
> > > Not close()ing the "we_created_lockfile" in unlock_mtab() seems like a
> > > bug to me, anyway ;)
> > 
> > Ya, but it gets closed at exit anyway.
> 
> Ah, of course!
> 
> I sent a mail earlier today, but it didn't come through to the list,
> dunno why, hence the Cc: on you.
> 
> The bug/race hit again when the system delivered about 95% of the mail,
> with a few directories in /homes/ and /nfs4homes/ that belonged to root
> and were empty, with an error in syslog that said "/homes/musrXXX
> already mounted" (I don't have the exact messages here, can post them
> tomorrow when I'm in the office).

An interesting dilemma, either let the process continue and risk
corrupting mtab or not update the mtab for a mount that is already
succeeded or possibly wait forever on a stale mtab lock file.

Ian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-30  7:21                     ` Ian Kent
@ 2007-01-31  4:46                       ` Lukas Kolbe
  0 siblings, 0 replies; 23+ messages in thread
From: Lukas Kolbe @ 2007-01-31  4:46 UTC (permalink / raw)
  To: Ian Kent; +Cc: autofs

On Di, 2007-01-30 at 16:21 +0900, Ian Kent wrote:

> Nevertheless that should also work.

Sorry for being so quiet lately, I'm away until friday and give you an
update then.

> Ian

-- 
Lukas

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-01-29 15:33           ` Lukas Kolbe
@ 2007-02-06  2:04             ` Ian Kent
  2007-02-06  9:45               ` Jan Christoph Nordholz
  0 siblings, 1 reply; 23+ messages in thread
From: Ian Kent @ 2007-02-06  2:04 UTC (permalink / raw)
  To: Lukas Kolbe; +Cc: autofs

On Mon, 2007-01-29 at 16:33 +0100, Lukas Kolbe wrote:
> Am Montag, den 29.01.2007, 16:19 +0100 schrieb Lukas Kolbe:
> 
> > The system is now through more than 25000 mails, and hasn't hit the bug
> > yet.
> > When it finished delivering, I'll setup another mailbomb and stress it
> > over the night, let's cross fingers that it works. I'll then file a bug
> > with the debian util-linux package and attach the patch to it.
> > Not close()ing the "we_created_lockfile" in unlock_mtab() seems like a
> > bug to me, anyway ;)
> 
> Okay, the race hit again. Damnit!
> 
> Out of about 30000 mails, 8(!!!) didn't get delivered because:

Life is so hard, isn't it.

I've just remembered that there are a number of patches for 4.1.4
(mostly from Jeff Moyer) that I need to review and merge but I haven't
got to it get.

One of them changes autofs to use /proc/mounts instead of /etc/mtab
which may be more appealing than using the symlink approach.

We could try this patch but I don't have a Debian install to check if
the patch applies to it. It would be out of order as well so may be a
bit of work to merge. I'm also not sure what patches are included in the
Debian package. I could just make it apply to 4.1.4 on top of the
patches on kernel.org and see how it goes for you.

What do you think?

Another thing, would you be interested in running your test with rc3 of
autofs version 5 (add all the patches in the distribution directory)?
The main reason I'd like to try this out is that it still
uses /etc/mtab, mainly because of possible performance impact
using /proc/mounts with large direct mount maps. So, if you have the
same problem with it then I really need to find a way to
use /proc/mounts in v5.

Ian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-02-06  2:04             ` Ian Kent
@ 2007-02-06  9:45               ` Jan Christoph Nordholz
  2007-02-06 17:45                 ` Ian Kent
  0 siblings, 1 reply; 23+ messages in thread
From: Jan Christoph Nordholz @ 2007-02-06  9:45 UTC (permalink / raw)
  To: autofs


[-- Attachment #1.1: Type: text/plain, Size: 992 bytes --]

Hi Ian,

> I've just remembered that there are a number of patches for 4.1.4
> (mostly from Jeff Moyer) that I need to review and merge but I haven't
> got to it get.
> 
> One of them changes autofs to use /proc/mounts instead of /etc/mtab
> which may be more appealing than using the symlink approach.
> 
> We could try this patch but I don't have a Debian install to check if
> the patch applies to it. It would be out of order as well so may be a
> bit of work to merge. I'm also not sure what patches are included in the
> Debian package. I could just make it apply to 4.1.4 on top of the
> patches on kernel.org and see how it goes for you.

I could certainly lend a hand wrt testing its integration into Debian.
Is there a place where unofficial or unreviewed patches are kept, so I
could have a look? - Btw, how did Steinar usually forward patch suggestions
upstream? Is there a dedicated address or a subject convention to tag those
messages?


Regards,

Jan

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 140 bytes --]

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Race condition in autofs 4.1.4
  2007-02-06  9:45               ` Jan Christoph Nordholz
@ 2007-02-06 17:45                 ` Ian Kent
  0 siblings, 0 replies; 23+ messages in thread
From: Ian Kent @ 2007-02-06 17:45 UTC (permalink / raw)
  To: Jan Christoph Nordholz; +Cc: autofs

On Tue, 2007-02-06 at 10:45 +0100, Jan Christoph Nordholz wrote:
> Hi Ian,
> 
> > I've just remembered that there are a number of patches for 4.1.4
> > (mostly from Jeff Moyer) that I need to review and merge but I haven't
> > got to it get.
> > 
> > One of them changes autofs to use /proc/mounts instead of /etc/mtab
> > which may be more appealing than using the symlink approach.
> > 
> > We could try this patch but I don't have a Debian install to check if
> > the patch applies to it. It would be out of order as well so may be a
> > bit of work to merge. I'm also not sure what patches are included in the
> > Debian package. I could just make it apply to 4.1.4 on top of the
> > patches on kernel.org and see how it goes for you.
> 
> I could certainly lend a hand wrt testing its integration into Debian.
> Is there a place where unofficial or unreviewed patches are kept, so I
> could have a look? - Btw, how did Steinar usually forward patch suggestions
> upstream? Is there a dedicated address or a subject convention to tag those
> messages?

It all gets done on the list.
I don't have any place special for patches.

The patch in question here may not apply but try it anyway.
There were quite a few patches before this so there may be a dependency.

You should be able to clone the git repo for 4.1.4 -> 4.1.5 development
using:
cg-clone http://www.kernel.org/pub/scm/linux/storage/autofs/autofs.git#autofs-4_1_4 autofs-4.1.4
from which you could cg-diff the patches.

Anyway, most of them have passed through the list at some point.

Ian

---

Hi, Ian, list,

Here is a patch to consult /proc/mounts instead of /etc/mtab when
deciding if a path has already been mounted.  The problem with using
/etc/mtab, as you know, is that it sometimes does not get updated.  We
know that /proc/mounts will always be right, so why not use it?

The patch will determine if /proc/mounts is available at run-time, and
so will continue to work in the event that proc isn't available.

Comments welcome.

-Jeff

diff --git a/daemon/automount.c b/daemon/automount.c
index c09eaf6..766c472 100644
--- a/daemon/automount.c
+++ b/daemon/automount.c
@@ -333,7 +333,7 @@ static int umount_multi(const char *path
 
 	debug("umount_multi: path=%s incl=%d\n", path, incl);
 
-	mntlist = get_mnt_list(_PATH_MOUNTED, path, incl);
+	mntlist = get_mnt_list(path_mounted, path, incl);
 
 	if (!mntlist) {
 		warn("umount_multi: no mounts found under %s", path);
@@ -445,7 +445,7 @@ static int mount_autofs(char *path)
 	struct stat st;
 	int len;
 
-	if ((ap.state != ST_INIT) || is_mounted(_PATH_MOUNTED, path)) {
+	if ((ap.state != ST_INIT) || is_mounted(path_mounted, path)) {
 		/* This can happen if an autofs process is already running*/
 		error("mount_autofs: already mounted");
 		return -1;
@@ -1844,6 +1844,16 @@ int handle_mounts(char *path)
 	return 0;
 }
 
+void init_path_mounted(void)
+{
+	struct stat st;
+
+	if (stat(PROC_MOUNTS, &st) == 0)
+		path_mounted = PROC_MOUNTS;
+	else
+		path_mounted = _PATH_MOUNTED;
+}
+
 int main(int argc, char *argv[])
 {
 	char *path, *map, *mapfmt;
@@ -1928,6 +1938,8 @@ int main(int argc, char *argv[])
 	if (!dumpmap)
 		become_daemon();
 
+	init_path_mounted();
+
 	path = argv[0];
 	map = argv[1];
 	mapargv = (const char **) &argv[2];
diff --git a/daemon/spawn.c b/daemon/spawn.c
index 53887f9..b7e1511 100644
--- a/daemon/spawn.c
+++ b/daemon/spawn.c
@@ -119,7 +119,7 @@ void discard_pending(int sig)
  */
 int signal_children(int sig)
 {
-	struct mnt_list *mnts = get_mnt_list(_PATH_MOUNTED, "/", 0);
+	struct mnt_list *mnts = get_mnt_list(path_mounted, "/", 0);
 	struct mnt_list *next;
 	pid_t pgrp = getpgrp();
 	int ret = -1;
diff --git a/include/automount.h b/include/automount.h
index 14ec07b..c5f4707 100644
--- a/include/automount.h
+++ b/include/automount.h
@@ -40,6 +40,7 @@ #endif
 #define AUTOFS_SUPER_MAGIC 0x00000187L
 
 #define DEFAULT_TIMEOUT (5*60)			/* 5 minutes */
+#define PROC_MOUNTS	"/proc/mounts"
 #define AUTOFS_LOCK	"/var/lock/autofs"	/* To serialize access to mount */
 #define MOUNTED_LOCK	_PATH_MOUNTED "~"	/* mounts' lock file */
 #define MTAB_NOTUPDATED 0x1000			/* mtab succeded but not updated */
@@ -121,6 +122,7 @@ struct autofs_point {
 };
 
 extern struct autofs_point ap; 
+const char *path_mounted;
 
 /* Standard function used by daemon or modules */
 
diff --git a/lib/cache.c b/lib/cache.c
index 0f71fe2..c774dc0 100644
--- a/lib/cache.c
+++ b/lib/cache.c
@@ -268,7 +268,7 @@ int cache_delete(const char *root, const
 	if (!path)
 		return CHE_FAIL;
 
-	if (is_mounted(_PATH_MOUNTED, path)) {
+	if (is_mounted(path_mounted, path)) {
 		free(path);
 		return CHE_FAIL;
 	}
@@ -439,7 +439,7 @@ int cache_ghost(const char *root, int gh
 				break;
 
 			case LKP_MOUNT:
-				if (!is_mounted(_PATH_MOUNTED, gc.direct_base)) {
+				if (!is_mounted(path_mounted, gc.direct_base)) {
 					debug("cache_ghost: attempting to mount map, "
 					      "key %s",
 					      gc.direct_base);
diff --git a/lib/mounts.c b/lib/mounts.c
index 1e40fc1..1dacf14 100644
--- a/lib/mounts.c
+++ b/lib/mounts.c
@@ -294,7 +294,7 @@ int contained_in_local_fs(const char *pa
 	if (!path || !pathlen || pathlen > PATH_MAX)
 		return 0;
 
-	mnts = get_mnt_list(_PATH_MOUNTED, "/", 1);
+	mnts = get_mnt_list(path_mounted, "/", 1);
 	if (!mnts)
 		return 0;
 
@@ -309,13 +309,15 @@ int contained_in_local_fs(const char *pa
 			rv = statfs(this->path, &fs);
 			if (rv != -1 && fs.f_type == AUTOFS_SUPER_MAGIC)
 				ret = 1;
+			/* What is the below testing?  --JEM */
 			else if (this->fs_name[0] == '/') {
 				if (strlen(this->fs_name) > 1) {
 					if (this->fs_name[1] != '/')
 						ret = 1;
 				} else
 					ret = 1;
-			}
+			} else if (!strncmp(this->fs_name, "rootfs", 6))
+				ret = 1;
 			break;
 		}
 	}
@@ -357,7 +359,7 @@ int allow_owner_mount(const char *path)
 	struct mntent ent;
 	int ret = 0;
 
-	if (getuid() || is_mounted(_PATH_MOUNTED, path))
+	if (getuid() || is_mounted(path_mounted, path))
 		return 0;
 
 	if (find_mntent(_PATH_MNTTAB, path, &ent)) {
diff --git a/modules/mount_autofs.c b/modules/mount_autofs.c
index f0edb0a..08ae5f1 100644
--- a/modules/mount_autofs.c
+++ b/modules/mount_autofs.c
@@ -71,7 +71,7 @@ int mount_mount(const char *root, const 
 	debug(MODPREFIX "fullpath=%s what=%s options=%s",
 		  fullpath, what, options);
 
-	if (is_mounted(_PATH_MOUNTED, fullpath)) {
+	if (is_mounted(path_mounted, fullpath)) {
 		error(MODPREFIX 
 		 "warning: about to mount over %s, continuing", fullpath);
 		return 0;
diff --git a/modules/mount_bind.c b/modules/mount_bind.c
index 20ab7bf..fbf1546 100644
--- a/modules/mount_bind.c
+++ b/modules/mount_bind.c
@@ -121,7 +121,7 @@ int mount_mount(const char *root, const 
 		if (!status)
 			existed = 0;
 
-		if (is_mounted(_PATH_MOUNTED, fullpath)) {
+		if (is_mounted(path_mounted, fullpath)) {
 			error(MODPREFIX 
 			  "warning: %s is already mounted", fullpath);
 			return 0;
diff --git a/modules/mount_ext2.c b/modules/mount_ext2.c
index 38d67bf..b2efb96 100644
--- a/modules/mount_ext2.c
+++ b/modules/mount_ext2.c
@@ -69,7 +69,7 @@ int mount_mount(const char *root, const 
 	if (!status)
 		existed = 0;
 
-	if (is_mounted(_PATH_MOUNTED, fullpath)) {
+	if (is_mounted(path_mounted, fullpath)) {
 		error(MODPREFIX 
 		  "warning: %s is already mounted", fullpath);
 		return 0;
diff --git a/modules/mount_generic.c b/modules/mount_generic.c
index b3179d6..0fe6a3d 100644
--- a/modules/mount_generic.c
+++ b/modules/mount_generic.c
@@ -68,7 +68,7 @@ int mount_mount(const char *root, const 
 	if (!status)
 		existed = 0;
 
-	if (is_mounted(_PATH_MOUNTED, fullpath)) {
+	if (is_mounted(path_mounted, fullpath)) {
 		error(MODPREFIX "warning: %s is already mounted", fullpath);
 		return 0;
 	}
diff --git a/modules/mount_nfs.c b/modules/mount_nfs.c
index 3f9de50..c0546b9 100644
--- a/modules/mount_nfs.c
+++ b/modules/mount_nfs.c
@@ -596,7 +596,7 @@ #endif
 	if (!status)
 		existed = 0;
 
-	if (is_mounted(_PATH_MOUNTED, fullpath)) {
+	if (is_mounted(path_mounted, fullpath)) {
 		error(MODPREFIX 
 		      "warning: %s is already mounted", fullpath);
 		return 0;

^ permalink raw reply related	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2007-02-06 17:45 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-01-26 16:24 Race condition in autofs 4.1.4 Lukas Kolbe
2007-01-26 17:12 ` Ian Kent
2007-01-26 18:10   ` Lukas Kolbe
2007-01-27  1:30     ` Ian Kent
2007-01-27  2:18       ` Ian Kent
2007-01-27 13:10         ` Jan Christoph Nordholz
2007-01-27 13:37           ` Lukas Kolbe
2007-01-28  6:40           ` Ian Kent
2007-01-29 14:33       ` Lukas Kolbe
2007-01-29 15:19         ` Lukas Kolbe
2007-01-29 15:33           ` Lukas Kolbe
2007-02-06  2:04             ` Ian Kent
2007-02-06  9:45               ` Jan Christoph Nordholz
2007-02-06 17:45                 ` Ian Kent
2007-01-29 16:55           ` Ian Kent
2007-01-29 20:56             ` Lukas Kolbe
2007-01-29 21:09               ` Jeff Moyer
2007-01-29 21:21                 ` Lukas Kolbe
2007-01-29 21:35                   ` Paul Smith
2007-01-30  7:21                     ` Ian Kent
2007-01-31  4:46                       ` Lukas Kolbe
2007-01-30  7:20                   ` Ian Kent
2007-01-30  7:38               ` Ian Kent

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.