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 <[email protected]>

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 <[email protected]> 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 <[email protected]>
> 
> 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 <[email protected]>
> > ---
> >  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 [email protected]
> > 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 [email protected]
> 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to