Re: ceph, cifs, nfs, fuse: boolean and / or confusion

From: Andrew Morton
Date: Mon Dec 12 2011 - 18:29:00 EST


On Tue, 13 Dec 2011 00:06:36 +0100
roel <roel.kluin@xxxxxxxxx> wrote:

> The test not SEEK_CUR or not SEEK_SET always evaluates to true
>
> Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx>
> ---
> fs/ceph/file.c | 2 +-
> fs/cifs/cifsfs.c | 2 +-
> fs/fuse/file.c | 2 +-
> fs/nfs/file.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index ce549d3..4d61a66 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -797,7 +797,7 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int origin)
>
> mutex_lock(&inode->i_mutex);
> __ceph_do_pending_vmtruncate(inode);
> - if (origin != SEEK_CUR || origin != SEEK_SET) {
> + if (origin != SEEK_CUR && origin != SEEK_SET) {
> ret = ceph_do_getattr(inode, CEPH_STAT_CAP_SIZE);
> if (ret < 0) {
> offset = ret;
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 8f1fe32..f8351c2 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -703,7 +703,7 @@ static loff_t cifs_llseek(struct file *file, loff_t offset, int origin)
> * origin == SEEK_END || SEEK_DATA || SEEK_HOLE => we must revalidate
> * the cached file length
> */
> - if (origin != SEEK_SET || origin != SEEK_CUR) {
> + if (origin != SEEK_SET && origin != SEEK_CUR) {
> int rc;
> struct inode *inode = file->f_path.dentry->d_inode;
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 594f07a..19029e9 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1556,7 +1556,7 @@ static loff_t fuse_file_llseek(struct file *file, loff_t offset, int origin)
> struct inode *inode = file->f_path.dentry->d_inode;
>
> mutex_lock(&inode->i_mutex);
> - if (origin != SEEK_CUR || origin != SEEK_SET) {
> + if (origin != SEEK_CUR && origin != SEEK_SET) {
> retval = fuse_update_attributes(inode, NULL, file, NULL);
> if (retval)
> goto exit;
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index eca56d4..606ef0f 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -147,7 +147,7 @@ static loff_t nfs_file_llseek(struct file *filp, loff_t offset, int origin)
> * origin == SEEK_END || SEEK_DATA || SEEK_HOLE => we must revalidate
> * the cached file length
> */
> - if (origin != SEEK_SET || origin != SEEK_CUR) {
> + if (origin != SEEK_SET && origin != SEEK_CUR) {
> struct inode *inode = filp->f_mapping->host;
>
> int retval = nfs_revalidate_file_size(inode, filp);

This fix will cause changed runtime behaviour, such as NFS no longer
running nfs_revalidate_file_size() for all seek modes.

So I think it should be reviewed, tested and merged by the various fs
maintainers. Splitting it into four patches would help that process,
but they could independently treat it as a bug report, too.

Meanwhile, I'll put this in my tree so I can keep an eye on people ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/