<feed xmlns='http://www.w3.org/2005/Atom'>
<title>linux.git/fs/locks.c, branch v2.6.24</title>
<subtitle>Linux kernel source tree</subtitle>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/'/>
<entry>
<title>locks: fix possible infinite loop in posix deadlock detection</title>
<updated>2007-10-30T16:04:18+00:00</updated>
<author>
<name>J. Bruce Fields</name>
<email>bfields@citi.umich.edu</email>
</author>
<published>2007-10-30T15:20:02+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=97855b49b6bac0bd25f16b017883634d13591d00'/>
<id>97855b49b6bac0bd25f16b017883634d13591d00</id>
<content type='text'>
It's currently possible to send posix_locks_deadlock() into an infinite
loop (under the BKL).

For now, fix this just by bailing out after a few iterations.  We may
want to fix this in a way that better clarifies the semantics of
deadlock detection.  But that will take more time, and this minimal fix
is probably adequate for any realistic scenario, and is simple enough to
be appropriate for applying to stable kernels now.

Thanks to George Davis for reporting the problem.

Cc: "George G. Davis" &lt;gdavis@mvista.com&gt;
Signed-off-by: J. Bruce Fields &lt;bfields@citi.umich.edu&gt;
Acked-by: Alan Cox &lt;alan@redhat.com&gt;
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
It's currently possible to send posix_locks_deadlock() into an infinite
loop (under the BKL).

For now, fix this just by bailing out after a few iterations.  We may
want to fix this in a way that better clarifies the semantics of
deadlock detection.  But that will take more time, and this minimal fix
is probably adequate for any realistic scenario, and is simple enough to
be appropriate for applying to stable kernels now.

Thanks to George Davis for reporting the problem.

Cc: "George G. Davis" &lt;gdavis@mvista.com&gt;
Signed-off-by: J. Bruce Fields &lt;bfields@citi.umich.edu&gt;
Acked-by: Alan Cox &lt;alan@redhat.com&gt;
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Slab API: remove useless ctor parameter and reorder parameters</title>
<updated>2007-10-17T15:42:45+00:00</updated>
<author>
<name>Christoph Lameter</name>
<email>clameter@sgi.com</email>
</author>
<published>2007-10-17T06:25:51+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=4ba9b9d0ba0a49d91fa6417c7510ee36f48cf957'/>
<id>4ba9b9d0ba0a49d91fa6417c7510ee36f48cf957</id>
<content type='text'>
Slab constructors currently have a flags parameter that is never used.  And
the order of the arguments is opposite to other slab functions.  The object
pointer is placed before the kmem_cache pointer.

Convert

        ctor(void *object, struct kmem_cache *s, unsigned long flags)

to

        ctor(struct kmem_cache *s, void *object)

throughout the kernel

[akpm@linux-foundation.org: coupla fixes]
Signed-off-by: Christoph Lameter &lt;clameter@sgi.com&gt;
Signed-off-by: Andrew Morton &lt;akpm@linux-foundation.org&gt;
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Slab constructors currently have a flags parameter that is never used.  And
the order of the arguments is opposite to other slab functions.  The object
pointer is placed before the kmem_cache pointer.

Convert

        ctor(void *object, struct kmem_cache *s, unsigned long flags)

to

        ctor(struct kmem_cache *s, void *object)

throughout the kernel

[akpm@linux-foundation.org: coupla fixes]
Signed-off-by: Christoph Lameter &lt;clameter@sgi.com&gt;
Signed-off-by: Andrew Morton &lt;akpm@linux-foundation.org&gt;
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Rework /proc/locks via seq_files and seq_list helpers</title>
<updated>2007-10-09T22:32:46+00:00</updated>
<author>
<name>Pavel Emelyanov</name>
<email>xemul@openvz.org</email>
</author>
<published>2007-10-01T21:41:15+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=7f8ada98d9edd83d6ebd01e431e15b024a4a3dc4'/>
<id>7f8ada98d9edd83d6ebd01e431e15b024a4a3dc4</id>
<content type='text'>
Currently /proc/locks is shown with a proc_read function, but its behavior
is rather complex as it has to manually handle current offset and buffer
length.  On the other hand, files that show objects from lists can be
easily reimplemented using the sequential files and the seq_list_XXX()
helpers.

This saves (as usually) 16 lines of code and more than 200 from
the .text section.

[akpm@linux-foundation.org: no externs in C]
[akpm@linux-foundation.org: warning fixes]
Signed-off-by: Pavel Emelyanov &lt;xemul@openvz.org&gt;
Cc: "J. Bruce Fields" &lt;bfields@fieldses.org&gt;
Cc: Trond Myklebust &lt;trond.myklebust@fys.uio.no&gt;
Signed-off-by: Andrew Morton &lt;akpm@linux-foundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Currently /proc/locks is shown with a proc_read function, but its behavior
is rather complex as it has to manually handle current offset and buffer
length.  On the other hand, files that show objects from lists can be
easily reimplemented using the sequential files and the seq_list_XXX()
helpers.

