diff mbox series

[01/23] ocfs2: Handle a symlink read error correctly

Message ID 20241205171653.3179945-2-willy@infradead.org (mailing list archive)
State New
Headers show
Series Convert ocfs2 to use folios | expand

Commit Message

Matthew Wilcox (Oracle) Dec. 5, 2024, 5:16 p.m. UTC
If we can't read the buffer, be sure to unlock the page before
returning.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: stable@vger.kernel.org
---
 fs/ocfs2/symlink.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Joseph Qi Dec. 14, 2024, 12:03 p.m. UTC | #1
On 2024/12/6 01:16, Matthew Wilcox (Oracle) wrote:
> If we can't read the buffer, be sure to unlock the page before
> returning.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Cc: stable@vger.kernel.org
> ---
>  fs/ocfs2/symlink.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/symlink.c b/fs/ocfs2/symlink.c
> index d4c5fdcfa1e4..f5cf2255dc09 100644
> --- a/fs/ocfs2/symlink.c
> +++ b/fs/ocfs2/symlink.c
> @@ -65,7 +65,7 @@ static int ocfs2_fast_symlink_read_folio(struct file *f, struct folio *folio)
>  

Better to move calling ocfs2_read_inode_block() here.

Thanks,
Joseph

>  	if (status < 0) {
>  		mlog_errno(status);
> -		return status;
> +		goto out;
>  	}
>  
>  	fe = (struct ocfs2_dinode *) bh->b_data;
> @@ -76,9 +76,10 @@ static int ocfs2_fast_symlink_read_folio(struct file *f, struct folio *folio)
>  	memcpy(kaddr, link, len + 1);
>  	kunmap_atomic(kaddr);
>  	SetPageUptodate(page);
> +out:
>  	unlock_page(page);
>  	brelse(bh);
> -	return 0;
> +	return status;
>  }
>  
>  const struct address_space_operations ocfs2_fast_symlink_aops = {
Matthew Wilcox (Oracle) Dec. 14, 2024, 3:20 p.m. UTC | #2
On Sat, Dec 14, 2024 at 08:03:30PM +0800, Joseph Qi wrote:
> On 2024/12/6 01:16, Matthew Wilcox (Oracle) wrote:
> > If we can't read the buffer, be sure to unlock the page before
> > returning.
> > 
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > Cc: stable@vger.kernel.org
> > ---
> >  fs/ocfs2/symlink.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/ocfs2/symlink.c b/fs/ocfs2/symlink.c
> > index d4c5fdcfa1e4..f5cf2255dc09 100644
> > --- a/fs/ocfs2/symlink.c
> > +++ b/fs/ocfs2/symlink.c
> > @@ -65,7 +65,7 @@ static int ocfs2_fast_symlink_read_folio(struct file *f, struct folio *folio)
> >  
> 
> Better to move calling ocfs2_read_inode_block() here.

Hm?  This is a bugfix; it should be as small as reasonable.  If you want
the code to be moved around, that should be left to a later patch.

> Thanks,
> Joseph
> 
> >  	if (status < 0) {
> >  		mlog_errno(status);
> > -		return status;
> > +		goto out;
> >  	}
> >  
> >  	fe = (struct ocfs2_dinode *) bh->b_data;
> > @@ -76,9 +76,10 @@ static int ocfs2_fast_symlink_read_folio(struct file *f, struct folio *folio)
> >  	memcpy(kaddr, link, len + 1);
> >  	kunmap_atomic(kaddr);
> >  	SetPageUptodate(page);
> > +out:
> >  	unlock_page(page);
> >  	brelse(bh);
> > -	return 0;
> > +	return status;
> >  }
> >  
> >  const struct address_space_operations ocfs2_fast_symlink_aops = {
>
Joseph Qi Dec. 14, 2024, 3:45 p.m. UTC | #3
On 2024/12/14 23:20, Matthew Wilcox wrote:
> On Sat, Dec 14, 2024 at 08:03:30PM +0800, Joseph Qi wrote:
>> On 2024/12/6 01:16, Matthew Wilcox (Oracle) wrote:
>>> If we can't read the buffer, be sure to unlock the page before
>>> returning.
>>>
>>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>>> Cc: stable@vger.kernel.org
>>> ---
>>>  fs/ocfs2/symlink.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/ocfs2/symlink.c b/fs/ocfs2/symlink.c
>>> index d4c5fdcfa1e4..f5cf2255dc09 100644
>>> --- a/fs/ocfs2/symlink.c
>>> +++ b/fs/ocfs2/symlink.c
>>> @@ -65,7 +65,7 @@ static int ocfs2_fast_symlink_read_folio(struct file *f, struct folio *folio)
>>>  
>>
>> Better to move calling ocfs2_read_inode_block() here.
> 
> Hm?  This is a bugfix; it should be as small as reasonable.  If you want
> the code to be moved around, that should be left to a later patch.
>

Well, either way is fine to me.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
diff mbox series

Patch

diff --git a/fs/ocfs2/symlink.c b/fs/ocfs2/symlink.c
index d4c5fdcfa1e4..f5cf2255dc09 100644
--- a/fs/ocfs2/symlink.c
+++ b/fs/ocfs2/symlink.c
@@ -65,7 +65,7 @@  static int ocfs2_fast_symlink_read_folio(struct file *f, struct folio *folio)
 
 	if (status < 0) {
 		mlog_errno(status);
-		return status;
+		goto out;
 	}
 
 	fe = (struct ocfs2_dinode *) bh->b_data;
@@ -76,9 +76,10 @@  static int ocfs2_fast_symlink_read_folio(struct file *f, struct folio *folio)
 	memcpy(kaddr, link, len + 1);
 	kunmap_atomic(kaddr);
 	SetPageUptodate(page);
+out:
 	unlock_page(page);
 	brelse(bh);
-	return 0;
+	return status;
 }
 
 const struct address_space_operations ocfs2_fast_symlink_aops = {