Hi,
the semantics of fnctl used together with F_SETLEASE and
argument F_RDLCK have been changed with bk changeset
1.1938.185.141 (sometime between 2.6.9 and 2.6.10).
Since then it's only possible to get a read lease when the
file in question does not have _any_ writers.
This is at least inconsistent with the man page of fcntl
and looks pretty much like this is a bug in the kernel.
Any comments?
Thanks,
Heiko
Hi Heiko,
On Mon, 2 May 2005 11:15:24 +0200 Heiko Carstens <[email protected]> wrote:
>
> the semantics of fnctl used together with F_SETLEASE and
> argument F_RDLCK have been changed with bk changeset
> 1.1938.185.141 (sometime between 2.6.9 and 2.6.10).
> Since then it's only possible to get a read lease when the
> file in question does not have _any_ writers.
> This is at least inconsistent with the man page of fcntl
> and looks pretty much like this is a bug in the kernel.
>
> Any comments?
The previous behaviour was a bug that occurred because at the time the
original lease code was written, it was not possible to tell if there were
writers when the read lease was being taken. Further improvements in the
kernel have since made this possible.
The intention of a read lease is to let the holder know is anyone tries to
modify the file.
The current behaviour does not conflict with the man pages on Debian
(although the previous behaviour did not either :-))
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/
> The intention of a read lease is to let the holder know is anyone tries
to
> modify the file.
>
> The current behaviour does not conflict with the man pages on Debian
> (although the previous behaviour did not either :-))
Now that I know what the intention of the read lease is, I agree that
there
is no conflict :). Thanks for clarifying!
Heiko
Hi Stephen,
> On Mon, 2 May 2005 11:15:24 +0200 Heiko Carstens
> <[email protected]> wrote:
> >
> > the semantics of fnctl used together with F_SETLEASE and
> > argument F_RDLCK have been changed with bk changeset
> > 1.1938.185.141 (sometime between 2.6.9 and 2.6.10).
> > Since then it's only possible to get a read lease when the
> > file in question does not have _any_ writers.
> > This is at least inconsistent with the man page of fcntl
> > and looks pretty much like this is a bug in the kernel.
> >
> > Any comments?
>
> The previous behaviour was a bug that occurred because at the time the
> original lease code was written, it was not possible to tell if there
> were
> writers when the read lease was being taken. Further improvements in the
> kernel have since made this possible.
>
> The intention of a read lease is to let the holder know is anyone tries
> to modify the file.
>
> The current behaviour does not conflict with the man pages on Debian
> (although the previous behaviour did not either :-))
Indeed the problem referred to is fixed, but it looks like another
one may have been introduced.
It now appears (I tested on 2.6.11.6) that if a process opens
a file O_RDWR and then tries to place a read lease via that
file descriptor, then the F_SETLEASE fails with EAGAIN,
even though no other process has the file open for writing.
(On the other hand, if the process opens the file
O_WRONLY, then it can place either a read or a write lease.
This is how I think things always worked, but it seems
inconsistent with the aforementioned behaviour.)
Some further testing showed the following (both open()
and fcntl(F_SETLEASE) from same process):
open() | lease requested
flag | F_RDLCK | F_WRLCK
---------+----------+----------
O_RDONLY | okay | okay
O_WRONLY | EAGAIN | okay
O_RDWR | EAGAIN | okay
This seems strange (I imagine the caller should be excluded
from the list of processes being checked to see if the file
is opened for writing), and differs from earlier kernel
versions. What is the intended behaviour here?
Cheers,
Michael
--
+++ Lassen Sie Ihren Gedanken freien Lauf... z.B. per FreeSMS +++
GMX bietet bis zu 100 FreeSMS/Monat: http://www.gmx.net/de/go/mail
Hi Michael,
On Tue, 3 May 2005 12:00:18 +0200 (MEST) "Michael Kerrisk" <[email protected]> wrote:
>
> Indeed the problem referred to is fixed, but it looks like another
> one may have been introduced.
>
> It now appears (I tested on 2.6.11.6) that if a process opens
> a file O_RDWR and then tries to place a read lease via that
> file descriptor, then the F_SETLEASE fails with EAGAIN,
> even though no other process has the file open for writing.
> (On the other hand, if the process opens the file
> O_WRONLY, then it can place either a read or a write lease.
> This is how I think things always worked, but it seems
> inconsistent with the aforementioned behaviour.)
>
> Some further testing showed the following (both open()
> and fcntl(F_SETLEASE) from same process):
>
> open() | lease requested
> flag | F_RDLCK | F_WRLCK
> ---------+----------+----------
> O_RDONLY | okay | okay
> O_WRONLY | EAGAIN | okay
> O_RDWR | EAGAIN | okay
>
> This seems strange (I imagine the caller should be excluded
> from the list of processes being checked to see if the file
> is opened for writing), and differs from earlier kernel
> versions. What is the intended behaviour here?
Thanks for the testing. My expectation is that it shouldn't matter how
the current process opened the file for either type of lease. However,
you are right (IMHO) that the current process should *not* be counted as a
writer in the case of trying to obtain a F_RDLCK lease.
How does this (completely untested, not even compiled) patch look?
diff -ruNP linus/fs/locks.c linus-leases.1/fs/locks.c
--- linus/fs/locks.c 2005-04-26 15:38:00.000000000 +1000
+++ linus-leases.1/fs/locks.c 2005-05-03 23:00:14.000000000 +1000
@@ -1288,7 +1288,8 @@
goto out;
error = -EAGAIN;
- if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0))
+ if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount)
+ > ((filp->f_mode & FMODE_WRITE) ? 1 : 0)))
goto out;
if ((arg == F_WRLCK)
&& ((atomic_read(&dentry->d_count) > 1)
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/
> Hi Michael,
>
> On Tue, 3 May 2005 12:00:18 +0200 (MEST) "Michael Kerrisk" <[email protected]> wrote:
> >
> > Indeed the problem referred to is fixed, but it looks like another
> > one may have been introduced.
> >
> > It now appears (I tested on 2.6.11.6) that if a process opens
> > a file O_RDWR and then tries to place a read lease via that
> > file descriptor, then the F_SETLEASE fails with EAGAIN,
> > even though no other process has the file open for writing.
> > (On the other hand, if the process opens the file
> > O_WRONLY, then it can place either a read or a write lease.
> > This is how I think things always worked, but it seems
> > inconsistent with the aforementioned behaviour.)
> >
> > Some further testing showed the following (both open()
> > and fcntl(F_SETLEASE) from same process):
> >
> > open() | lease requested
> > flag | F_RDLCK | F_WRLCK
> > ---------+----------+----------
> > O_RDONLY | okay | okay
> > O_WRONLY | EAGAIN | okay
> > O_RDWR | EAGAIN | okay
> >
> > This seems strange (I imagine the caller should be excluded
> > from the list of processes being checked to see if the file
> > is opened for writing), and differs from earlier kernel
> > versions. What is the intended behaviour here?
>
> Thanks for the testing. My expectation is that it shouldn't matter how
> the current process opened the file for either type of lease. However,
> you are right (IMHO) that the current process should *not* be counted as a
> writer in the case of trying to obtain a F_RDLCK lease.
i believe the current implementation is correct. opening a file for write
means that you can not have a read lease, caller included.
-->Andy
>
> How does this (completely untested, not even compiled) patch look?
>
> diff -ruNP linus/fs/locks.c linus-leases.1/fs/locks.c
> --- linus/fs/locks.c 2005-04-26 15:38:00.000000000 +1000
> +++ linus-leases.1/fs/locks.c 2005-05-03 23:00:14.000000000 +1000
> @@ -1288,7 +1288,8 @@
> goto out;
>
> error = -EAGAIN;
> - if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0))
> + if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount)
> + > ((filp->f_mode & FMODE_WRITE) ? 1 : 0)))
> goto out;
> if ((arg == F_WRLCK)
> && ((atomic_read(&dentry->d_count) > 1)
>
> --
> Cheers,
> Stephen Rothwell [email protected]
> http://www.canb.auug.org.au/~sfr/
On Tue, May 03, 2005 at 09:55:42AM -0400, William A.(Andy) Adamson wrote:
> i believe the current implementation is correct. opening a file for write
> means that you can not have a read lease, caller included.
Why not? Certainly, others will not be able to take out a read lease,
so there's very little point to only having a read lease, but I don't
see why we should deny it.
--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain
> On Tue, May 03, 2005 at 09:55:42AM -0400, William A.(Andy) Adamson wrote:
> > i believe the current implementation is correct. opening a file for write
> > means that you can not have a read lease, caller included.
>
> Why not? Certainly, others will not be able to take out a read lease,
> so there's very little point to only having a read lease, but I don't
> see why we should deny it.
>
by definition: a read lease means there are no writers. so, the question is
not 'why not', the question is why? why hand out a read lease to an open for
write?
-->Andy
> > On Tue, May 03, 2005 at 09:55:42AM -0400, William A.(Andy) Adamson
> > wrote:
> > > i believe the current implementation is correct. opening a file for
> > > write
> > > means that you can not have a read lease, caller included.
> >
> > Why not? Certainly, others will not be able to take out a read lease,
> > so there's very little point to only having a read lease, but I don't
> > see why we should deny it.
> >
>
> by definition: a read lease means there are no writers. so, the question
> is
> not 'why not', the question is why? why hand out a read lease to an open
> for write?
Andy,
Look more closely at my earlier table.
Regardless of what the answer to your question is, the
current semantics are bizarre. As things stand, a process
can open a file O_RDWR, and and can place a WRITE lease
but not a READ lease. That can't be right.
FWIW it's worth, I think the read lease should be allowed.
Oplocks are concerned with what other processes are doing,
not what the caller is doing. Also, the current semantcis
break backward compatibility.
Cheers,
Michael
--
+++ Neu: Echte DSL-Flatrates von GMX - Surfen ohne Limits +++
Always online ab 4,99 Euro/Monat: http://www.gmx.net/de/go/dsl
> > > On Tue, May 03, 2005 at 09:55:42AM -0400, William A.(Andy) Adamson
> > > wrote:
> > > > i believe the current implementation is correct. opening a file for
> > > > write
> > > > means that you can not have a read lease, caller included.
> > >
> > > Why not? Certainly, others will not be able to take out a read lease,
> > > so there's very little point to only having a read lease, but I don't
> > > see why we should deny it.
> > >
> >
> > by definition: a read lease means there are no writers. so, the question
> > is
> > not 'why not', the question is why? why hand out a read lease to an open
> > for write?
>
> Andy,
>
> Look more closely at my earlier table.
>
> Regardless of what the answer to your question is, the
> current semantics are bizarre. As things stand, a process
> can open a file O_RDWR, and and can place a WRITE lease
> but not a READ lease. That can't be right.
yes - i was being too strict. looking at NFSv4 delegations; a read lease does
not mean there are no writers, it means there are no other clients (fl_owners)
writing.
the other side of the coin would be break_lease. it should not break a read
lease on an open for write in the case where the fl_owner of the read lease is
also the owner of the open for write.
-->Andy
>
> FWIW it's worth, I think the read lease should be allowed.
> Oplocks are concerned with what other processes are doing,
> not what the caller is doing. Also, the current semantcis
> break backward compatibility.
>
> Cheers,
>
> Michael
>
> --
> +++ Neu: Echte DSL-Flatrates von GMX - Surfen ohne Limits +++
> Always online ab 4,99 Euro/Monat: http://www.gmx.net/de/go/dsl
On Tue, May 03, 2005 at 12:21:24PM -0400, William A.(Andy) Adamson wrote:
> the other side of the coin would be break_lease.
Yeah, I'm a little confused as to why anyone would have the expectation
that read leases would not conflict with write opens by the same
process, given that break_lease() has never functioned that way, so
later write opens by the same process have always broken any read lease.
Are there applications that actually depend on the old behaviour? Is
there any documentation that blesses it? All I can find is the fcntl
man page, and as far as I can tell an implementation that makes read
leases conflict with all write opens (by the same process or not) is
consistent with that man page.
--b.
Hello Stephen,
Sorry for the long delay in replying; other things intervened.
> On Tue, 3 May 2005 12:00:18 +0200 (MEST) "Michael Kerrisk"
> <[email protected]> wrote:
> >
> > Indeed the problem referred to is fixed, but it looks like another
> > one may have been introduced.
> >
> > It now appears (I tested on 2.6.11.6) that if a process opens
> > a file O_RDWR and then tries to place a read lease via that
> > file descriptor, then the F_SETLEASE fails with EAGAIN,
> > even though no other process has the file open for writing.
> > (On the other hand, if the process opens the file
> > O_WRONLY, then it can place either a read or a write lease.
> > This is how I think things always worked, but it seems
> > inconsistent with the aforementioned behaviour.)
> >
> > Some further testing showed the following (both open()
> > and fcntl(F_SETLEASE) from same process):
> >
> > open() | lease requested
> > flag | F_RDLCK | F_WRLCK
> > ---------+----------+----------
> > O_RDONLY | okay | okay
> > O_WRONLY | EAGAIN | okay
> > O_RDWR | EAGAIN | okay
> >
> > This seems strange (I imagine the caller should be excluded
> > from the list of processes being checked to see if the file
> > is opened for writing), and differs from earlier kernel
> > versions. What is the intended behaviour here?
>
> Thanks for the testing. My expectation is that it shouldn't matter how
> the current process opened the file for either type of lease. However,
> you are right (IMHO) that the current process should *not* be counted as
> a writer in the case of trying to obtain a F_RDLCK lease.
>
> How does this (completely untested, not even compiled) patch look?
>
> diff -ruNP linus/fs/locks.c linus-leases.1/fs/locks.c
> --- linus/fs/locks.c 2005-04-26 15:38:00.000000000 +1000
> +++ linus-leases.1/fs/locks.c 2005-05-03 23:00:14.000000000 +1000
> @@ -1288,7 +1288,8 @@
> goto out;
>
> error = -EAGAIN;
> - if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0))
> + if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount)
> + > ((filp->f_mode & FMODE_WRITE) ? 1 : 0)))
> goto out;
> if ((arg == F_WRLCK)
> && ((atomic_read(&dentry->d_count) > 1)
I applied this against 2.6.12-rc4, and it fixes the problem
(and I've also teasted various other facets of file leases
and this change causes no obvious breakage elsewhere).
Are you going to push this fix into 2.6.12?
Cheers,
Michael
--
Geschenkt: 3 Monate GMX ProMail gratis + 3 Ausgaben stern gratis
++ Jetzt anmelden & testen ++ http://www.gmx.net/de/go/promail ++
On Tue, May 31, 2005 at 04:53:50PM +0200, Michael Kerrisk wrote:
> I applied this against 2.6.12-rc4, and it fixes the problem
> (and I've also teasted various other facets of file leases
> and this change causes no obvious breakage elsewhere).
>
> Are you going to push this fix into 2.6.12?
Are you sure this is actually a problem?
I still have the following questions I had before:
> I'm a little confused as to why anyone would have the expectation
> that read leases would not conflict with write opens by the same
> process, given that break_lease() has never functioned that way, so
> later write opens by the same process have always broken any read lease.
>
> Are there applications that actually depend on the old behaviour? Is
> there any documentation that blesses it? All I can find is the fcntl
> man page, and as far as I can tell an implementation that makes read
> leases conflict with all write opens (by the same process or not) is
> consistent with that man page.
--b.
Bruce,
> On Tue, May 31, 2005 at 04:53:50PM +0200, Michael Kerrisk wrote:
> > I applied this against 2.6.12-rc4, and it fixes the problem
> > (and I've also teasted various other facets of file leases
> > and this change causes no obvious breakage elsewhere).
> >
> > Are you going to push this fix into 2.6.12?
>
> Are you sure this is actually a problem?
>
> I still have the following questions I had before:
>
> > I'm a little confused as to why anyone would have the expectation
> > that read leases would not conflict with write opens by the same
> > process, given that break_lease() has never functioned that way, so
> > later write opens by the same process have always broken any read
> > lease.
> >
> > Are there applications that actually depend on the old behaviour? Is
> > there any documentation that blesses it? All I can find is the fcntl
> > man page, and as far as I can tell an implementation that makes read
> > leases conflict with all write opens (by the same process or not) is
> > consistent with that man page.
I believe it is still a problem: primarily because it broke
old behavior for no apparent reason (Stephen Rothwell, who was
one of the original implementers seems to agree, since he
suggested that one line patch). I suspect the change was
unintentional.
By the way, I wrote the text in the fcntl() man page
by looking at the code and experimenting. There was no
existing documentation of F_SETLEASE. I'm questioning
the change based on my understanding of how things should
work (I didn't happen to write up this point because it
seemed self-evident *to me*); however, I know rather
little of the workings of SAMBA.
Cheers,
Michael
--
Weitersagen: GMX DSL-Flatrates mit Tempo-Garantie!
Ab 4,99 Euro/Monat: http://www.gmx.net/de/go/dsl