This saves (as usually) 16 lines of code and more than 200 from
the .text section.

[akpm@linux-foundation.org: no externs in C]
[akpm@linux-foundation.org: warning fixes]
Signed-off-by: Pavel Emelyanov &lt;xemul@openvz.org&gt;
Cc: "J. Bruce Fields" &lt;bfields@fieldses.org&gt;
Cc: Trond Myklebust &lt;trond.myklebust@fys.uio.no&gt;
Signed-off-by: Andrew Morton &lt;akpm@linux-foundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>fs/locks.c: use list_for_each_entry() instead of list_for_each()</title>
<updated>2007-10-09T22:32:46+00:00</updated>
<author>
<name>Matthias Kaehlcke</name>
<email>matthias.kaehlcke@gmail.com</email>
</author>
<published>2007-10-02T18:21:34+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=094f2825218fec1b240cb8537d2d0a10edf5ddc9'/>
<id>094f2825218fec1b240cb8537d2d0a10edf5ddc9</id>
<content type='text'>
fs/locks.c: use list_for_each_entry() instead of list_for_each() in
posix_locks_deadlock() and get_locks_status()

Signed-off-by: Matthias Kaehlcke &lt;matthias.kaehlcke@gmail.com&gt;
Signed-off-by: Andrew Morton &lt;akpm@linux-foundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
fs/locks.c: use list_for_each_entry() instead of list_for_each() in
posix_locks_deadlock() and get_locks_status()

Signed-off-by: Matthias Kaehlcke &lt;matthias.kaehlcke@gmail.com&gt;
Signed-off-by: Andrew Morton &lt;akpm@linux-foundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Cleanup macros for distinguishing mandatory locks</title>
<updated>2007-10-09T22:32:46+00:00</updated>
<author>
<name>Pavel Emelyanov</name>
<email>xemul@openvz.org</email>
</author>
<published>2007-10-01T21:41:11+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=a16877ca9cec211708a161057a7cbfbf2cbc3a53'/>
<id>a16877ca9cec211708a161057a7cbfbf2cbc3a53</id>
<content type='text'>
The combination of S_ISGID bit set and S_IXGRP bit unset is used to mark the
inode as "mandatory lockable" and there's a macro for this check called
MANDATORY_LOCK(inode).  However, fs/locks.c and some filesystems still perform
the explicit i_mode checking.  Besides, Andrew pointed out, that this macro is
buggy itself, as it dereferences the inode arg twice.

Convert this macro into static inline function and switch its users to it,
making the code shorter and more readable.

The __mandatory_lock() helper is to be used in places where the IS_MANDLOCK()
for superblock is already known to be true.

Signed-off-by: Pavel Emelyanov &lt;xemul@openvz.org&gt;
Cc: Trond Myklebust &lt;trond.myklebust@fys.uio.no&gt;
Cc: "J. Bruce Fields" &lt;bfields@fieldses.org&gt;
Cc: David Howells &lt;dhowells@redhat.com&gt;
Cc: Eric Van Hensbergen &lt;ericvh@gmail.com&gt;
Cc: Ron Minnich &lt;rminnich@sandia.gov&gt;
Cc: Latchesar Ionkov &lt;lucho@ionkov.net&gt;
Cc: Steven Whitehouse &lt;swhiteho@redhat.com&gt;
Signed-off-by: Andrew Morton &lt;akpm@linux-foundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The combination of S_ISGID bit set and S_IXGRP bit unset is used to mark the
inode as "mandatory lockable" and there's a macro for this check called
MANDATORY_LOCK(inode).  However, fs/locks.c and some filesystems still perform
the explicit i_mode checking.  Besides, Andrew pointed out, that this macro is
buggy itself, as it dereferences the inode arg twice.

Convert this macro into static inline function and switch its users to it,
making the code shorter and more readable.

The __mandatory_lock() helper is to be used in places where the IS_MANDLOCK()
for superblock is already known to be true.

