Hi Shin'ichiro,
On 2019/4/10 16:56, 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);
It needs to handle the error of read_file() here?
Thanks,
> + 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;
>
>
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
