| Message ID | 7c56077a080b9ab77d1a722cb3bdde50e83895c4.1646589622.git.kreijack@inwind.it (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: allocation_hint | expand |
On Sun, Mar 06, 2022 at 07:14:41PM +0100, Goffredo Baroncelli wrote: > From: Goffredo Baroncelli <kreijack@inwind.it> > > This patch allow to change the allocation_hint property writing > a numerical value in the file. > > /sysfs/fs/btrfs/<UUID>/devinfo/<devid>/allocation_hint > > To update this field it is added the property "allocation_hint" in > btrfs-prog too. > > Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it> > --- > fs/btrfs/sysfs.c | 76 +++++++++++++++++++++++++++++++++++++++++++++- > fs/btrfs/volumes.c | 2 +- > fs/btrfs/volumes.h | 2 ++ > 3 files changed, 78 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c > index 59d92a385a96..c6723456c0e1 100644 > --- a/fs/btrfs/sysfs.c > +++ b/fs/btrfs/sysfs.c > @@ -1606,7 +1606,81 @@ static ssize_t btrfs_devinfo_allocation_hint_show(struct kobject *kobj, > } > return scnprintf(buf, PAGE_SIZE, "<UNKNOWN>\n"); > } > -BTRFS_ATTR(devid, allocation_hint, btrfs_devinfo_allocation_hint_show); > + > +static ssize_t btrfs_devinfo_allocation_hint_store(struct kobject *kobj, > + struct kobj_attribute *a, > + const char *buf, size_t len) If you're using vim, use set cindent set cino=(0 This will give you the proper formatting we expect, this should be something like statice ssize_t btrfs_devinfo_allocation_hint_store(struct kobject *kobj, struct kobj_attribute *a, const char *buf, size_t len) > +{ > + struct btrfs_fs_info *fs_info; > + struct btrfs_root *root; > + struct btrfs_device *device; > + int ret; > + struct btrfs_trans_handle *trans; > + int i, l; > + u64 type, prev_type; > + > + if (len < 1) > + return -EINVAL; > + > + /* remove trailing newline */ > + l = len; > + if (buf[len-1] == '\n') > + l--; > + This is unnecessary, because lower you can do... > + for (i = 0 ; i < ARRAY_SIZE(allocation_hint_name) ; i++) { > + if (l != strlen(allocation_hint_name[i].name)) > + continue; > + > + if (strncasecmp(allocation_hint_name[i].name, buf, l)) > + continue; > + strmatch(buf, allocation_hint_name[i].name) I would make a separate patch to change strmatch to do strncasecmp instead, but then you can just use that helper. > + type = allocation_hint_name[i].value; > + break; > + } > + > + if (i >= ARRAY_SIZE(allocation_hint_name)) > + return -EINVAL; > + > + device = container_of(kobj, struct btrfs_device, devid_kobj); > + fs_info = device->fs_info; > + if (!fs_info) > + return -EPERM; > + > + root = fs_info->chunk_root; > + if (sb_rdonly(fs_info->sb)) > + return -EROFS; > + > + /* check if a change is really needed */ > + if ((device->type & BTRFS_DEV_ALLOCATION_HINT_MASK) == type) > + return len; > + > + trans = btrfs_start_transaction(root, 1); > + if (IS_ERR(trans)) > + return PTR_ERR(trans); > + > + prev_type = device->type; > + device->type = (device->type & ~BTRFS_DEV_ALLOCATION_HINT_MASK) | type; > + > + ret = btrfs_update_device(trans, device); > + > + if (ret < 0) { > + btrfs_abort_transaction(trans, ret); > + btrfs_end_transaction(trans); > + goto abort; > + } > + > + ret = btrfs_commit_transaction(trans); > + if (ret < 0) > + goto abort; > + > + return len; > +abort: > + device->type = prev_type; > + return ret; Extra whitespace here. > +} > +BTRFS_ATTR_RW(devid, allocation_hint, btrfs_devinfo_allocation_hint_show, > + btrfs_devinfo_allocation_hint_store); > + > > /* > * Information about one device. > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 5e3e13d4940b..d4ac90f5c949 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -2846,7 +2846,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > return ret; > } > > -static noinline int btrfs_update_device(struct btrfs_trans_handle *trans, > +noinline int btrfs_update_device(struct btrfs_trans_handle *trans, > struct btrfs_device *device) > { > int ret; You can drop the noinline thing here as well, and make sure to fix the indentation. > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index bd297f23d19e..93ac27d8097c 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -636,5 +636,7 @@ int btrfs_bg_type_to_factor(u64 flags); > const char *btrfs_bg_type_to_raid_name(u64 flags); > int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info); > bool btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical); > +int btrfs_update_device(struct btrfs_trans_handle *trans, > + struct btrfs_device *device); Make the indentation correct. Thanks, Josef
On 22/03/2022 20.19, Josef Bacik wrote: > On Sun, Mar 06, 2022 at 07:14:41PM +0100, Goffredo Baroncelli wrote: >> From: Goffredo Baroncelli <kreijack@inwind.it> >> >> This patch allow to change the allocation_hint property writing >> a numerical value in the file. >> >> /sysfs/fs/btrfs/<UUID>/devinfo/<devid>/allocation_hint >> >> To update this field it is added the property "allocation_hint" in >> btrfs-prog too. >> >> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it> >> --- >> fs/btrfs/sysfs.c | 76 +++++++++++++++++++++++++++++++++++++++++++++- >> fs/btrfs/volumes.c | 2 +- >> fs/btrfs/volumes.h | 2 ++ >> 3 files changed, 78 insertions(+), 2 deletions(-) >> >> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c >> index 59d92a385a96..c6723456c0e1 100644 >> --- a/fs/btrfs/sysfs.c >> +++ b/fs/btrfs/sysfs.c >> @@ -1606,7 +1606,81 @@ static ssize_t btrfs_devinfo_allocation_hint_show(struct kobject *kobj, >> } >> return scnprintf(buf, PAGE_SIZE, "<UNKNOWN>\n"); >> } >> -BTRFS_ATTR(devid, allocation_hint, btrfs_devinfo_allocation_hint_show); >> + >> +static ssize_t btrfs_devinfo_allocation_hint_store(struct kobject *kobj, >> + struct kobj_attribute *a, >> + const char *buf, size_t len) > > If you're using vim, use > > set cindent > set cino=(0 > > This will give you the proper formatting we expect, this should be something > like > > statice ssize_t btrfs_devinfo_allocation_hint_store(struct kobject *kobj, > struct kobj_attribute *a, > const char *buf, size_t len) > > >> +{ >> + struct btrfs_fs_info *fs_info; >> + struct btrfs_root *root; >> + struct btrfs_device *device; >> + int ret; >> + struct btrfs_trans_handle *trans; >> + int i, l; >> + u64 type, prev_type; >> + >> + if (len < 1) >> + return -EINVAL; >> + >> + /* remove trailing newline */ >> + l = len; >> + if (buf[len-1] == '\n') >> + l--; >> + > > This is unnecessary, because lower you can do... > >> + for (i = 0 ; i < ARRAY_SIZE(allocation_hint_name) ; i++) { >> + if (l != strlen(allocation_hint_name[i].name)) >> + continue; >> + >> + if (strncasecmp(allocation_hint_name[i].name, buf, l)) >> + continue; >> + > > strmatch(buf, allocation_hint_name[i].name) > > I would make a separate patch to change strmatch to do strncasecmp instead, but > then you can just use that helper. Looking at the code of strmatch() /* * Look for an exact string @string in @buffer with possible leading or * trailing whitespace */ static bool strmatch(const char *buffer, const char *string) { const size_t len = strlen(string); /* Skip leading whitespace */ buffer = skip_spaces(buffer); /* Match entire string, check if the rest is whitespace or empty */ if (strncmp(string, buffer, len) == 0 && strlen(skip_spaces(buffer + len)) == 0) return true; return false; } Is buf always zero terminated ? > >> + type = allocation_hint_name[i].value; >> + break; >> + } >> + >> + if (i >= ARRAY_SIZE(allocation_hint_name)) >> + return -EINVAL; >> + >> + device = container_of(kobj, struct btrfs_device, devid_kobj); >> + fs_info = device->fs_info; >> + if (!fs_info) >> + return -EPERM; >> + >> + root = fs_info->chunk_root; >> + if (sb_rdonly(fs_info->sb)) >> + return -EROFS; >> + >> + /* check if a change is really needed */ >> + if ((device->type & BTRFS_DEV_ALLOCATION_HINT_MASK) == type) >> + return len; >> + >> + trans = btrfs_start_transaction(root, 1); >> + if (IS_ERR(trans)) >> + return PTR_ERR(trans); >> + >> + prev_type = device->type; >> + device->type = (device->type & ~BTRFS_DEV_ALLOCATION_HINT_MASK) | type; >> + >> + ret = btrfs_update_device(trans, device); >> + >> + if (ret < 0) { >> + btrfs_abort_transaction(trans, ret); >> + btrfs_end_transaction(trans); >> + goto abort; >> + } >> + >> + ret = btrfs_commit_transaction(trans); >> + if (ret < 0) >> + goto abort; >> + >> + return len; >> +abort: >> + device->type = prev_type; >> + return ret; > > Extra whitespace here. Ok > >> +} >> +BTRFS_ATTR_RW(devid, allocation_hint, btrfs_devinfo_allocation_hint_show, >> + btrfs_devinfo_allocation_hint_store); >> + >> >> /* >> * Information about one device. >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index 5e3e13d4940b..d4ac90f5c949 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -2846,7 +2846,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path >> return ret; >> } >> >> -static noinline int btrfs_update_device(struct btrfs_trans_handle *trans, >> +noinline int btrfs_update_device(struct btrfs_trans_handle *trans, >> struct btrfs_device *device) >> { >> int ret; > > You can drop the noinline thing here as well, and make sure to fix the > indentation. Ok > >> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h >> index bd297f23d19e..93ac27d8097c 100644 >> --- a/fs/btrfs/volumes.h >> +++ b/fs/btrfs/volumes.h >> @@ -636,5 +636,7 @@ int btrfs_bg_type_to_factor(u64 flags); >> const char *btrfs_bg_type_to_raid_name(u64 flags); >> int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info); >> bool btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical); >> +int btrfs_update_device(struct btrfs_trans_handle *trans, >> + struct btrfs_device *device); > > Make the indentation correct. Thanks, It is correct. Are the '+' and the tab that misaligned the patch. > > Josef
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 59d92a385a96..c6723456c0e1 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -1606,7 +1606,81 @@ static ssize_t btrfs_devinfo_allocation_hint_show(struct kobject *kobj, } return scnprintf(buf, PAGE_SIZE, "<UNKNOWN>\n"); } -BTRFS_ATTR(devid, allocation_hint, btrfs_devinfo_allocation_hint_show); + +static ssize_t btrfs_devinfo_allocation_hint_store(struct kobject *kobj, + struct kobj_attribute *a, + const char *buf, size_t len) +{ + struct btrfs_fs_info *fs_info; + struct btrfs_root *root; + struct btrfs_device *device; + int ret; + struct btrfs_trans_handle *trans; + int i, l; + u64 type, prev_type; + + if (len < 1) + return -EINVAL; + + /* remove trailing newline */ + l = len; + if (buf[len-1] == '\n') + l--; + + for (i = 0 ; i < ARRAY_SIZE(allocation_hint_name) ; i++) { + if (l != strlen(allocation_hint_name[i].name)) + continue; + + if (strncasecmp(allocation_hint_name[i].name, buf, l)) + continue; + + type = allocation_hint_name[i].value; + break; + } + + if (i >= ARRAY_SIZE(allocation_hint_name)) + return -EINVAL; + + device = container_of(kobj, struct btrfs_device, devid_kobj); + fs_info = device->fs_info; + if (!fs_info) + return -EPERM; + + root = fs_info->chunk_root; + if (sb_rdonly(fs_info->sb)) + return -EROFS; + + /* check if a change is really needed */ + if ((device->type & BTRFS_DEV_ALLOCATION_HINT_MASK) == type) + return len; + + trans = btrfs_start_transaction(root, 1); + if (IS_ERR(trans)) + return PTR_ERR(trans); + + prev_type = device->type; + device->type = (device->type & ~BTRFS_DEV_ALLOCATION_HINT_MASK) | type; + + ret = btrfs_update_device(trans, device); + + if (ret < 0) { + btrfs_abort_transaction(trans, ret); + btrfs_end_transaction(trans); + goto abort; + } + + ret = btrfs_commit_transaction(trans); + if (ret < 0) + goto abort; + + return len; +abort: + device->type = prev_type; + return ret; +} +BTRFS_ATTR_RW(devid, allocation_hint, btrfs_devinfo_allocation_hint_show, + btrfs_devinfo_allocation_hint_store); + /* * Information about one device. diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 5e3e13d4940b..d4ac90f5c949 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -2846,7 +2846,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path return ret; } -static noinline int btrfs_update_device(struct btrfs_trans_handle *trans, +noinline int btrfs_update_device(struct btrfs_trans_handle *trans, struct btrfs_device *device) { int ret; diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index bd297f23d19e..93ac27d8097c 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -636,5 +636,7 @@ int btrfs_bg_type_to_factor(u64 flags); const char *btrfs_bg_type_to_raid_name(u64 flags); int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info); bool btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical); +int btrfs_update_device(struct btrfs_trans_handle *trans, + struct btrfs_device *device); #endif