Hi Shin'ichiro,

I had to modify this patch on top of Damien's patches.

Could you please review and test this?

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test

Thanks,

On 04/10, Shin'ichiro Kawasaki wrote:
> The mkfs.f2fs checks 'zoned' and 'chunk_sectors' sysfs attributes of
> zoned block devices to place F2FS areas aligned to block zone
> boundaries. However, mkfs.f2fs code assumes the given device name has
> corresponding sysfs path under /sys/block directory. This is not the
> case when partition devices or symbolic links are specified as the
> command line argument of mkfs.f2fs. In such a case, mkfs.f2fs fails to
> read sysfs attributes for zoned block devices, handles the device as a
> regular block device, and do not align F2FS areas to the block zone
> boundaries. This results in F2FS write failures because of zone write
> pointer miss-alignment in sequential write required zones.
> 
> Change mkfs.f2fs to handle sysfs path correctly. Read symbolic link
> to get canonical sysfs path. If the device is a partition, cut the last
> directory name in the sysfs path to refer the holder device attributes.
> 
> Also introduce read_file() and read_sys_attr() helper functions for
> code simplicity.
> 
> Signed-off-by: Shin'ichiro Kawasaki <[email protected]>
> ---
>  lib/libf2fs_zoned.c | 134 ++++++++++++++++++++++++++++++--------------
>  1 file changed, 91 insertions(+), 43 deletions(-)
> 
> diff --git a/lib/libf2fs_zoned.c b/lib/libf2fs_zoned.c
> index 6e32f32..e645004 100644
> --- a/lib/libf2fs_zoned.c
> +++ b/lib/libf2fs_zoned.c
> @@ -8,6 +8,7 @@
>   */
>  #define _LARGEFILE64_SOURCE
>  
> +#define _GNU_SOURCE
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -15,6 +16,8 @@
>  #include <unistd.h>
>  #include <fcntl.h>
>  #include <sys/stat.h>
> +#include <sys/sysmacros.h>
> +#include <linux/limits.h>
>  #ifndef ANDROID_WINDOWS_HOST
>  #include <sys/ioctl.h>
>  #endif
> @@ -24,67 +27,112 @@
>  
>  #ifdef HAVE_LINUX_BLKZONED_H
>  
> +/*
> + * Read up to 255 characters from the first line of a file. Strip the 
> trailing
> + * newline.
> + */
> +static char *read_file(const char *path)
> +{
> +     char line[256], *p = line;
> +     FILE *f;
> +
> +     f = fopen(path, "rb");
> +     if (!f)
> +             return NULL;
> +     if (!fgets(line, sizeof(line), f))
> +             line[0] = '\0';
> +     strsep(&p, "\n");
> +     fclose(f);
> +
> +     return strdup(line);
> +}
> +
> +static char *read_sys_attr(const char *dev_path, const char *attr)
> +{
> +     struct stat statbuf;
> +     char *sys_devno_path = NULL;
> +     char sys_path[PATH_MAX];
> +     ssize_t sz;
> +     char *part_attr_path = NULL;
> +     char *part_str = NULL;
> +     char *delim = NULL;
> +     char *attr_path = NULL;
> +     char *attr_str = NULL;
> +
> +     if (stat(dev_path, &statbuf) < 0)
> +             goto out;
> +
> +     if (asprintf(&sys_devno_path, "/sys/dev/block/%d:%d",
> +                  major(statbuf.st_rdev), minor(statbuf.st_rdev)) < 0)
> +             goto out;
> +
> +     sz = readlink(sys_devno_path, sys_path, sizeof(sys_path) - 1);
> +     if (sz < 0)
> +             goto out;
> +     sys_path[sz] = '\0';
> +
> +     /*
> +      * If the device is a partition device, cut the device name in the
> +      * canonical sysfs path to obtain the sysfs path of the holder device.
> +      *   e.g.:  /sys/devices/.../sda/sda1 -> /sys/devices/.../sda
> +      */
> +     if (asprintf(&part_attr_path, "/sys/dev/block/%s/partition",
> +                  sys_path) < 0)
> +             goto out;
> +     part_str = read_file(part_attr_path);
> +     if (part_str && *part_str == '1') {
> +             delim = strrchr(sys_path, '/');
> +             if (!delim)
> +                     goto out;
> +             *delim = '\0';
> +     }
> +
> +     if (asprintf(&attr_path, "/sys/dev/block/%s/%s", sys_path, attr) < 0)
> +             goto out;
> +
> +     attr_str = read_file(attr_path);
> +out:
> +     free(attr_path);
> +     free(part_str);
> +     free(part_attr_path);
> +     free(sys_devno_path);
> +     return attr_str;
> +}
> +
>  void f2fs_get_zoned_model(int i)
>  {
>       struct device_info *dev = c.devices + i;
> -     char str[128];
> -     FILE *file;
> -     int res;
> -
> -     /* Check that this is a zoned block device */
> -     snprintf(str, sizeof(str),
> -              "/sys/block/%s/queue/zoned",
> -              basename(dev->path));
> -     file = fopen(str, "r");
> -     if (!file)
> -             goto not_zoned;
> -
> -     memset(str, 0, sizeof(str));
> -     res = fscanf(file, "%s", str);
> -     fclose(file);
> -
> -     if (res != 1)
> -             goto not_zoned;
> -
> -     if (strcmp(str, "host-aware") == 0) {
> -             dev->zoned_model = F2FS_ZONED_HA;
> +     char *model_str;
> +
> +     dev->zoned_model = F2FS_ZONED_NONE;
> +
> +     model_str = read_sys_attr(dev->path, "queue/zoned");
> +     if (!model_str)
>               return;
> -     }
> -     if (strcmp(str, "host-managed") == 0) {
> +
> +     if (strcmp(model_str, "host-aware") == 0)
> +             dev->zoned_model = F2FS_ZONED_HA;
> +     else if (strcmp(model_str, "host-managed") == 0)
>               dev->zoned_model = F2FS_ZONED_HM;
> -             return;
> -     }
>  
> -not_zoned:
> -     dev->zoned_model = F2FS_ZONED_NONE;
> +     free(model_str);
>  }
>  
>  int f2fs_get_zone_blocks(int i)
>  {
>       struct device_info *dev = c.devices + i;
>       uint64_t sectors;
> -     char str[128];
> -     FILE *file;
> -     int res;
> +     char * cs_str;
>  
>       /* Get zone size */
>       dev->zone_blocks = 0;
>  
> -     snprintf(str, sizeof(str),
> -              "/sys/block/%s/queue/chunk_sectors",
> -              basename(dev->path));
> -     file = fopen(str, "r");
> -     if (!file)
> -             return -1;
> -
> -     memset(str, 0, sizeof(str));
> -     res = fscanf(file, "%s", str);
> -     fclose(file);
> -
> -     if (res != 1)
> +     cs_str = read_sys_attr(dev->path, "queue/chunk_sectors");
> +     if (!cs_str)
>               return -1;
>  
> -     sectors = atol(str);
> +     sectors = atol(cs_str);
> +     free(cs_str);
>       if (!sectors)
>               return -1;
>  
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to