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

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

[Christoph Hellwig]

On Thu, Sep 15, 2011 at 04:06:47PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> 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@linux.intel.com>
> ---
>  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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---

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

[Andi Kleen]

On Fri, Sep 16, 2011 at 11:48:15AM -0400, Christoph Hellwig wrote:
> On Thu, Sep 15, 2011 at 04:06:47PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > Introduced by 9a4327ca1f45f82edad7dc0a4e52ce9316e0950c
> 
> I think this should go to Chris/Linus ASAP.  But a slightly better
> patch description wouldn't hurt either.

Josef acked it earlier, but with a missing Chris the btrfs merge
pipeline seems to be disfunctional at the moment.

-Andi

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

[Jeff Liu]

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;
                }


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@linux.intel.com>
>>
>> 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@linux.intel.com>
>> ---
>>  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@vger.kernel.org
>> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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@oracle.com> 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@linux.intel.com>
>>> 
>>> 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@linux.intel.com>
>>> ---
>>> 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@vger.kernel.org
>>> 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@vger.kernel.org
>> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Andi Kleen]

> > 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. 

And also i_size must be always read with i_size_read()

Anyways clearly there's a problem in btrfs land with merging fixes in time.
Is anyone collecting patches while Chris is gone?

-Andi

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

[Jeff Liu]

Hi Andreas and Andi,

Thanks for your comments.

On 09/18/2011 09:46 AM, Andi Kleen wrote:

>>> 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. 

Per my tryout, except that, if the offset >= source file size, call
lseek(fd, offset, SEEK_DATA/SEEK_HOLE) against Btrfs will always return
the total file size rather than -ENXIO.  however, our desired result it
-ENXIO in this case, Am I right?

> 
> And also i_size must be always read with i_size_read()

Thanks for pointing this out!
Would you please kindly review the revised as below?

Signed-off-by: Jie Liu <jeff.liu@oracle.com>

---
 fs/btrfs/file.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index e7872e4..40c1ef3 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1813,6 +1813,11 @@ static loff_t btrfs_file_llseek(struct file
*file, loff_t offset, int origin)
 		goto out;
 	case SEEK_DATA:
 	case SEEK_HOLE:
+		if (offset >= i_size_read(inode)) {
+			mutex_unlock(&inode->i_mutex);
+			return -ENXIO;
+		}
+
 		ret = find_desired_extent(inode, &offset, origin);
 		if (ret) {
 			mutex_unlock(&inode->i_mutex);
@@ -1821,11 +1826,11 @@ static loff_t btrfs_file_llseek(struct file
*file, loff_t offset, int origin)
 	}

 	if (offset < 0 && !(file->f_mode & FMODE_UNSIGNED_OFFSET)) {
-		ret = -EINVAL;
+		offset = -EINVAL;
 		goto out;
 	}
 	if (offset > inode->i_sb->s_maxbytes) {
-		ret = -EINVAL;
+		offset = -EINVAL;
 		goto out;
 	}

-- 
1.7.4.1

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

[Marco Stornelli]

Il 18/09/2011 09:29, Jeff Liu ha scritto:
> Hi Andreas and Andi,
>
> Thanks for your comments.
>
> On 09/18/2011 09:46 AM, Andi Kleen wrote:
>
>>>> 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.
>
> Per my tryout, except that, if the offset>= source file size, call
> lseek(fd, offset, SEEK_DATA/SEEK_HOLE) against Btrfs will always return
> the total file size rather than -ENXIO.  however, our desired result it
> -ENXIO in this case, Am I right?
>

Yes, ENXIO should be the operation result.

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

[Jeff liu]

=D4=DA 2011-9-18=A3=AC=CF=C2=CE=E74:42=A3=AC Marco Stornelli =D0=B4=B5=C0=
=A3=BA

> Il 18/09/2011 09:29, Jeff Liu ha scritto:
>> Hi Andreas and Andi,
>>=20
>> Thanks for your comments.
>>=20
>> On 09/18/2011 09:46 AM, Andi Kleen wrote:
>>=20
>>>>> with an additional improvement if the offset is larger or equal t=
o the
>>>>> file size, return -ENXIO in directly:
>>>>>=20
>>>>>                if (offset>=3D inode->i_size) {
>>>>>                        mutex_unlock(&inode->i_mutex);
>>>>>                        return -ENXIO;
>>>>>                }
>>>>=20
>>>> Except that is wrong, because it would then be impossible to write=
 sparse files.