Signed-off-by: Pavel Emelyanov &lt;xemul@openvz.org&gt;
Cc: Trond Myklebust &lt;trond.myklebust@fys.uio.no&gt;
Cc: "J. Bruce Fields" &lt;bfields@fieldses.org&gt;
Cc: David Howells &lt;dhowells@redhat.com&gt;
Cc: Eric Van Hensbergen &lt;ericvh@gmail.com&gt;
Cc: Ron Minnich &lt;rminnich@sandia.gov&gt;
Cc: Latchesar Ionkov &lt;lucho@ionkov.net&gt;
Cc: Steven Whitehouse &lt;swhiteho@redhat.com&gt;
Signed-off-by: Andrew Morton &lt;akpm@linux-foundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>locks: Fix potential OOPS in generic_setlease()</title>
<updated>2007-10-09T22:32:45+00:00</updated>
<author>
<name>Pavel Emelyanov</name>
<email>xemul@openvz.org</email>
</author>
<published>2007-09-20T08:45:02+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=85c59580b30c82aa771aa33b37217a6b6851bc14'/>
<id>85c59580b30c82aa771aa33b37217a6b6851bc14</id>
<content type='text'>
This code is run under lock_kernel(), which is dropped during
sleeping operations, so the following race is possible:

CPU1:                                CPU2:
  vfs_setlease();                    vfs_setlease();
  lock_kernel();
                                     lock_kernel(); /* spin */
  generic_setlease():
    ...
    for (before = ...)
    /* here we found some lease after
     * which we will insert the new one
     */
    fl = locks_alloc_lock();
    /* go to sleep in this allocation and
     * drop the BKL
     */
                                     generic_setlease():
                                       ...
                                       for (before = ...)
                                       /* here we find the "before" pointing
                                        * at the one we found on CPU1
                                        */
                                      -&gt;fl_change(my_before, arg);
                                              lease_modify();
                                                     locks_free_lock();
                                                     /* and we freed it */
                                     ...
                                     unlock_kernel();
   locks_insert_lock(before, fl);
   /* OOPS! We have just tried to add the lease
    * at the tail of already removed one
    */

The similar races are already handled in other code - all the
allocations are performed before any checks/updates.

Thanks to Kamalesh Babulal for testing and for a bug report on an
earlier version.

Signed-off-by: Pavel Emelyanov &lt;xemul@openvz.org&gt;
Signed-off-by: J. Bruce Fields &lt;bfields@citi.umich.edu&gt;
Cc: Kamalesh Babulal &lt;kamalesh@linux.vnet.ibm.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This code is run under lock_kernel(), which is dropped during
sleeping operations, so the following race is possible:

CPU1:                                CPU2:
  vfs_setlease();                    vfs_setlease();
  lock_kernel();
                                     lock_kernel(); /* spin */
  generic_setlease():
    ...
    for (before = ...)
    /* here we found some lease after
     * which we will insert the new one
     */
    fl = locks_alloc_lock();
    /* go to sleep in this allocation and
     * drop the BKL
     */
                                     generic_setlease():
                                       ...
                                       for (before = ...)
                                       /* here we find the "before" pointing
                                        * at the one we found on CPU1
                                        */
                                      -&gt;fl_change(my_before, arg);
                                              lease_modify();
                                                     locks_free_lock();
                                                     /* and we freed it */
                                     ...
                                     unlock_kernel();
   locks_insert_lock(before, fl);
   /* OOPS! We have just tried to add the lease
    * at the tail of already removed one
    */

The similar races are already handled in other code - all the
allocations are performed before any checks/updates.

Thanks to Kamalesh Babulal for testing and for a bug report on an
earlier version.

Signed-off-by: Pavel Emelyanov &lt;xemul@openvz.org&gt;
Signed-off-by: J. Bruce Fields &lt;bfields@citi.umich.edu&gt;
Cc: Kamalesh Babulal &lt;kamalesh@linux.vnet.ibm.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Use list_first_entry in locks_wake_up_blocks</title>
<updated>2007-10-09T22:32:45+00:00</updated>
<author>
<name>Pavel Emelyanov</name>
<email>xemul@openvz.org</email>
</author>
<published>2007-09-19T12:44:07+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=f0c1cd0eaf0b127356c2c09e40305453bc361b0f'/>
<id>f0c1cd0eaf0b127356c2c09e40305453bc361b0f</id>
<content type='text'>
This routine deletes all the elements from the list
with the "while (!list_empty())" loop, and we already
have a list_first_entry() macro to help it look nicer :)

Signed-off-by: Pavel Emelyanov &lt;xemul@openvz.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This routine deletes all the elements from the list
with the "while (!list_empty())" loop, and we already
have a list_first_entry() macro to help it look nicer :)

Signed-off-by: Pavel Emelyanov &lt;xemul@openvz.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>locks: fix flock_lock_file() comment</title>
<updated>2007-10-09T22:32:45+00:00</updated>
<author>
<name>J. Bruce Fields</name>
<email>bfields@citi.umich.edu</email>
</author>
<published>2007-09-12T19:45:07+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=02888f41e9d7fa95d1f5b2f76e0f0af6ea8198cc'/>
<id>02888f41e9d7fa95d1f5b2f76e0f0af6ea8198cc</id>
<content type='text'>
This comment wasn't updated when lease support was added, and it makes
essentially the same mistake that the code made before a recent bugfix.

