Hi, thank you for the prompt review > * Although nilfs_test_opt(sbi, DISCARD) is used, > the declaration of NILFS_MOUNT_DISCARD is not included. > > * nilfs2.txt in Documentation/filesystems should be modified to > add the new discard option. grrr, I only committed under fs/ directory.. I'll revsize and resend the patch. thanks, regards -- Jiro SEKIBA <jir@xxxxxxxxx> At Fri, 29 Jan 2010 16:58:46 +0900 (JST), Ryusuke Konishi wrote: > > Hi, > > On Fri, 29 Jan 2010 13:14:56 +0900, Jiro SEKIBA <jir@xxxxxxxxx> wrote: > > Hi, > > > > I found the attached patch in experimental tree. > > It looks ok, but require mount option to enable the feature > > and modification for recent block device layer api. > > > > I haven't tried with device with "trim" command, but checked > > that blkdev_issue_discard is correctly issued. > > > > > > > > This adds a function to send discard requests for given array of > > segment numbers, and calls the function when garbage collection > > succeeded. > > > > Signed-off-by: Jiro SEKIBA <jir@xxxxxxxxx> > > Thank you. > > Roughly ok, but it seems to need a few correction: > > * Although nilfs_test_opt(sbi, DISCARD) is used, > the declaration of NILFS_MOUNT_DISCARD is not included. > > * nilfs2.txt in Documentation/filesystems should be modified to > add the new discard option. > > Thanks, > Ryusuke Konishi > > > Signed-off-by: Ryusuke Konishi <konishi.ryusuke@xxxxxxxxxxxxx> > > --- > > fs/nilfs2/segment.c | 3 +++ > > fs/nilfs2/super.c | 8 +++++++- > > fs/nilfs2/the_nilfs.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > > fs/nilfs2/the_nilfs.h | 1 + > > 4 files changed, 56 insertions(+), 1 deletions(-) > > > > diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c > > index 17584c5..db6869f 100644 > > --- a/fs/nilfs2/segment.c > > +++ b/fs/nilfs2/segment.c > > @@ -2560,6 +2560,9 @@ int nilfs_clean_segments(struct super_block *sb, struct nilfs_argv *argv, > > set_current_state(TASK_INTERRUPTIBLE); > > schedule_timeout(sci->sc_interval); > > } > > + if (nilfs_test_opt(sbi, DISCARD)) > > + nilfs_discard_segments(nilfs, sci->sc_freesegs, > > + sci->sc_nfreesegs); > > > > out_unlock: > > sci->sc_freesegs = NULL; > > diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c > > index 8173fae..3f88401 100644 > > --- a/fs/nilfs2/super.c > > +++ b/fs/nilfs2/super.c > > @@ -481,6 +481,8 @@ static int nilfs_show_options(struct seq_file *seq, struct vfsmount *vfs) > > seq_printf(seq, ",order=strict"); > > if (nilfs_test_opt(sbi, NORECOVERY)) > > seq_printf(seq, ",norecovery"); > > + if (nilfs_test_opt(sbi, DISCARD)) > > + seq_printf(seq, ",discard"); > > > > return 0; > > } > > @@ -550,7 +552,7 @@ static const struct export_operations nilfs_export_ops = { > > enum { > > Opt_err_cont, Opt_err_panic, Opt_err_ro, > > Opt_nobarrier, Opt_snapshot, Opt_order, Opt_norecovery, > > - Opt_err, > > + Opt_discard, Opt_err, > > }; > > > > static match_table_t tokens = { > > @@ -561,6 +563,7 @@ static match_table_t tokens = { > > {Opt_snapshot, "cp=%u"}, > > {Opt_order, "order=%s"}, > > {Opt_norecovery, "norecovery"}, > > + {Opt_discard, "discard"}, > > {Opt_err, NULL} > > }; > > > > @@ -614,6 +617,9 @@ static int parse_options(char *options, struct super_block *sb) > > case Opt_norecovery: > > nilfs_set_opt(sbi, NORECOVERY); > > break; > > + case Opt_discard: > > + nilfs_set_opt(sbi, DISCARD); > > + break; > > default: > > printk(KERN_ERR > > "NILFS: Unrecognized mount option \"%s\"\n", p); > > diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c > > index 6241e17..9ad5b83 100644 > > --- a/fs/nilfs2/the_nilfs.c > > +++ b/fs/nilfs2/the_nilfs.c > > @@ -646,6 +646,51 @@ int init_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi, char *data) > > goto out; > > } > > > > +void nilfs_discard_segments(struct the_nilfs *nilfs, __u64 *segnump, > > + size_t nsegs) > > +{ > > + sector_t seg_start, seg_end; > > + sector_t start = 0, nblocks = 0; > > + unsigned int sects_per_block; > > + __u64 *sn; > > + int ret = 0; > > + > > + sects_per_block = (1 << nilfs->ns_blocksize_bits) / > > + bdev_logical_block_size(nilfs->ns_bdev); > > + for (sn = segnump; sn < segnump + nsegs; sn++) { > > + nilfs_get_segment_range(nilfs, *sn, &seg_start, &seg_end); > > + > > + if (!nblocks) { > > + start = seg_start; > > + nblocks = seg_end - seg_start + 1; > > + } else if (start + nblocks == seg_start) { > > + nblocks += seg_end - seg_start + 1; > > + } else { > > + ret = blkdev_issue_discard(nilfs->ns_bdev, > > + start * sects_per_block, > > + nblocks * sects_per_block, > > + GFP_NOFS, > > + DISCARD_FL_BARRIER); > > + if (ret < 0) > > + goto failed; > > + nblocks = 0; > > + } > > + } > > + if (nblocks) { > > + ret = blkdev_issue_discard(nilfs->ns_bdev, > > + start * sects_per_block, > > + nblocks * sects_per_block, > > + GFP_NOFS, DISCARD_FL_BARRIER); > > + if (ret < 0) > > + goto failed; > > + } > > + return; > > + failed: > > + printk(KERN_WARNING "NILFS warning: error %d on discard request, " > > + "turning discards off for the device\n", ret); > > + nilfs_clear_opt(nilfs->ns_current, DISCARD); > > +} > > + > > int nilfs_count_free_blocks(struct the_nilfs *nilfs, sector_t *nblocks) > > { > > struct inode *dat = nilfs_dat_inode(nilfs); > > diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h > > index 589786e..8e3de6e 100644 > > --- a/fs/nilfs2/the_nilfs.h > > +++ b/fs/nilfs2/the_nilfs.h > > @@ -221,6 +221,7 @@ struct the_nilfs *find_or_create_nilfs(struct block_device *); > > void put_nilfs(struct the_nilfs *); > > int init_nilfs(struct the_nilfs *, struct nilfs_sb_info *, char *); > > int load_nilfs(struct the_nilfs *, struct nilfs_sb_info *); > > +void nilfs_discard_segments(struct the_nilfs *, __u64 *, size_t); > > int nilfs_count_free_blocks(struct the_nilfs *, sector_t *); > > struct nilfs_sb_info *nilfs_find_sbinfo(struct the_nilfs *, int, __u64); > > int nilfs_checkpoint_is_mounted(struct the_nilfs *, __u64, int); > > -- > > 1.5.6.5 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html