From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754737AbdCLFXL (ORCPT ); Sun, 12 Mar 2017 00:23:11 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:34032 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750998AbdCLFXC (ORCPT ); Sun, 12 Mar 2017 00:23:02 -0500 Date: Sun, 12 Mar 2017 06:22:47 +0100 From: Greg Kroah-Hartman To: Ben Hutchings Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Theodore Tso Subject: Re: [PATCH 4.4 48/91] ext4: fix inline data error paths Message-ID: <20170312052247.GA22750@kroah.com> References: <20170310083900.730556986@linuxfoundation.org> <20170310083903.182707143@linuxfoundation.org> <1489164532.2593.17.camel@decadent.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1489164532.2593.17.camel@decadent.org.uk> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Mar 10, 2017 at 04:48:52PM +0000, Ben Hutchings wrote: > On Fri, 2017-03-10 at 10:08 +0100, Greg Kroah-Hartman wrote: > > 4.4-stable review patch.  If anyone has any objections, please let me > > know. > > > > ------------------ > > > > From: Theodore Ts'o > > > > commit eb5efbcb762aee4b454b04f7115f73ccbcf8f0ef upstream. > > > > The write_end() function must always unlock the page and drop its ref > > count, even on an error. > > This looks like a theoretical rather than a real issue, because I can't > see how ext4_write_inline_data_end() ever returns an error code. I'll leave that up to Ted to justify :) > > Signed-off-by: Theodore Ts'o > > Signed-off-by: Greg Kroah-Hartman > > > > --- > >  fs/ext4/inline.c |    9 ++++++++- > >  fs/ext4/inode.c  |   20 +++++++++++++++----- > >  2 files changed, 23 insertions(+), 6 deletions(-) > > > > --- a/fs/ext4/inline.c > > +++ b/fs/ext4/inline.c > > @@ -933,8 +933,15 @@ int ext4_da_write_inline_data_end(struct > > >     struct page *page) > >  { > >   int i_size_changed = 0; > > + int ret; > >   > > - copied = ext4_write_inline_data_end(inode, pos, len, copied, page); > > + ret = ext4_write_inline_data_end(inode, pos, len, copied, page); > > + if (ret < 0) { > > + unlock_page(page); > > + put_page(page); > [...] > > For 4.4 each put_page() should ideally be changed to > page_cache_release(). It makes no practical difference but would be > consistent with other paths. As it's still the same logic, I'd prefer to stick to what newer kernels do if at all possible. thanks, greg k-h From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linuxfoundation.org ([140.211.169.12]:34032 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750998AbdCLFXC (ORCPT ); Sun, 12 Mar 2017 00:23:02 -0500 Date: Sun, 12 Mar 2017 06:22:47 +0100 From: Greg Kroah-Hartman To: Ben Hutchings Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Theodore Tso Subject: Re: [PATCH 4.4 48/91] ext4: fix inline data error paths Message-ID: <20170312052247.GA22750@kroah.com> References: <20170310083900.730556986@linuxfoundation.org> <20170310083903.182707143@linuxfoundation.org> <1489164532.2593.17.camel@decadent.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1489164532.2593.17.camel@decadent.org.uk> Sender: stable-owner@vger.kernel.org List-ID: On Fri, Mar 10, 2017 at 04:48:52PM +0000, Ben Hutchings wrote: > On Fri, 2017-03-10 at 10:08 +0100, Greg Kroah-Hartman wrote: > > 4.4-stable review patch.��If anyone has any objections, please let me > > know. > > > > ------------------ > > > > From: Theodore Ts'o > > > > commit eb5efbcb762aee4b454b04f7115f73ccbcf8f0ef upstream. > > > > The write_end() function must always unlock the page and drop its ref > > count, even on an error. > > This looks like a theoretical rather than a real issue, because I can't > see how ext4_write_inline_data_end() ever returns an error code. I'll leave that up to Ted to justify :) > > Signed-off-by: Theodore Ts'o > > Signed-off-by: Greg Kroah-Hartman > > > > --- > > �fs/ext4/inline.c |����9 ++++++++- > > �fs/ext4/inode.c��|���20 +++++++++++++++----- > > �2 files changed, 23 insertions(+), 6 deletions(-) > > > > --- a/fs/ext4/inline.c > > +++ b/fs/ext4/inline.c > > @@ -933,8 +933,15 @@ int ext4_da_write_inline_data_end(struct > > > � ��struct page *page) > > �{ > > � int i_size_changed = 0; > > + int ret; > > � > > - copied = ext4_write_inline_data_end(inode, pos, len, copied, page); > > + ret = ext4_write_inline_data_end(inode, pos, len, copied, page); > > + if (ret < 0) { > > + unlock_page(page); > > + put_page(page); > [...] > > For 4.4 each put_page() should ideally be changed to > page_cache_release(). It makes no practical difference but would be > consistent with other paths. As it's still the same logic, I'd prefer to stick to what newer kernels do if at all possible. thanks, greg k-h