>>=20
>> Per my tryout, except that, if the offset>=3D source file size, call
>> lseek(fd, offset, SEEK_DATA/SEEK_HOLE) against Btrfs will always ret=
urn
>> the total file size rather than -ENXIO.  however, our desired result=
 it
>> -ENXIO in this case, Am I right?
>>=20
>=20
> Yes, ENXIO should be the operation result.

Thanks for your kind confirmation.


-Jeff

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

[Chris Mason]

Excerpts from Jeff liu's message of 2011-09-18 06:33:38 -0400:
>=20
> =E5=9C=A8 2011-9-18=EF=BC=8C=E4=B8=8B=E5=8D=884:42=EF=BC=8C Marco Sto=
rnelli =E5=86=99=E9=81=93=EF=BC=9A
>=20
> > Il 18/09/2011 09:29, Jeff Liu ha scritto:
> >> Hi Andreas and Andi,
> >>=20
> >> Thanks for your comments.
> >>=20
> >> On 09/18/2011 09:46 AM, Andi Kleen wrote:
> >>=20
> >>>>> with an additional improvement if the offset is larger or equal=
 to the
> >>>>> file size, return -ENXIO in directly:
> >>>>>=20
> >>>>>                if (offset>=3D inode->i_size) {
> >>>>>                        mutex_unlock(&inode->i_mutex);
> >>>>>                        return -ENXIO;
> >>>>>                }
> >>>>=20
> >>>> Except that is wrong, because it would then be impossible to wri=
te sparse files.
> >>=20
> >> Per my tryout, except that, if the offset>=3D source file size, ca=
ll
> >> lseek(fd, offset, SEEK_DATA/SEEK_HOLE) against Btrfs will always r=
eturn
> >> the total file size rather than -ENXIO.  however, our desired resu=
lt it
> >> -ENXIO in this case, Am I right?
> >>=20
> >=20
> > Yes, ENXIO should be the operation result.
>=20
> Thanks for your kind confirmation.

Thanks everyone, I've put Jeff's last version of this in my queue.

-chris

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

[Andi Kleen]

> Thanks everyone, I've put Jeff's last version of this in my queue.

Can you post the version you merged? The previous ones all had issues.

-Andi

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

[Chris Mason]

Excerpts from Andi Kleen's message of 2011-09-19 13:52:03 -0400:
> > Thanks everyone, I've put Jeff's last version of this in my queue.
> 
> Can you post the version you merged? The previous ones all had issues.

https://github.com/chrismason/linux/commit/48802c8ae2a9d618ec734a61283d645ad527e06c

This was the last one sent, I thought it combined all the fixes.


commit 48802c8ae2a9d618ec734a61283d645ad527e06c
Author: Jeff Liu <jeff.liu@oracle.com>
Date:   Sun Sep 18 10:34:02 2011 -0400

    BTRFS: Fix lseek return value for error
    
    The recent reworking of btrfs' lseek lead to incorrect
    values being returned.  This adds checks for seeking
    beyond EOF in SEEK_HOLE and makes sure the error
    values come back correct.
    
    Andi Kleen also sent in similar patches.
    
    Signed-off-by: Jie Liu <jeff.liu@oracle.com>
    Reported-by: Andi Kleen <ak@linux.intel.com>
    Signed-off-by: Chris Mason <chris.mason@oracle.com>

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 3c3abff..a381cd2 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1817,6 +1817,11 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin)
 		goto out;
 	case SEEK_DATA:
 	case SEEK_HOLE:
+		if (offset >= i_size_read(inode)) {
+			mutex_unlock(&inode->i_mutex);
+			return -ENXIO;
+		}
+
 		ret = find_desired_extent(inode, &offset, origin);
 		if (ret) {
 			mutex_unlock(&inode->i_mutex);
@@ -1825,11 +1830,11 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin)
 	}
 
 	if (offset < 0 && !(file->f_mode & FMODE_UNSIGNED_OFFSET)) {
-		ret = -EINVAL;
+		offset = -EINVAL;
 		goto out;
 	}
 	if (offset > inode->i_sb->s_maxbytes) {
-		ret = -EINVAL;
+		offset = -EINVAL;
 		goto out;
 	}
 

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

[Andi Kleen]

On Mon, Sep 19, 2011 at 03:30:02PM -0400, Chris Mason wrote:
> Excerpts from Andi Kleen's message of 2011-09-19 13:52:03 -0400:
> > > Thanks everyone, I've put Jeff's last version of this in my queue.
> > 
> > Can you post the version you merged? The previous ones all had issues.
> 
> https://github.com/chrismason/linux/commit/48802c8ae2a9d618ec734a61283d645ad527e06c
> 
> This was the last one sent, I thought it combined all the fixes.

