Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752824Ab3EBHnT (ORCPT ); Thu, 2 May 2013 03:43:19 -0400 Received: from mail.parknet.co.jp ([210.171.160.6]:46706 "EHLO mail.parknet.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751121Ab3EBHnQ (ORCPT ); Thu, 2 May 2013 03:43:16 -0400 From: OGAWA Hirofumi To: Namjae Jeon Cc: akpm@linux-foundation.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Namjae Jeon , Amit Sahrawat Subject: Re: [PATCH RESEND v5] fat: editions to support fat_fallocate References: <1367107703-2665-1-git-send-email-linkinjeon@gmail.com> <87ppxd4ddm.fsf@devron.myhome.or.jp> <87bo8v42wx.fsf@devron.myhome.or.jp> <87vc722cdu.fsf@devron.myhome.or.jp> <87r4hp3kax.fsf@devron.myhome.or.jp> Date: Thu, 02 May 2013 16:43:13 +0900 In-Reply-To: <87r4hp3kax.fsf@devron.myhome.or.jp> (OGAWA Hirofumi's message of "Thu, 02 May 2013 16:36:22 +0900") Message-ID: <87k3nh3jzi.fsf@devron.myhome.or.jp> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=iso-2022-jp Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3415 Lines: 86 OGAWA Hirofumi writes: > Namjae Jeon writes: > >>> Then, per-file discard fallocate space sounds like wrong. fallocate >>> space probably is inode attribute. >> Since, our preallocation will not be persistent after umount. So, we >> need to free up the space at some point. >> If we consider for normal pre-allocation in ext4, in that case also >> the blocks are removed in ext4_release_file when the last writer >> closes the file. >> >> ext4_release_file() >> { >> ... >> /* if we are the last writer on the inode, drop the block reservation */ >> if ((filp->f_mode & FMODE_WRITE) && >> (atomic_read(&inode->i_writecount) == 1) && >> !EXT4_I(inode)->i_reserved_data_blocks) >> { >> down_write(&EXT4_I(inode)->i_data_sem); >> ext4_discard_preallocations(inode); >> up_write(&EXT4_I(inode)->i_data_sem); >> } >> >> So, we will need to have this per file . May be the condition for >> checking is wrong which can be correct but the correctness points >> should be same. We can give a thought on using "i_writecount" for >> controlling the parallel write in FAT also. >> how do you think ? > > AFAIK, preallocation != fallocate. ext*'s preallocation was there at > before fallocation to optimize block allocation for user data blocks. > >>>>> I know. Question is, why do we need to initialize twice. >>>>> >>>>> 1) zeroed for uninitialized area, 2) then copy user data area. We need >>>>> only either, right? This seems to be doing both for all fallocated area. >>>> We did not initialize twice. We are using the ‘pos’ as the attribute >>>> to define zeroing length in case of pre-allocation. >>>> Zeroing out occurs till the ‘pos’ while actual write occur after ‘pos’. >>>> If we file size is 100KB and we pre-allocated till 1MB. Next if we try >>>> to write at 500KB, >>>> Then zeroing out will occur only for 100KB->500KB, after that there >>>> will be normal write. There is no duplication for the same space. >>> >>> Ah. Then write_begin() really initialize after i_size until page cache >>> boudary for append write? I wonder if this patch works correctly for >>> mmap. >> Since you already provided me review comments to check truncate and >> mmap, we checked all points for those cases. > > cluster size == 512b > > 1) create new file > 2) fallocate 100MB > 3) write(2) data for each 512b > > With this, write_begin() will be called for each 512b data. When we > allocates new page for this file, write_begin() writes data 0-512. Then, > we have to initialize 512-4096 by zero. Because mmap read maps 0-4096, > even if i_size == 512. > > Who is initializing area for 512-4096? >From other view, I guess fat_zero_falloc_area() is for filling zero for 0-10000, in the following case? 1) create new file 2) lseek(10000) 3) write data by write(2) This job is for cont_write_begin(). If example is correct, why cont_write_begin() doesn't work? I guess, because get_block() doesn't set buffer_new() for those area. If above is correct, right implement to change get_block(). Thanks. -- OGAWA Hirofumi -- 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/