(unknown)

(unknown)

[Jason Baron]

Hi,

If one tries to do a fcntl(fd, F_SETLEASE, F_RDLCK) on a file that is open 
for writing, the error returned is always -EAGAIN. This seems like the 
wrong error return for the case where 'fd' points to a file_struct that 
has FMODE_WRITE set. No matter how many times one calls the fcntl on that 
'fd' it will fail. Therefore, i think the return value should be -EINVAL 
in this case. The patch below implements this behavior. Some more context 
for this issue can be found at: http://lkml.org/lkml/2005/5/2/20. Patch is 
based on a suggestion from Jeff Layton.

thanks,

-Jason

Signed-off-by: Jason Baron <jbaron@redhat.com>


--- linux-2.6/fs/locks.c.bak	2006-06-13 10:36:58.000000000 -0400
+++ linux-2.6/fs/locks.c	2006-06-13 10:41:19.000000000 -0400
@@ -1338,8 +1338,11 @@
 	lease = *flp;
 
 	error = -EAGAIN;
-	if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0))
+	if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0)) {
+		if (filp->f_mode & FMODE_WRITE)
+			error = -EINVAL;
 		goto out;
+	}
 	if ((arg == F_WRLCK)
 	    && ((atomic_read(&dentry->d_count) > 1)
 		|| (atomic_read(&inode->i_count) > 1)))

(unknown),

[William A.(Andy) Adamson]

hi.

from man fcntl

     EACCES or EAGAIN
              Operation  is  prohibited by locks held by other processes.  Or,
              operation is prohibited because the file has been  memory-mapped
              by another process.

a process with file open for writing is essentially holding a lock WRT 
allowing a read lease, so i think EAGAIN is the appropriate error.


again from man fcntl

      EINVAL For F_DUPFD, arg is negative or  is  greater  than  the  maximum
              allowable  value.   For F_SETSIG, arg is not an allowable signal
              number.

the arguments to the fcntl setlease call are correct, not invalid, so this is 
the wrong error.

-->Andy

> Hi,
> 
> If one tries to do a fcntl(fd, F_SETLEASE, F_RDLCK) on a file that is open 
> for writing, the error returned is always -EAGAIN. This seems like the 
> wrong error return for the case where 'fd' points to a file_struct that 
> has FMODE_WRITE set. No matter how many times one calls the fcntl on that 
> 'fd' it will fail. Therefore, i think the return value should be -EINVAL 
> in this case. The patch below implements this behavior. Some more context 
> for this issue can be found at: http://lkml.org/lkml/2005/5/2/20. Patch is 
> based on a suggestion from Jeff Layton.
> 
> thanks,
> 
> -Jason
> 
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> 
> 
> --- linux-2.6/fs/locks.c.bak	2006-06-13 10:36:58.000000000 -0400
> +++ linux-2.6/fs/locks.c	2006-06-13 10:41:19.000000000 -0400
> @@ -1338,8 +1338,11 @@
>  	lease = *flp;
>  
>  	error = -EAGAIN;
> -	if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0))
> +	if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0)) {
> +		if (filp->f_mode & FMODE_WRITE)
> +			error = -EINVAL;
>  		goto out;
> +	}
>  	if ((arg == F_WRLCK)
>  	    && ((atomic_read(&dentry->d_count) > 1)
>  		|| (atomic_read(&inode->i_count) > 1)))
> -
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

setlease return code

[Jason Baron]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1099 bytes --]




On Tue, 13 Jun 2006, William A.(Andy) Adamson wrote:

> hi.
> 
> from man fcntl
> 
>      EACCES or EAGAIN
>               Operation  is  prohibited by locks held by other processes.  Or,
>               operation is prohibited because the file has been  memory-mapped
>               by another process.
> 
> a process with file open for writing is essentially holding a lock WRT 
> allowing a read lease, so i think EAGAIN is the appropriate error.
> 
> 

its not prohibited in this case by another process...

> again from man fcntl
> 
>       EINVAL For F_DUPFD, arg is negative or  is  greater  than  the  maximum
>               allowable  value.   For F_SETSIG, arg is not an allowable signal
>               number.
> 
> the arguments to the fcntl setlease call are correct, not invalid, so this is 
> the wrong error.
> 
> -->Andy


does EBADF make more sense then?

from the manpage:

    EBADF  
	fd is not an open file descriptor, or the  command  was  F_SETLK  or
        F_SETLKW  and  the  file descriptor open mode doesn’t match with the
        type of lock requested.


-Jason

Re: setlease return code

[William A.(Andy) Adamson]

> 
> 
> 
> On Tue, 13 Jun 2006, William A.(Andy) Adamson wrote:
> 
> > hi.
> > 
> > from man fcntl
> > 
> >      EACCES or EAGAIN
> >               Operation  is  prohibited by locks held by other processes.  Or,
> >               operation is prohibited because the file has been  memory-mapped
> >               by another process.
> > 
> > a process with file open for writing is essentially holding a lock WRT 
> > allowing a read lease, so i think EAGAIN is the appropriate error.
> > 
> > 
> 
> its not prohibited in this case by another process...

sure it is. it's prohibited by the process holding the open(WRITE). EAGAIN 
says try again later because a lock is held - which it is.

(actually process is a confusing word - should be 'lock owner'. it's only a 
process in the local case. in the NFSv4 or Samba case, it's a pointer to a 
struct describing a remote entity)

> 
> > again from man fcntl
> > 
> >       EINVAL For F_DUPFD, arg is negative or  is  greater  than  the  maximum
> >               allowable  value.   For F_SETSIG, arg is not an allowable signal
> >               number.
> > 
> > the arguments to the fcntl setlease call are correct, not invalid, so this is 
> > the wrong error.
> > 
> > -->Andy
> 
> 
> does EBADF make more sense then?
> 
> from the manpage:
> 
>     EBADF  
> 	fd is not an open file descriptor, or the  command  was  F_SETLK  or
>         F_SETLKW  and  the  file descriptor open mode doesn’t match with the
>         type of lock requested.

no. there is nothing wrong with the input args. fd is an open file descriptor, 
and the mode of the setlease request matches the mode of the open.

why is EAGAIN a problem?

-->Andy

> 
> 
> -Jason
> 


-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: setlease return code

[Jason Baron]

On Tue, 13 Jun 2006, William A.(Andy) Adamson wrote:

> no. there is nothing wrong with the input args. fd is an open file descriptor, 
> and the mode of the setlease request matches the mode of the open.
> 
> why is EAGAIN a problem?
> 

its not a major issue, i just thought EAGAIN made little sense in this 
case since the 'fd' we are using is already opened for write, and thus no 
matter how many times a read lock is requested for the 'fd' it will never 
succeed.