Ok looks good, but it will be all obsolete once my patchkit lseek
get in (except for the SEEK_DATA/HOLE hunk)

-Andi

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

[Chris Mason]

Excerpts from Andi Kleen's message of 2011-09-19 15:59:52 -0400:
> On Mon, Sep 19, 2011 at 03:30:02PM -0400, Chris Mason wrote:
> > Excerpts from Andi Kleen's message of 2011-09-19 13:52:03 -0400:
> > > > Thanks everyone, I've put Jeff's last version of this in my queue.
> > > 
> > > Can you post the version you merged? The previous ones all had issues.
> > 
> > https://github.com/chrismason/linux/commit/48802c8ae2a9d618ec734a61283d645ad527e06c
> > 
> > This was the last one sent, I thought it combined all the fixes.
> 
> Ok looks good, but it will be all obsolete once my patchkit lseek
> get in (except for the SEEK_DATA/HOLE hunk)

Yeah, it's similar to yours except for the hole hunk.  Your gotos are
fine by me, whatever makes your patch cleaner.

-chris

Re: [PATCH 1/2] LSEEK: BTRFS: Avoid i_mutex for SEEK_{CUR,SET,END}

[Christoph Hellwig]

Andres, can you check with Chris that the btrfs changes made it to
his tree?  The core lseek changes from Andi are in mainline now, but
I think these bits are better off going through Chrises btrfs tree.

