Hi Jaegeuk, Thank you for your action. To reflect review comments, I'm preparing the v2 patch. I included your changes in the v2 patch and rebased to the dev-test tree, so that it can be applied on top of Damien's patches. Your review for the v2 patch will be appreciated.
-- Best Regards, Shin'ichiro Kawasaki On 4/17/19 4:51 AM, Jaegeuk Kim wrote: > 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
