lslocks – "failed to parse pid: 'WRITE'"

lslocks – "failed to parse pid: 'WRITE'"

[Mantas M.]

When one process is holding an exclusive lock on a file, and other
processes are waiting for the lock to be released, the contents of
/proc/locks look like this:

$ cat /proc/locks
1: FLOCK  ADVISORY  WRITE 431143 00:0f:6325223 0 EOF
1: -> FLOCK  ADVISORY  WRITE 405209 00:0f:6325223 0 EOF
1: -> FLOCK  ADVISORY  WRITE 399578 00:0f:6325223 0 EOF
1: -> FLOCK  ADVISORY  WRITE 434120 00:0f:6325223 0 EOF
2: FLOCK  ADVISORY  WRITE 359765 08:04:28180839 0 EOF
3: FLOCK  ADVISORY  WRITE 359765 08:04:28180837 0 EOF
4: POSIX  ADVISORY  WRITE 434729 08:04:8653261 0 EOF
4: -> POSIX  ADVISORY  WRITE 434737 08:04:8653261 0 EOF
5: FLOCK  ADVISORY  WRITE 359765 08:04:28180836 0 EOF
6: ...

If I try to run `lslocks`, it fails to parse the "->" lines and exits
with an error message:

    lslocks: failed to parse pid: 'WRITE'

Can be reproduced by:

touch foo & flock foo -c "sleep 100" & flock foo -c "sleep 100" &
touch foo & lckdo -w foo sleep 100 & lckdo -w foo sleep 100 &

-- 
Mantas Mikulėnas <grawity@gmail.com>

Re: lslocks – "failed to parse pid: 'WRITE'"

[Sami Kerola]

On Thu, Feb 7, 2013 at 2:42 PM, Mantas M. <grawity@gmail.com> wrote:
> When one process is holding an exclusive lock on a file, and other
> processes are waiting for the lock to be released, the contents of
> /proc/locks look like this:
>
> $ cat /proc/locks
> 1: FLOCK  ADVISORY  WRITE 431143 00:0f:6325223 0 EOF
> 1: -> FLOCK  ADVISORY  WRITE 405209 00:0f:6325223 0 EOF
> 1: -> FLOCK  ADVISORY  WRITE 399578 00:0f:6325223 0 EOF
> 1: -> FLOCK  ADVISORY  WRITE 434120 00:0f:6325223 0 EOF
> 2: FLOCK  ADVISORY  WRITE 359765 08:04:28180839 0 EOF
> 3: FLOCK  ADVISORY  WRITE 359765 08:04:28180837 0 EOF
> 4: POSIX  ADVISORY  WRITE 434729 08:04:8653261 0 EOF
> 4: -> POSIX  ADVISORY  WRITE 434737 08:04:8653261 0 EOF
> 5: FLOCK  ADVISORY  WRITE 359765 08:04:28180836 0 EOF
> 6: ...
>
> If I try to run `lslocks`, it fails to parse the "->" lines and exits
> with an error message:
>
>     lslocks: failed to parse pid: 'WRITE'
>
> Can be reproduced by:
>
> touch foo & flock foo -c "sleep 100" & flock foo -c "sleep 100" &
> touch foo & lckdo -w foo sleep 100 & lckdo -w foo sleep 100 &

Hi Mantas,

Thank you for reproducing bit which made creating some sort of fix
quite easy. Karel, and others, what do you think about adding a mode
to output listing which some might say is not a mode at all?


>From 9a71631ffd9181225323747eea971e2cd0d80eb8 Mon Sep 17 00:00:00 2001
From: Sami Kerola <kerolasa@iki.fi>
Date: Mon, 11 Feb 2013 22:30:49 +0000
Subject: [PATCH] lslocks: print waiting state when necessary
Organization: Lastminute.com

Earlier output was primarily unexpected.

$ flock foo -c "sleep 100" & flock foo -c "sleep 100" & flock foo -c
"sleep 100" & lslocks
[1] 22352
[2] 22353
[3] 22354
lslocks: failed to parse pid: 'WRITE'

This commit adds WAITING mode to output listing.  The locks which failed
with earlier version will look the following.