On Sat, Oct 01, 2011 at 10:49:27PM +0200, Andres Freund wrote:
> 
> Don't need the i_mutex for those cases, only for SEEK_HOLE/DATA.
> 
> Really-From: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Andres Freund <andres@anarazel.de>
> ---
>  fs/btrfs/file.c |   27 +++++++++++----------------
>  1 files changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 7a13337..5bc7116 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1809,24 +1809,19 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin)
>  	struct inode *inode = file->f_mapping->host;
>  	int ret;
>  
> +	if (origin != SEEK_DATA && origin != SEEK_HOLE)
> +		return generic_file_llseek(file, offset, origin);
> +
>  	mutex_lock(&inode->i_mutex);
> -	switch (origin) {
> -	case SEEK_END:
> -	case SEEK_CUR:
> -		offset = generic_file_llseek(file, offset, origin);
> -		goto out;
> -	case SEEK_DATA:
> -	case SEEK_HOLE:
> -		if (offset >= i_size_read(inode)) {
> -			mutex_unlock(&inode->i_mutex);
> -			return -ENXIO;
> -		}
> +	if (offset >= i_size_read(inode)) {
> +		mutex_unlock(&inode->i_mutex);
> +		return -ENXIO;
> +	}
>  
> -		ret = find_desired_extent(inode, &offset, origin);
> -		if (ret) {
> -			mutex_unlock(&inode->i_mutex);
> -			return ret;
> -		}
> +	ret = find_desired_extent(inode, &offset, origin);
> +	if (ret) {
> +		mutex_unlock(&inode->i_mutex);
> +		return ret;
>  	}
>  
>  	if (offset < 0 && !(file->f_mode & FMODE_UNSIGNED_OFFSET)) {
> -- 
> 1.7.6.409.ge7a85.dirty
> 
> --
> 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
---end quoted text---

Re: [PATCH 1/2] LSEEK: BTRFS: Avoid i_mutex for SEEK_{CUR,SET,END}

[Chris Mason]

On Wed, Nov 02, 2011 at 04:29:15AM -0400, Christoph Hellwig wrote:
> Andres, can you check with Chris that the btrfs changes made it to
> his tree?  The core lseek changes from Andi are in mainline now, but
> I think these bits are better off going through Chrises btrfs tree.

I'm pulling these in, thanks!

-chris

> 
> On Sat, Oct 01, 2011 at 10:49:27PM +0200, Andres Freund wrote:
> > 
> > Don't need the i_mutex for those cases, only for SEEK_HOLE/DATA.
> > 
> > Really-From: Andi Kleen <ak@linux.intel.com>
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> > Signed-off-by: Andres Freund <andres@anarazel.de>
> > ---
> >  fs/btrfs/file.c |   27 +++++++++++----------------
> >  1 files changed, 11 insertions(+), 16 deletions(-)
> > 
> > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> > index 7a13337..5bc7116 100644
> > --- a/fs/btrfs/file.c
> > +++ b/fs/btrfs/file.c
> > @@ -1809,24 +1809,19 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin)
> >  	struct inode *inode = file->f_mapping->host;
> >  	int ret;
> >  
> > +	if (origin != SEEK_DATA && origin != SEEK_HOLE)
> > +		return generic_file_llseek(file, offset, origin);
> > +
> >  	mutex_lock(&inode->i_mutex);
> > -	switch (origin) {
> > -	case SEEK_END:
> > -	case SEEK_CUR:
> > -		offset = generic_file_llseek(file, offset, origin);
> > -		goto out;
> > -	case SEEK_DATA:
> > -	case SEEK_HOLE:
> > -		if (offset >= i_size_read(inode)) {
> > -			mutex_unlock(&inode->i_mutex);
> > -			return -ENXIO;
> > -		}
> > +	if (offset >= i_size_read(inode)) {
> > +		mutex_unlock(&inode->i_mutex);
> > +		return -ENXIO;
> > +	}
> >  
> > -		ret = find_desired_extent(inode, &offset, origin);
> > -		if (ret) {
> > -			mutex_unlock(&inode->i_mutex);
> > -			return ret;
> > -		}
> > +	ret = find_desired_extent(inode, &offset, origin);
> > +	if (ret) {
> > +		mutex_unlock(&inode->i_mutex);
> > +		return ret;
> >  	}
> >  
> >  	if (offset < 0 && !(file->f_mode & FMODE_UNSIGNED_OFFSET)) {
> > -- 
> > 1.7.6.409.ge7a85.dirty
> > 
> > --
> > 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
> ---end quoted text---

Re: [PATCH 1/2] LSEEK: BTRFS: Avoid i_mutex for SEEK_{CUR,SET,END}

[Andres Freund]

On Saturday, November 05, 2011 04:27:49 PM Chris Mason wrote:
> On Wed, Nov 02, 2011 at 04:29:15AM -0400, Christoph Hellwig wrote:
> > Andres, can you check with Chris that the btrfs changes made it to
> > his tree?  The core lseek changes from Andi are in mainline now, but
> > I think these bits are better off going through Chrises btrfs tree.
> 
> I'm pulling these in, thanks!
I just stumbled uppon this email when opening up my email client which 
triggered me to recheck the issue - do you have pulled those? A very quick 
look didn't intoicate so.

Andres
> > On Sat, Oct 01, 2011 at 10:49:27PM +0200, Andres Freund wrote:
> > > Don't need the i_mutex for those cases, only for SEEK_HOLE/DATA.
> > > 
> > > Really-From: Andi Kleen <ak@linux.intel.com>
> > > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> > > Signed-off-by: Andres Freund <andres@anarazel.de>
> > > ---
> > > 
> > >  fs/btrfs/file.c |   27 +++++++++++----------------
> > >  1 files changed, 11 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> > > index 7a13337..5bc7116 100644
> > > --- a/fs/btrfs/file.c
> > > +++ b/fs/btrfs/file.c
> > > @@ -1809,24 +1809,19 @@ static loff_t btrfs_file_llseek(struct file
> > > *file, loff_t offset, int origin)
> > > 
> > >  	struct inode *inode = file->f_mapping->host;
> > >  	int ret;
> > > 
> > > +	if (origin != SEEK_DATA && origin != SEEK_HOLE)
> > > +		return generic_file_llseek(file, offset, origin);
> > > +
> > > 
> > >  	mutex_lock(&inode->i_mutex);
> > > 
> > > -	switch (origin) {
> > > -	case SEEK_END:
> > > -	case SEEK_CUR:
> > > -		offset = generic_file_llseek(file, offset, origin);
> > > -		goto out;
> > > -	case SEEK_DATA:
> > > -	case SEEK_HOLE:
> > > -		if (offset >= i_size_read(inode)) {
> > > -			mutex_unlock(&inode->i_mutex);
> > > -			return -ENXIO;
> > > -		}
> > > +	if (offset >= i_size_read(inode)) {
> > > +		mutex_unlock(&inode->i_mutex);
> > > +		return -ENXIO;
> > > +	}
> > > 
> > > -		ret = find_desired_extent(inode, &offset, origin);
> > > -		if (ret) {
> > > -			mutex_unlock(&inode->i_mutex);
> > > -			return ret;
> > > -		}
> > > +	ret = find_desired_extent(inode, &offset, origin);
> > > +	if (ret) {
> > > +		mutex_unlock(&inode->i_mutex);
> > > +		return ret;
> > > 
> > >  	}
> > >  	
> > >  	if (offset < 0 && !(file->f_mode & FMODE_UNSIGNED_OFFSET)) {
> > 
> > ---end quoted text---
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/