From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kyungmin Park Subject: Re: [PATCH v6] fat: Batched discard support for fat Date: Tue, 29 Mar 2011 14:11:35 +0900 Message-ID: References: <20110328103431.GA22323@july> <87k4fi4iwq.fsf@devron.myhome.or.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Lukas Czerner To: OGAWA Hirofumi Return-path: Received: from mail-ww0-f42.google.com ([74.125.82.42]:40697 "EHLO mail-ww0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751396Ab1C2FLg convert rfc822-to-8bit (ORCPT ); Tue, 29 Mar 2011 01:11:36 -0400 In-Reply-To: <87k4fi4iwq.fsf@devron.myhome.or.jp> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Mar 29, 2011 at 2:04 PM, OGAWA Hirofumi wrote: > Kyungmin Park writes: > >> +int fat_trim_fs(struct super_block *sb, struct fstrim_range *range) >> +{ >> + =A0 =A0 struct msdos_sb_info *sbi =3D MSDOS_SB(sb); >> + =A0 =A0 struct fatent_operations *ops =3D sbi->fatent_ops; >> + =A0 =A0 struct fat_entry fatent; >> + =A0 =A0 unsigned long reada_blocks, reada_mask, cur_block; >> + =A0 =A0 int err, free, count, entry; >> + =A0 =A0 int start, len, minlen, trimmed; >> + >> + =A0 =A0 start =3D range->start >> sb->s_blocksize_bits; >> + =A0 =A0 len =3D range->len >> sb->s_blocksize_bits; >> + =A0 =A0 len =3D round_down(start + len, sbi->sec_per_clus); >> + =A0 =A0 start =3D round_up(start, sbi->sec_per_clus); >> + =A0 =A0 minlen =3D range->minlen >> sb->s_blocksize_bits; >> + =A0 =A0 minlen =3D round_up(minlen, sbi->sec_per_clus); >> + =A0 =A0 trimmed =3D 0; >> + =A0 =A0 count =3D 0; >> + =A0 =A0 err =3D -EINVAL; > > Sorry for didn't mention at previous. You can use ->cluster_size, and > ->cluster_bits. > >> + =A0 =A0 if (start >=3D sbi->max_cluster) >> + =A0 =A0 =A0 =A0 =A0 =A0 goto out; >> + >> + =A0 =A0 len =3D (len > sbi->max_cluster) ? sbi->max_cluster : len; > > [...] > >> + =A0 =A0 =A0 =A0 =A0 =A0 trimmed +=3D free; >> + =A0 =A0 } >> + =A0 =A0 range->len =3D (u64)(trimmed * sbi->sec_per_clus) << sb->s= _blocksize_bits; >> + =A0 =A0 fatent_brelse(&fatent); >> +out: >> + =A0 =A0 unlock_fat(sbi); >> + =A0 =A0 return err; > > Again, this ioctl's design is unclear, and seems to be strange. I > wouldn't want to add this before clearing it. Please explain what is > right behavior. Umm it's out of my scope. it's trim design. See also btrfs batched discard support. it's also no consideration as you mentioned. As I know, now xfs, ext4, and btrfs support this fstrim without these c= oncern. http://thread.gmane.org/gmane.comp.file-systems.btrfs/9758 Thank you, Kyungmin Park > > E.g. if user specified 0-1024 and FS data block was actually started = at > 2048. What is right behavior? And if the end of blocks, what returned= ? > For now, it seems to return range->len =3D=3D 0 on both cases. > > Well, so, my suggestion is providing this like flat one extent > file. I.e. FS have to map actual block placement to flat. And result > also like write/read (return bytes as trimed, and at EOF returns 0). > > Thanks. > -- > OGAWA Hirofumi > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdev= el" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at =A0http://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