Signed-off-by: J. Bruce Fields &lt;bfields@citi.umich.edu&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This comment wasn't updated when lease support was added, and it makes
essentially the same mistake that the code made before a recent bugfix.

Signed-off-by: J. Bruce Fields &lt;bfields@citi.umich.edu&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Memory shortage can result in inconsistent flocks state</title>
<updated>2007-10-09T22:32:45+00:00</updated>
<author>
<name>Pavel Emelyanov</name>
<email>xemul@openvz.org</email>
</author>
<published>2007-09-11T12:38:13+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=84d535ade62b6f8ce852745731ad6200c46b977c'/>
<id>84d535ade62b6f8ce852745731ad6200c46b977c</id>
<content type='text'>
When the flock_lock_file() is called to change the flock
from F_RDLCK to F_WRLCK or vice versa the existing flock
can be removed without appropriate warning.

Look:
        for_each_lock(inode, before) {
                struct file_lock *fl = *before;
                if (IS_POSIX(fl))
                        break;
                if (IS_LEASE(fl))
                        continue;
                if (filp != fl-&gt;fl_file)
                        continue;
                if (request-&gt;fl_type == fl-&gt;fl_type)
                        goto out;
                found = 1;
                locks_delete_lock(before); &lt;&lt;&lt;&lt;&lt;&lt; !
                break;
        }

if after this point the subsequent locks_alloc_lock() will
fail the return code will be -ENOMEM, but the existing lock
is already removed.

This is a known feature that such "re-locking" is not atomic,
but in the racy case the file should stay locked (although by
some other process), but in this case the file will be unlocked.

The proposal is to prepare the lock in advance keeping no chance
to fail in the future code.

Found during making the flocks pid-namespaces aware.

(Note: Thanks to Reuben Farrelly for finding a bug in an earlier version
of this patch.)

Signed-off-by: Pavel Emelyanov &lt;xemul@openvz.org&gt;
Signed-off-by: J. Bruce Fields &lt;bfields@citi.umich.edu&gt;
Cc: Reuben Farrelly &lt;reuben-linuxkernel@reub.net&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When the flock_lock_file() is called to change the flock
from F_RDLCK to F_WRLCK or vice versa the existing flock
can be removed without appropriate warning.

Look:
        for_each_lock(inode, before) {
                struct file_lock *fl = *before;
                if (IS_POSIX(fl))
                        break;
                if (IS_LEASE(fl))
                        continue;
                if (filp != fl-&gt;fl_file)
                        continue;
                if (request-&gt;fl_type == fl-&gt;fl_type)
                        goto out;
                found = 1;
                locks_delete_lock(before); &lt;&lt;&lt;&lt;&lt;&lt; !
                break;
        }

if after this point the subsequent locks_alloc_lock() will
fail the return code will be -ENOMEM, but the existing lock
is already removed.

This is a known feature that such "re-locking" is not atomic,
but in the racy case the file should stay locked (although by
some other process), but in this case the file will be unlocked.

The proposal is to prepare the lock in advance keeping no chance
to fail in the future code.

Found during making the flocks pid-namespaces aware.

(Note: Thanks to Reuben Farrelly for finding a bug in an earlier version
of this patch.)

Signed-off-by: Pavel Emelyanov &lt;xemul@openvz.org&gt;
Signed-off-by: J. Bruce Fields &lt;bfields@citi.umich.edu&gt;
Cc: Reuben Farrelly &lt;reuben-linuxkernel@reub.net&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>locks: kill redundant local variable</title>
<updated>2007-10-09T22:32:45+00:00</updated>
<author>
<name>J. Bruce Fields</name>
<email>bfields@fieldses.org</email>
</author>
<published>2006-11-14T21:54:36+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=526985b9dd6ef7716b87f5fe6f0e2438ea3a89c7'/>
<id>526985b9dd6ef7716b87f5fe6f0e2438ea3a89c7</id>
<content type='text'>
There's no need for another variable local to this loop; we can use the
variable (of the same name!) already declared at the top of the function,
and not used till later (at which point it's initialized, so this is safe).

Signed-off-by: J. Bruce Fields &lt;bfields@citi.umich.edu&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
There's no need for another variable local to this loop; we can use the
variable (of the same name!) already declared at the top of the function,
and not used till later (at which point it's initialized, so this is safe).

Signed-off-by: J. Bruce Fields &lt;bfields@citi.umich.edu&gt;
</pre>
</div>
</content>
</entry>
</feed>