COMMAND           PID  TYPE SIZE MODE    M      START        END PATH
[...]
flock           22354 FLOCK   0B WAITING 0          0          0
/home/src/util-linux/foo
flock           22352 FLOCK   0B WAITING 0          0          0
/home/src/util-linux/foo
flock           22353 FLOCK   0B WRITE   0          0          0
/home/src/util-linux/foo

Reported-by: Mantas Mikulenas <grawity@gmail.com>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/lslocks.8 |  2 +-
 misc-utils/lslocks.c | 13 +++++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/misc-utils/lslocks.8 b/misc-utils/lslocks.8
index 24cda14..34f611c 100644
--- a/misc-utils/lslocks.8
+++ b/misc-utils/lslocks.8
@@ -44,7 +44,7 @@ Type of lock, can be FLOCK (created with flock(2))
or POSIX (created with fcntl(
 Size of the locked file.

 .IP "MODE"
-Lock access permissions (read, write).
+Lock access permissions (read, write, waiting).

 .IP "M"
 Mandatory state of the lock: 0 if none; 1 if set.  (See chmod(1)).
diff --git a/misc-utils/lslocks.c b/misc-utils/lslocks.c
index 0a82129..1e8f51f 100644
--- a/misc-utils/lslocks.c
+++ b/misc-utils/lslocks.c
@@ -226,7 +226,7 @@ static ino_t get_dev_inode(char *str, dev_t *dev)

 static int get_local_locks(struct list_head *locks)
 {
-	int i;
+	int i, waiting;
 	ino_t inode = 0;
 	FILE *fp;
 	char buf[PATH_MAX], *szstr = NULL, *tok = NULL;
@@ -241,6 +241,7 @@ static int get_local_locks(struct list_head *locks)

 		l = xcalloc(1, sizeof(*l));
 		INIT_LIST_HEAD(&l->locks);
+		waiting = 0;

 		for (tok = strtok(buf, " "), i = 0; tok;
 		     tok = strtok(NULL, " "), i++) {
@@ -253,6 +254,11 @@ static int get_local_locks(struct list_head *locks)
 			case 0: /* ignore */
 				break;
 			case 1: /* posix, flock, etc */
+				if (strcmp(tok, "->") == 0) {
+					i--;
+					waiting = 1;
+					continue;
+				}
 				l->type = xstrdup(tok);
 				break;

@@ -260,7 +266,10 @@ static int get_local_locks(struct list_head *locks)
 				l->mandatory = *tok == 'M' ? TRUE : FALSE;
 				break;
 			case 3: /* lock mode */
-				l->mode = xstrdup(tok);
+				if (waiting)
+					l->mode = xstrdup("WAITING");
+				else
+					l->mode = xstrdup(tok);
 				break;

 			case 4: /* PID */
-- 
1.8.1.3

Re: lslocks – "failed to parse pid: 'WRITE'"

[Mantas Mikulėnas]

On 2013-02-12 00:42, Sami Kerola wrote:
> Karel, and others, what do you think about adding a mode
> to output listing which some might say is not a mode at all?

Just tested the patch and it works here, but wouldn't it be better to
add a "waiting" flag to the mode display, or something like that?
("WAIT-READ" and "WAIT-WRITE"?)



Side note: It seems that fs/locks.c:lock_get_status can output many
other variations – e.g. mandatory locks have a third type called
"ACCESS", shown when a program tries to simply read/write a locked file;
in this case listing the R/W mode might be more useful than "WAITING".
Mandatory locks can apparently say "RW" as well, although I haven't
found a way to create a "RW" lock yet.

Also, while I haven't yet seen this in practice either, the same
fs/locks.c can output lock type "LEASE", which has completely different
values for the 3rd field, and cannot be represented by a binary
"M[andatory] = 0/1" column.

-- 
Mantas Mikulėnas <grawity@gmail.com>

Re: lslocks – "failed to parse pid: 'WRITE'"

[Karel Zak]

On Mon, Feb 11, 2013 at 10:42:36PM +0000, Sami Kerola wrote:
> Subject: [PATCH] lslocks: print waiting state when necessary
> Organization: Lastminute.com
> 
> Earlier output was primarily unexpected.
> 
> $ flock foo -c "sleep 100" & flock foo -c "sleep 100" & flock foo -c
> "sleep 100" & lslocks
> [1] 22352
> [2] 22353
> [3] 22354
> lslocks: failed to parse pid: 'WRITE'
> 
> This commit adds WAITING mode to output listing.  The locks which failed
> with earlier version will look the following.
> 
> COMMAND           PID  TYPE SIZE MODE    M      START        END PATH
> [...]
> flock           22354 FLOCK   0B WAITING 0          0          0
> /home/src/util-linux/foo
> flock           22352 FLOCK   0B WAITING 0          0          0
> /home/src/util-linux/foo
> flock           22353 FLOCK   0B WRITE   0          0          0
> /home/src/util-linux/foo

 It's probably good idea to add information that the process is blocked,
 but I don't think that use WAITING in the MODE column is the right way. 
 We will lost the original information about the lock mode.

 It would be better to add a new column (e.g. BLOCKED).

 We can also add a prefix to MODE, for example ">WRITE" for blocked
 process. The disadvantage is that ">WRITE" is not backwardly compatible.

 (Hmm.. lslocks was introduced in release 2.22 and it's broken now ...
  maybe it's not so big problem ;-)

 It would be also nice to have --blocked command line option to list
 blocked processes only.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: lslocks – "failed to parse pid: 'WRITE'"

[Karel Zak]

On Tue, Feb 12, 2013 at 01:23:21AM +0200, Mantas Mikulėnas wrote:
> On 2013-02-12 00:42, Sami Kerola wrote:
> > Karel, and others, what do you think about adding a mode
> > to output listing which some might say is not a mode at all?
> 
> Just tested the patch and it works here, but wouldn't it be better to
> add a "waiting" flag to the mode display, or something like that?
> ("WAIT-READ" and "WAIT-WRITE"?)
> 
> 
> 
> Side note: It seems that fs/locks.c:lock_get_status can output many
> other variations – e.g. mandatory locks have a third type called
> "ACCESS", shown when a program tries to simply read/write a locked file;
> in this case listing the R/W mode might be more useful than "WAITING".
> Mandatory locks can apparently say "RW" as well, although I haven't
> found a way to create a "RW" lock yet.

 Yes, IMHO we have to follow kernel and keep the original string in
 the MODE column.

> Also, while I haven't yet seen this in practice either, the same
> fs/locks.c can output lock type "LEASE", which has completely different
> values for the 3rd field, and cannot be represented by a binary
> "M[andatory] = 0/1" column.

 Good point. This should be fixed. I think that lease locks could be
 also interpreted as mandatory (so "1" should be used in "M" column). 
 
 But you're right that we need something more to describe lease locks
 -- probably by another column "PROCMODE" (process mode) and use
 keywords BREAKING, ACTIVE and BREAKER from kernel.  
 
 Maybe we can use the same column to identify blocked flock/posix
 processes (by BLOCKED keyword) as I described in my previous email.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: lslocks – "failed to parse pid: 'WRITE'"

[Sami Kerola]

On Tue, Feb 12, 2013 at 11:12 AM, Karel Zak <kzak@redhat.com> wrote:
> On Tue, Feb 12, 2013 at 01:23:21AM +0200, Mantas Mikul=C4=97nas wrote:
>> On 2013-02-12 00:42, Sami Kerola wrote:
>> > Karel, and others, what do you think about adding a mode
>> > to output listing which some might say is not a mode at all?
>>
>> Just tested the patch and it works here, but wouldn't it be better to
>> add a "waiting" flag to the mode display, or something like that?
>> ("WAIT-READ" and "WAIT-WRITE"?)
>>
>>
>>
>> Side note: It seems that fs/locks.c:lock_get_status can output many
>> other variations =E2=80=93 e.g. mandatory locks have a third type called
>> "ACCESS", shown when a program tries to simply read/write a locked file;
>> in this case listing the R/W mode might be more useful than "WAITING".
>> Mandatory locks can apparently say "RW" as well, although I haven't
>> found a way to create a "RW" lock yet.
>
>  Yes, IMHO we have to follow kernel and keep the original string in
>  the MODE column.

Perhaps the columns lslocks displays by default should not be changed.
For the blocked locks adding a small markup to MODE field such as ">"
prefix makes sense to me, as it gives a hint to use there is
information in BLOCKED column.

>> Also, while I haven't yet seen this in practice either, the same
>> fs/locks.c can output lock type "LEASE", which has completely different
>> values for the 3rd field, and cannot be represented by a binary
>> "M[andatory] =3D 0/1" column.
>
>  Good point. This should be fixed. I think that lease locks could be
>  also interpreted as mandatory (so "1" should be used in "M" column).
>
>  But you're right that we need something more to describe lease locks
>  -- probably by another column "PROCMODE" (process mode) and use
>  keywords BREAKING, ACTIVE and BREAKER from kernel.
>
>  Maybe we can use the same column to identify blocked flock/posix
>  processes (by BLOCKED keyword) as I described in my previous email.

I quite like the BLOCKED column, especially when it tells what/who is block=
ing.

Almost off topic.

I noticed another small lslocks annoyance. With narrow window 'lslocks
| cat' is truncating lines. IMHO when writing  to pipe command should
ignore window width.

--=20
   Sami Kerola
   http://www.iki.fi/kerolasa/

Re: lslocks – "failed to parse pid: 'WRITE'"

[Bernhard Voelker]

On 02/12/2013 12:23 PM, Sami Kerola wrote:
> Almost off topic.

while we're at it - ;-)
is it necessary to open/read/close "/proc/self/mountinfo" so often?

  $ strace -e open ./lslocks 2>&1 | grep mountinfo | wc -l
  67

Have a nice day,
Berny

Re: lslocks – "failed to parse pid: 'WRITE'"

[Karel Zak]

On Tue, Feb 12, 2013 at 12:56:10PM +0100, Bernhard Voelker wrote:
> On 02/12/2013 12:23 PM, Sami Kerola wrote:
> > Almost off topic.
> 
> while we're at it - ;-)
> is it necessary to open/read/close "/proc/self/mountinfo" so often?
> 
>   $ strace -e open ./lslocks 2>&1 | grep mountinfo | wc -l
>   67

especially when we have libmount in the same source tree :-) Just use
mnt_new_table_from_file(_PATH_PROC_MOUNTINFO) and keep the pointer...

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: lslocks – "failed to parse pid: 'WRITE'"

[Karel Zak]

On Tue, Feb 12, 2013 at 11:23:49AM +0000, Sami Kerola wrote:
> I noticed another small lslocks annoyance. With narrow window 'lslocks
> | cat' is truncating lines. IMHO when writing  to pipe command should
> ignore window width.

 This is generic lib/tt.c problem. I'll fix it.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: lslocks – "failed to parse pid: 'WRITE'"

[Karel Zak]

On Tue, Feb 12, 2013 at 02:18:30PM +0100, Karel Zak wrote:
> On Tue, Feb 12, 2013 at 12:56:10PM +0100, Bernhard Voelker wrote:
> > On 02/12/2013 12:23 PM, Sami Kerola wrote:
> > > Almost off topic.
> > 
> > while we're at it - ;-)
> > is it necessary to open/read/close "/proc/self/mountinfo" so often?
> > 
> >   $ strace -e open ./lslocks 2>&1 | grep mountinfo | wc -l
> >   67
> 
> especially when we have libmount in the same source tree :-) Just use
> mnt_new_table_from_file(_PATH_PROC_MOUNTINFO) and keep the pointer...

Fixed.

$ strace -e open ./lslocks 2>&1 | grep mountinfo | wc -l
1

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: lslocks – "failed to parse pid: 'WRITE'"

[Karel Zak]

On Tue, Feb 12, 2013 at 11:23:49AM +0000, Sami Kerola wrote:
> riations =E2=80=93 e.g. mandatory locks have a third type called
> >> "ACCESS", shown when a program tries to simply read/write a locked file;
> >> in this case listing the R/W mode might be more useful than "WAITING".
> >> Mandatory locks can apparently say "RW" as well, although I haven't
> >> found a way to create a "RW" lock yet.
> >
> >  Yes, IMHO we have to follow kernel and keep the original string in
> >  the MODE column.
> 
> Perhaps the columns lslocks displays by default should not be changed.
> For the blocked locks adding a small markup to MODE field such as ">"

 I have added "*", for example:

COMMAND           PID  TYPE  SIZE MODE   M      START        END PATH
flock            1318 FLOCK    0B WRITE* 0          0          0 /home/projects/

 it seems better to use postfix than prefix.

> I quite like the BLOCKED column, especially when it tells what/who is block=
> ing.

Good point.

Tomorrow I'm going to add BLOCKED-BY column with PID of the blocker.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com