On 07/20/2012 09:15 PM, Josef Bacik wrote:
> SSD's do not gain anything by having metadata DUP turned on.  The underlying
> file system that is a part of all SSD's could easily map duplicate metadat

If I understood correctly you are stating that because an SSD *might*
"eliminates the benefit of duplicating the metadata"  mkfs.btrfs *must*
remove _silently_ this behaviour on all SSD ?

To me it seems too strong; or almost it should be documented in the man
page and/or issuing a warning during the format process.

> blocks into the same erase block which effectively eliminates the benefit of
> duplicating the metadata on disk.  So detect if we are formatting a single
> SSD drive and if we are do not use DUP.  Thanks,
> 
> Signed-off-by: Josef Bacik <jba...@fusionio.com>
> ---
> V1->V2: use blkid to get the full disk in case we happen to be formatting a
> partition.
> 
>  Makefile |    2 +-
>  mkfs.c   |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 9694444..d827216 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -65,7 +65,7 @@ btrfsck: $(objects) btrfsck.o
>       $(CC) $(CFLAGS) -o btrfsck btrfsck.o $(objects) $(LDFLAGS) $(LIBS)
>  
>  mkfs.btrfs: $(objects) mkfs.o
> -     $(CC) $(CFLAGS) -o mkfs.btrfs $(objects) mkfs.o $(LDFLAGS) $(LIBS)
> +     $(CC) $(CFLAGS) -o mkfs.btrfs $(objects) mkfs.o $(LDFLAGS) $(LIBS) 
> -lblkid
>  
>  btrfs-debug-tree: $(objects) debug-tree.o
>       $(CC) $(CFLAGS) -o btrfs-debug-tree $(objects) debug-tree.o $(LDFLAGS) 
> $(LIBS)
> diff --git a/mkfs.c b/mkfs.c
> index dff5eb8..fc2b6ed 100644
> --- a/mkfs.c
> +++ b/mkfs.c
> @@ -37,6 +37,7 @@
>  #include <linux/fs.h>
>  #include <ctype.h>
>  #include <attr/xattr.h>
> +#include <blkid/blkid.h>
>  #include "kerncompat.h"
>  #include "ctree.h"
>  #include "disk-io.h"
> @@ -234,7 +235,7 @@ static int create_one_raid_group(struct 
> btrfs_trans_handle *trans,
>  static int create_raid_groups(struct btrfs_trans_handle *trans,
>                             struct btrfs_root *root, u64 data_profile,
>                             int data_profile_opt, u64 metadata_profile,
> -                           int metadata_profile_opt, int mixed)
> +                           int metadata_profile_opt, int mixed, int ssd)
>  {
>       u64 num_devices = btrfs_super_num_devices(&root->fs_info->super_copy);
>       u64 allowed;
> @@ -246,7 +247,7 @@ static int create_raid_groups(struct btrfs_trans_handle 
> *trans,
>        */
>       if (!metadata_profile_opt && !mixed) {

Please put something like

+               if(ssd)
+                   printf("SSD detected: remove the metadata duplication;");


>               metadata_profile = (num_devices > 1) ?
> -                     BTRFS_BLOCK_GROUP_RAID1 : BTRFS_BLOCK_GROUP_DUP;
> +                     BTRFS_BLOCK_GROUP_RAID1 : (ssd) ? 0: 
> BTRFS_BLOCK_GROUP_DUP;
>       }
>       if (!data_profile_opt && !mixed) {
>               data_profile = (num_devices > 1) ?
> @@ -1201,6 +1202,49 @@ static int zero_output_file(int out_fd, u64 size, u32 
> sectorsize)
>       return ret;
>  }
>  
> +static int is_ssd(const char *file)
> +{
> +     char *devname;
> +     blkid_probe probe;
> +     char *dev;
> +     char path[PATH_MAX];
> +     dev_t disk;
> +     int fd;
> +     char rotational;
> +
> +     probe = blkid_new_probe_from_filename(file);
> +     if (!probe)
> +             return 0;
> +
> +     /*
> +      * We want to use blkid_devno_to_wholedisk() but it's broken for some
> +      * reason on F17 at least so we'll do this trickery
> +      */
> +     disk = blkid_probe_get_wholedisk_devno(probe);
> +     devname = blkid_devno_to_devname(disk);
> +
> +     dev = strrchr(devname, '/');
> +     dev++;
> +
> +     snprintf(path, PATH_MAX, "/sys/block/%s/queue/rotational", dev);
> +
> +     free(devname);
> +     blkid_free_probe(probe);
> +
> +     fd = open(path, O_RDONLY);
> +     if (fd < 0) {
> +             return 0;
> +     }
> +
> +     if (read(fd, &rotational, sizeof(char)) < sizeof(char)) {
> +             close(fd);
> +             return 0;
> +     }
> +     close(fd);
> +
> +     return !atoi((const char *)&rotational);
> +}
> +
>  int main(int ac, char **av)
>  {
>       char *file;
> @@ -1227,6 +1271,7 @@ int main(int ac, char **av)
>       int data_profile_opt = 0;
>       int metadata_profile_opt = 0;
>       int nodiscard = 0;
> +     int ssd = 0;
>  
>       char *source_dir = NULL;
>       int source_dir_set = 0;
> @@ -1352,6 +1397,9 @@ int main(int ac, char **av)
>                       exit(1);
>               }
>       }
> +
> +     ssd = is_ssd(file);
> +


>       if (mixed) {
>               if (metadata_profile != data_profile) {
>                       fprintf(stderr, "With mixed block groups data and 
> metadata "
> @@ -1438,7 +1486,7 @@ raid_groups:
>       if (!source_dir_set) {
>               ret = create_raid_groups(trans, root, data_profile,
>                                data_profile_opt, metadata_profile,
> -                              metadata_profile_opt, mixed);
> +                              metadata_profile_opt, mixed, ssd);
>               BUG_ON(ret);
>       }
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to