All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael Kerrisk" <michael.kerrisk@gmx.net>
To: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: mtk-lkml@gmx.net, heiko.carstens@de.ibm.com,
	linux-kernel@vger.kernel.org, andros@citi.umich.edu,
	matthew@wil.cx, schwidefsky@de.ibm.com
Subject: Re: fcntl: F_SETLEASE/F_RDLCK question
Date: Tue, 31 May 2005 16:53:50 +0200 (MEST)	[thread overview]
Message-ID: <32555.1117551230@www14.gmx.net> (raw)
In-Reply-To: 20050503231408.7c045648.sfr@canb.auug.org.au

Hello Stephen,

Sorry for the long delay in replying; other things intervened.

> On Tue, 3 May 2005 12:00:18 +0200 (MEST) "Michael Kerrisk"
> <mtk-lkml@gmx.net> 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 ++

  parent reply	other threads:[~2005-05-31 14:53 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-05-02  9:15 fcntl: F_SETLEASE/F_RDLCK question Heiko Carstens
2005-05-02 11:04 ` Stephen Rothwell
2005-05-02 12:10   ` Heiko Carstens
2005-05-03 10:00   ` Michael Kerrisk
2005-05-03 13:14     ` Stephen Rothwell
2005-05-03 13:55       ` William A.(Andy) Adamson
2005-05-03 13:59         ` Matthew Wilcox
2005-05-03 14:15           ` William A.(Andy) Adamson
2005-05-03 14:50             ` Michael Kerrisk
2005-05-03 16:21               ` William A.(Andy) Adamson
2005-05-03 16:36                 ` J. Bruce Fields
2005-05-31 14:53       ` Michael Kerrisk [this message]
2005-05-31 15:23         ` J. Bruce Fields
2005-05-31 15:41           ` Michael Kerrisk
2005-05-31 15:34       ` File leases and fork() Michael Kerrisk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32555.1117551230@www14.gmx.net \
    --to=michael.kerrisk@gmx.net \
    --cc=andros@citi.umich.edu \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew@wil.cx \
    --cc=mtk-lkml@gmx.net \
    --cc=schwidefsky@de.ibm.com \
    --cc=sfr@canb.auug.org.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.