From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754850Ab0GGAoW (ORCPT ); Tue, 6 Jul 2010 20:44:22 -0400 Received: from rcsinet10.oracle.com ([148.87.113.121]:37556 "EHLO rcsinet10.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753284Ab0GGAoV (ORCPT ); Tue, 6 Jul 2010 20:44:21 -0400 Message-ID: <4C33CD8D.4070205@oracle.com> Date: Wed, 07 Jul 2010 08:42:53 +0800 From: Tao Ma User-Agent: Thunderbird 2.0.0.24 (Windows/20100228) MIME-Version: 1.0 To: Tao Ma , Dave Chinner , Linus Torvalds , Linux Kernel , ocfs2-devel@oss.oracle.com, Dave Chinner , Christoph Hellwig , Mark Fasheh , Joel Becker Subject: Re: [PATCH 1/2] ocfs2: Zero the tail cluster when extending past i_size v2 References: <20100629005403.GC24343@mail.oracle.com> <20100629015615.GZ6590@dastard> <20100629020420.GE24343@mail.oracle.com> <20100629022757.GA6590@dastard> <20100629071817.GA4150@mail.oracle.com> <20100702224912.GC5800@mail.oracle.com> <20100703213219.GB21262@mail.oracle.com> <4C3156D0.5090001@oracle.com> <20100706071741.GF13023@mail.oracle.com> <4C32E152.1090902@oracle.com> <20100706115827.GG13023@mail.oracle.com> In-Reply-To: <20100706115827.GG13023@mail.oracle.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: acsmt353.oracle.com [141.146.40.153] X-Auth-Type: Internal IP X-CT-RefId: str=0001.0A090203.4C33CDBF.00A8:SCFMA4539814,ss=1,fgs=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Joel Becker wrote: > On Tue, Jul 06, 2010 at 03:54:58PM +0800, Tao Ma wrote: > >> On 07/06/2010 03:17 PM, Joel Becker wrote: >> >>>>> + /* Is there a cluster to zero? */ >>>>> + if (!p_cpos) >>>>> + goto out; >>>>> >>>> For unwritten extent, we also need to clear the pages? If yes, the >>>> solution doesn't complete if we have 2 unwritten extent, one >>>> contains i_size while one passes i_size. Here we only clear the >>>> pages for the 1st unwritten extent and leave the 2nd one untouched. >>>> >>> We probably don't need to zero unwritten extents. We cannot >>> have an extent past i_size, can we? >>> >> we can. AFAICS, ocfs2_change_file_space will allocate unwritten >> extents and does't change i_size. >> > > Oh, you're right. We need to walk the entire extent range > between i_size and pos and figure out what needs CoW. This needs to > happen no matter what. > Actually we can only have unwritten extents after i_size and it shouldn't hurt you in this case. So do we really need to CoW all the unwritten extents? All I want to say is that since they are unwritten, they should also mean 'zero' for the user space. So can we just need to skip clearing pages if i_size is in an unwritten extent? > >>> But you dropped the check for pos_blkno alignment. >>> Unconditionally adding the +1 doesn't seem like a good idea. >>> >> You can add it as you wish. >> I just thought that you add one more extra cluster if pos_blkno >> isn't aligned so as to zero blocks in [pos_cpos_start_block, >> pos_blkno). >> But As I said in the comments, you will soon write pos_blkno(it also >> needs to be CoW since it is within this refcounted extent), so if we >> can CoW it out now, maybe we have a chance to not call >> ocfs2_refcount_cow later. >> > > I'd much rather let the write handle its own contiguousness. If > we get lucky, that CoW melds with our CoW. If we don't get lucky, isn't > it better to have the newly changed area be fully contiguous rather than > have the first extent of it not be and then the remaining extents be? > fair enough. Regards, Tao From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tao Ma Date: Wed, 07 Jul 2010 08:42:53 +0800 Subject: [Ocfs2-devel] [PATCH 1/2] ocfs2: Zero the tail cluster when extending past i_size v2 In-Reply-To: <20100706115827.GG13023@mail.oracle.com> References: <20100629005403.GC24343@mail.oracle.com> <20100629015615.GZ6590@dastard> <20100629020420.GE24343@mail.oracle.com> <20100629022757.GA6590@dastard> <20100629071817.GA4150@mail.oracle.com> <20100702224912.GC5800@mail.oracle.com> <20100703213219.GB21262@mail.oracle.com> <4C3156D0.5090001@oracle.com> <20100706071741.GF13023@mail.oracle.com> <4C32E152.1090902@oracle.com> <20100706115827.GG13023@mail.oracle.com> Message-ID: <4C33CD8D.4070205@oracle.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Tao Ma , Dave Chinner , Linus Torvalds , Linux Kernel , ocfs2-devel@oss.oracle.com, Dave Chinner , Christoph Hellwig , Mark Fasheh , Joel Becker Joel Becker wrote: > On Tue, Jul 06, 2010 at 03:54:58PM +0800, Tao Ma wrote: > >> On 07/06/2010 03:17 PM, Joel Becker wrote: >> >>>>> + /* Is there a cluster to zero? */ >>>>> + if (!p_cpos) >>>>> + goto out; >>>>> >>>> For unwritten extent, we also need to clear the pages? If yes, the >>>> solution doesn't complete if we have 2 unwritten extent, one >>>> contains i_size while one passes i_size. Here we only clear the >>>> pages for the 1st unwritten extent and leave the 2nd one untouched. >>>> >>> We probably don't need to zero unwritten extents. We cannot >>> have an extent past i_size, can we? >>> >> we can. AFAICS, ocfs2_change_file_space will allocate unwritten >> extents and does't change i_size. >> > > Oh, you're right. We need to walk the entire extent range > between i_size and pos and figure out what needs CoW. This needs to > happen no matter what. > Actually we can only have unwritten extents after i_size and it shouldn't hurt you in this case. So do we really need to CoW all the unwritten extents? All I want to say is that since they are unwritten, they should also mean 'zero' for the user space. So can we just need to skip clearing pages if i_size is in an unwritten extent? > >>> But you dropped the check for pos_blkno alignment. >>> Unconditionally adding the +1 doesn't seem like a good idea. >>> >> You can add it as you wish. >> I just thought that you add one more extra cluster if pos_blkno >> isn't aligned so as to zero blocks in [pos_cpos_start_block, >> pos_blkno). >> But As I said in the comments, you will soon write pos_blkno(it also >> needs to be CoW since it is within this refcounted extent), so if we >> can CoW it out now, maybe we have a chance to not call >> ocfs2_refcount_cow later. >> > > I'd much rather let the write handle its own contiguousness. If > we get lucky, that CoW melds with our CoW. If we don't get lucky, isn't > it better to have the newly changed area be fully contiguous rather than > have the first extent of it not be and then the remaining extents be? > fair enough. Regards, Tao