the following program will oom a the 2.6.14.1 kernel, running as an
ordinary user:
#include <unistd.h>
#include <stdlib.h>
#include <linux/fcntl.h>
int main(int ac, char **av)
{
char *fname = av[0];
int fd = open(fname, O_RDONLY);
int r;
while (1) {
r = fcntl(fd, F_SETLEASE, F_RDLCK);
if (r == -1) {
perror("F_SETLEASE, F_RDLCK");
exit(1);
}
r = fcntl(fd, F_SETLEASE, F_UNLCK);
if (r == -1) {
perror("F_SETLEASE, F_UNLCK");
exit(1);
}
}
return 0;
}
it will suck all available memory into fasync_cache, causing an oom. a
workaround is to set fs.leases-enable to 0.
this has already been reported to lkml[1] and fedora[2], with no effect.
[1] http://www.ussg.iu.edu/hypermail/linux/kernel/0510.2/1589.html
[2] https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=172691
* Avi Kivity ([email protected]) wrote:
> the following program will oom a the 2.6.14.1 kernel, running as an
> ordinary user:
I don't have a good mechanism for testing leases, but this should fix
the leak. Mind testing?
thanks,
-chris
--
When unlocking lease make sure to release fasync_struct.
Signed-off-by: Chris Wright <[email protected]>
---
diff --git a/fs/locks.c b/fs/locks.c
index a1e8b22..abba0f6 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1412,7 +1412,7 @@ int fcntl_setlease(unsigned int fd, stru
struct file_lock fl, *flp = &fl;
struct dentry *dentry = filp->f_dentry;
struct inode *inode = dentry->d_inode;
- int error;
+ int error, on = 1;
if ((current->fsuid != inode->i_uid) && !capable(CAP_LEASE))
return -EACCES;
@@ -1433,7 +1433,10 @@ int fcntl_setlease(unsigned int fd, stru
if (error)
goto out_unlock;
- error = fasync_helper(fd, filp, 1, &flp->fl_fasync);
+ if (arg == F_UNLCK)
+ on = 0;
+
+ error = fasync_helper(fd, filp, on, &flp->fl_fasync);
if (error < 0) {
/* remove lease just inserted by __setlease */
flp->fl_type = F_UNLCK | F_INPROGRESS;
Chris Wright wrote:
>* Avi Kivity ([email protected]) wrote:
>
>
>>the following program will oom a the 2.6.14.1 kernel, running as an
>>ordinary user:
>>
>>
>
>I don't have a good mechanism for testing leases, but this should fix
>the leak. Mind testing?
>
>
>
the test program of course passes, but now samba hangs when reading a
file (mount -t cifs from the same machine). 2.6.14.1 reads the file but
leaks memory.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
Avi Kivity wrote:
>>
>> I don't have a good mechanism for testing leases, but this should fix
>> the leak. Mind testing?
>>
>>
>>
> the test program of course passes, but now samba hangs when reading a
> file (mount -t cifs from the same machine). 2.6.14.1 reads the file
> but leaks memory.
>
Sorry, it doesn't hang, it's just very slow (3Mbit/sec over loopback),
but that's not because of the leases AFAICT.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
On Fri, 2005-11-11 at 00:45 -0800, Chris Wright wrote:
> * Avi Kivity ([email protected]) wrote:
> > the following program will oom a the 2.6.14.1 kernel, running as an
> > ordinary user:
>
> I don't have a good mechanism for testing leases, but this should fix
> the leak. Mind testing?
>
Bruce has a simpler patch (see attachment). The call to fasync_helper()
in order to free active structures will have already been done in
locks_delete_lock(), so in principle, all we want to do is to skip the
fasync_helper() call in fcntl_setlease().
Cheers,
Trond
* Trond Myklebust ([email protected]) wrote:
> Bruce has a simpler patch (see attachment). The call to fasync_helper()
> in order to free active structures will have already been done in
> locks_delete_lock(), so in principle, all we want to do is to skip the
> fasync_helper() call in fcntl_setlease().
Yes, that's better, thanks. Will you make sure it gets to Linus?
thanks,
-chris
On Fri, 2005-11-11 at 10:35 -0800, Chris Wright wrote:
> * Trond Myklebust ([email protected]) wrote:
> > Bruce has a simpler patch (see attachment). The call to fasync_helper()
> > in order to free active structures will have already been done in
> > locks_delete_lock(), so in principle, all we want to do is to skip the
> > fasync_helper() call in fcntl_setlease().
>
> Yes, that's better, thanks. Will you make sure it gets to Linus?
Sure, but I'd like a mail from Avi confirming that this patch too fixes
his problem, please.
Cheers,
Trond
* Trond Myklebust ([email protected]) wrote:
> On Fri, 2005-11-11 at 10:35 -0800, Chris Wright wrote:
> > * Trond Myklebust ([email protected]) wrote:
> > > Bruce has a simpler patch (see attachment). The call to fasync_helper()
> > > in order to free active structures will have already been done in
> > > locks_delete_lock(), so in principle, all we want to do is to skip the
> > > fasync_helper() call in fcntl_setlease().
> >
> > Yes, that's better, thanks. Will you make sure it gets to Linus?
>
> Sure, but I'd like a mail from Avi confirming that this patch too fixes
> his problem, please.
OK, I tested with Avi's test program, and a couple other's I cobbled
together, and they seem to work fine. But didn't test the samba case
(shouldn't be different...but...). BTW, the bit below looks like
debugging code. It's a way for users to spam the kernel log (granted
there is some bit of throttling):
thanks,
-chris
--
Remove time_out_leases() printk that's easily triggered by users.
Signed-off-by: Chris Wright <[email protected]>
---
diff --git a/fs/locks.c b/fs/locks.c
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1105,7 +1105,6 @@ static void time_out_leases(struct inode
before = &fl->fl_next;
continue;
}
- printk(KERN_INFO "lease broken - owner pid = %d\n", fl->fl_pid);
lease_modify(before, fl->fl_type & ~F_INPROGRESS);
if (fl == *before) /* lease_modify may have freed fl */
before = &fl->fl_next;
Chris Wright wrote:
>>Sure, but I'd like a mail from Avi confirming that this patch too fixes
>>his problem, please.
>>
>>
>
>OK, I tested with Avi's test program, and a couple other's I cobbled
>together, and they seem to work fine. But didn't test the samba case
>(shouldn't be different...but...).
>
FWIW, I've run a short (and successful) test with samba.
Avi