Re: [PATCH 1/7] BTRFS: Fix lseek return value for error

[Andreas Dilger]

On 2011-09-17, at 12:10 AM, Jeff Liu <jeff.liu@xxxxxxxxxx> wrote:
> I once posted a similar patch for this issue which can be found at:
> http://www.spinics.net/lists/linux-btrfs/msg12169.html
> 
> with an additional improvement if the offset is larger or equal to the
> file size, return -ENXIO in directly:
> 
>                if (offset >= inode->i_size) {
>                        mutex_unlock(&inode->i_mutex);
>                        return -ENXIO;
>                }

Except that is wrong, because it would then be impossible to write sparse files. 

> Thanks,
> -Jeff
> On 09/16/2011 11:48 PM, Christoph Hellwig wrote:
> 
>> On Thu, Sep 15, 2011 at 04:06:47PM -0700, Andi Kleen wrote:
>>> From: Andi Kleen <ak@xxxxxxxxxxxxxxx>
>>> 
>>> Introduced by 9a4327ca1f45f82edad7dc0a4e52ce9316e0950c
>> 
>> I think this should go to Chris/Linus ASAP.  But a slightly better
>> patch description wouldn't hurt either.
>> 
>> Also any reason you captialize BTRFS?
>> 
>>> 
>>> Signed-off-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
>>> ---
>>> fs/btrfs/file.c |   13 +++++++------
>>> 1 files changed, 7 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
>>> index 3c3abff..7ec0a24 100644
>>> --- a/fs/btrfs/file.c
>>> +++ b/fs/btrfs/file.c
>>> @@ -1818,19 +1818,17 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin)
>>>    case SEEK_DATA:
>>>    case SEEK_HOLE:
>>>        ret = find_desired_extent(inode, &offset, origin);
>>> -        if (ret) {
>>> -            mutex_unlock(&inode->i_mutex);
>>> -            return ret;
>>> -        }
>>> +        if (ret)
>>> +            goto error;
>>>    }
>>> 
>>>    if (offset < 0 && !(file->f_mode & FMODE_UNSIGNED_OFFSET)) {
>>>        ret = -EINVAL;
>>> -        goto out;
>>> +        goto error;
>>>    }
>>>    if (offset > inode->i_sb->s_maxbytes) {
>>>        ret = -EINVAL;
>>> -        goto out;
>>> +        goto error;
>>>    }
>>> 
>>>    /* Special lock needed here? */
>>> @@ -1841,6 +1839,9 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin)
>>> out:
>>>    mutex_unlock(&inode->i_mutex);
>>>    return offset;
>>> +error:
>>> +    mutex_unlock(&inode->i_mutex);
>>> +    return ret;
>>> }
>>> 
>>> const struct file_operations btrfs_file_operations = {
>>> -- 
>>> 1.7.4.4
>>> 
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> ---end quoted text---
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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/