Commit 8f18cf13 authored by Chris Mason's avatar Chris Mason

Btrfs: Make the resizer work based on shrinking and growing devices

Signed-off-by: default avatarChris Mason <chris.mason@oracle.com>
parent 5e478dc9
...@@ -474,6 +474,7 @@ struct btrfs_block_group_cache { ...@@ -474,6 +474,7 @@ struct btrfs_block_group_cache {
u64 pinned; u64 pinned;
u64 flags; u64 flags;
int cached; int cached;
int ro;
}; };
struct btrfs_device; struct btrfs_device;
......
...@@ -818,6 +818,10 @@ struct btrfs_root *btrfs_read_fs_root_no_name(struct btrfs_fs_info *fs_info, ...@@ -818,6 +818,10 @@ struct btrfs_root *btrfs_read_fs_root_no_name(struct btrfs_fs_info *fs_info,
return fs_info->tree_root; return fs_info->tree_root;
if (location->objectid == BTRFS_EXTENT_TREE_OBJECTID) if (location->objectid == BTRFS_EXTENT_TREE_OBJECTID)
return fs_info->extent_root; return fs_info->extent_root;
if (location->objectid == BTRFS_CHUNK_TREE_OBJECTID)
return fs_info->chunk_root;
if (location->objectid == BTRFS_DEV_TREE_OBJECTID)
return fs_info->dev_root;
root = radix_tree_lookup(&fs_info->fs_roots_radix, root = radix_tree_lookup(&fs_info->fs_roots_radix,
(unsigned long)location->objectid); (unsigned long)location->objectid);
......
...@@ -187,6 +187,7 @@ static int noinline find_search_start(struct btrfs_root *root, ...@@ -187,6 +187,7 @@ static int noinline find_search_start(struct btrfs_root *root,
if (!cache) if (!cache)
goto out; goto out;
total_fs_bytes = btrfs_super_total_bytes(&root->fs_info->super_copy); total_fs_bytes = btrfs_super_total_bytes(&root->fs_info->super_copy);
free_space_cache = &root->fs_info->free_space_cache; free_space_cache = &root->fs_info->free_space_cache;
...@@ -196,7 +197,7 @@ static int noinline find_search_start(struct btrfs_root *root, ...@@ -196,7 +197,7 @@ static int noinline find_search_start(struct btrfs_root *root,
goto out; goto out;
last = max(search_start, cache->key.objectid); last = max(search_start, cache->key.objectid);
if (!block_group_bits(cache, data)) { if (!block_group_bits(cache, data) || cache->ro) {
goto new_group; goto new_group;
} }
...@@ -221,6 +222,8 @@ static int noinline find_search_start(struct btrfs_root *root, ...@@ -221,6 +222,8 @@ static int noinline find_search_start(struct btrfs_root *root,
continue; continue;
} }
spin_unlock_irq(&free_space_cache->lock); spin_unlock_irq(&free_space_cache->lock);
if (cache->ro)
goto new_group;
if (start + num > cache->key.objectid + cache->key.offset) if (start + num > cache->key.objectid + cache->key.offset)
goto new_group; goto new_group;
if (start + num > total_fs_bytes) if (start + num > total_fs_bytes)
...@@ -319,7 +322,7 @@ struct btrfs_block_group_cache *btrfs_find_block_group(struct btrfs_root *root, ...@@ -319,7 +322,7 @@ struct btrfs_block_group_cache *btrfs_find_block_group(struct btrfs_root *root,
if (search_start && search_start < total_fs_bytes) { if (search_start && search_start < total_fs_bytes) {
struct btrfs_block_group_cache *shint; struct btrfs_block_group_cache *shint;
shint = btrfs_lookup_block_group(info, search_start); shint = btrfs_lookup_block_group(info, search_start);
if (shint && block_group_bits(shint, data)) { if (shint && block_group_bits(shint, data) && !shint->ro) {
used = btrfs_block_group_used(&shint->item); used = btrfs_block_group_used(&shint->item);
if (used + shint->pinned < if (used + shint->pinned <
div_factor(shint->key.offset, factor)) { div_factor(shint->key.offset, factor)) {
...@@ -327,7 +330,7 @@ struct btrfs_block_group_cache *btrfs_find_block_group(struct btrfs_root *root, ...@@ -327,7 +330,7 @@ struct btrfs_block_group_cache *btrfs_find_block_group(struct btrfs_root *root,
} }
} }
} }
if (hint && block_group_bits(hint, data) && if (hint && !hint->ro && block_group_bits(hint, data) &&
hint->key.objectid < total_fs_bytes) { hint->key.objectid < total_fs_bytes) {
used = btrfs_block_group_used(&hint->item); used = btrfs_block_group_used(&hint->item);
if (used + hint->pinned < if (used + hint->pinned <
...@@ -364,7 +367,7 @@ struct btrfs_block_group_cache *btrfs_find_block_group(struct btrfs_root *root, ...@@ -364,7 +367,7 @@ struct btrfs_block_group_cache *btrfs_find_block_group(struct btrfs_root *root,
if (cache->key.objectid > total_fs_bytes) if (cache->key.objectid > total_fs_bytes)
break; break;
if (block_group_bits(cache, data)) { if (!cache->ro && block_group_bits(cache, data)) {
if (full_search) if (full_search)
free_check = cache->key.offset; free_check = cache->key.offset;
else else
...@@ -1020,6 +1023,7 @@ static int update_space_info(struct btrfs_fs_info *info, u64 flags, ...@@ -1020,6 +1023,7 @@ static int update_space_info(struct btrfs_fs_info *info, u64 flags,
if (found) { if (found) {
found->total_bytes += total_bytes; found->total_bytes += total_bytes;
found->bytes_used += bytes_used; found->bytes_used += bytes_used;
found->full = 0;
WARN_ON(found->total_bytes < found->bytes_used); WARN_ON(found->total_bytes < found->bytes_used);
*space_info = found; *space_info = found;
return 0; return 0;
...@@ -1700,7 +1704,6 @@ int btrfs_alloc_extent(struct btrfs_trans_handle *trans, ...@@ -1700,7 +1704,6 @@ int btrfs_alloc_extent(struct btrfs_trans_handle *trans,
u64 super_used; u64 super_used;
u64 root_used; u64 root_used;
u64 search_start = 0; u64 search_start = 0;
u64 new_hint;
u64 alloc_profile; u64 alloc_profile;
u32 sizes[2]; u32 sizes[2];
struct btrfs_fs_info *info = root->fs_info; struct btrfs_fs_info *info = root->fs_info;
...@@ -1724,7 +1727,7 @@ int btrfs_alloc_extent(struct btrfs_trans_handle *trans, ...@@ -1724,7 +1727,7 @@ int btrfs_alloc_extent(struct btrfs_trans_handle *trans,
data = BTRFS_BLOCK_GROUP_METADATA | alloc_profile; data = BTRFS_BLOCK_GROUP_METADATA | alloc_profile;
} }
again: again:
if (root->ref_cows) { if (root != root->fs_info->extent_root) {
if (!(data & BTRFS_BLOCK_GROUP_METADATA)) { if (!(data & BTRFS_BLOCK_GROUP_METADATA)) {
ret = do_chunk_alloc(trans, root->fs_info->extent_root, ret = do_chunk_alloc(trans, root->fs_info->extent_root,
2 * 1024 * 1024, 2 * 1024 * 1024,
...@@ -1738,10 +1741,6 @@ int btrfs_alloc_extent(struct btrfs_trans_handle *trans, ...@@ -1738,10 +1741,6 @@ int btrfs_alloc_extent(struct btrfs_trans_handle *trans,
BUG_ON(ret); BUG_ON(ret);
} }
new_hint = max(hint_byte, root->fs_info->alloc_start);
if (new_hint < btrfs_super_total_bytes(&info->super_copy))
hint_byte = new_hint;
WARN_ON(num_bytes < root->sectorsize); WARN_ON(num_bytes < root->sectorsize);
ret = find_free_extent(trans, root, num_bytes, empty_size, ret = find_free_extent(trans, root, num_bytes, empty_size,
search_start, search_end, hint_byte, ins, search_start, search_end, hint_byte, ins,
...@@ -2473,15 +2472,16 @@ static int noinline relocate_one_extent(struct btrfs_root *extent_root, ...@@ -2473,15 +2472,16 @@ static int noinline relocate_one_extent(struct btrfs_root *extent_root,
return ret; return ret;
} }
int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 new_size) int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 shrink_start)
{ {
struct btrfs_trans_handle *trans; struct btrfs_trans_handle *trans;
struct btrfs_root *tree_root = root->fs_info->tree_root; struct btrfs_root *tree_root = root->fs_info->tree_root;
struct btrfs_path *path; struct btrfs_path *path;
u64 cur_byte; u64 cur_byte;
u64 total_found; u64 total_found;
u64 shrink_last_byte;
struct btrfs_block_group_cache *shrink_block_group;
struct btrfs_fs_info *info = root->fs_info; struct btrfs_fs_info *info = root->fs_info;
struct extent_io_tree *block_group_cache;
struct btrfs_key key; struct btrfs_key key;
struct btrfs_key found_key; struct btrfs_key found_key;
struct extent_buffer *leaf; struct extent_buffer *leaf;
...@@ -2489,17 +2489,29 @@ int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 new_size) ...@@ -2489,17 +2489,29 @@ int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 new_size)
int ret; int ret;
int progress = 0; int progress = 0;
btrfs_set_super_total_bytes(&info->super_copy, new_size); shrink_block_group = btrfs_lookup_block_group(root->fs_info,
clear_extent_dirty(&info->free_space_cache, new_size, (u64)-1, shrink_start);
GFP_NOFS); BUG_ON(!shrink_block_group);
block_group_cache = &info->block_group_cache;
shrink_last_byte = shrink_start + shrink_block_group->key.offset;
shrink_block_group->space_info->total_bytes -=
shrink_block_group->key.offset;
printk("shrink_extent_tree %Lu -> %Lu type %Lu\n", shrink_start, shrink_last_byte, shrink_block_group->flags);
path = btrfs_alloc_path(); path = btrfs_alloc_path();
root = root->fs_info->extent_root; root = root->fs_info->extent_root;
path->reada = 2; path->reada = 2;
again: again:
trans = btrfs_start_transaction(root, 1);
do_chunk_alloc(trans, root->fs_info->extent_root,
btrfs_block_group_used(&shrink_block_group->item) +
2 * 1024 * 1024, shrink_block_group->flags);
btrfs_end_transaction(trans, root);
shrink_block_group->ro = 1;
total_found = 0; total_found = 0;
key.objectid = new_size; key.objectid = shrink_start;
key.offset = 0; key.offset = 0;
key.type = 0; key.type = 0;
cur_byte = key.objectid; cur_byte = key.objectid;
...@@ -2511,10 +2523,12 @@ int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 new_size) ...@@ -2511,10 +2523,12 @@ int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 new_size)
ret = btrfs_previous_item(root, path, 0, BTRFS_EXTENT_ITEM_KEY); ret = btrfs_previous_item(root, path, 0, BTRFS_EXTENT_ITEM_KEY);
if (ret < 0) if (ret < 0)
goto out; goto out;
if (ret == 0) { if (ret == 0) {
leaf = path->nodes[0]; leaf = path->nodes[0];
btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]); btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
if (found_key.objectid + found_key.offset > new_size) { if (found_key.objectid + found_key.offset > shrink_start &&
found_key.objectid < shrink_last_byte) {
cur_byte = found_key.objectid; cur_byte = found_key.objectid;
key.objectid = cur_byte; key.objectid = cur_byte;
} }
...@@ -2543,6 +2557,9 @@ int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 new_size) ...@@ -2543,6 +2557,9 @@ int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 new_size)
btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]); btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
if (found_key.objectid >= shrink_last_byte)
break;
if (progress && need_resched()) { if (progress && need_resched()) {
memcpy(&key, &found_key, sizeof(key)); memcpy(&key, &found_key, sizeof(key));
mutex_unlock(&root->fs_info->fs_mutex); mutex_unlock(&root->fs_info->fs_mutex);
...@@ -2583,68 +2600,31 @@ int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 new_size) ...@@ -2583,68 +2600,31 @@ int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 new_size)
goto again; goto again;
} }
/*
* we've freed all the extents, now remove the block
* group item from the tree
*/
trans = btrfs_start_transaction(root, 1); trans = btrfs_start_transaction(root, 1);
key.objectid = new_size; memcpy(&key, &shrink_block_group->key, sizeof(key));
key.offset = 0;
key.type = 0;
while(1) {
u64 ptr;
ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
if (ret < 0)
goto out;
leaf = path->nodes[0];
nritems = btrfs_header_nritems(leaf);
bg_next:
if (path->slots[0] >= nritems) {
ret = btrfs_next_leaf(root, path);
if (ret < 0)
break;
if (ret == 1) {
ret = 0;
break;
}
leaf = path->nodes[0];
btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
/* ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
* btrfs_next_leaf doesn't cow buffers, we have to if (ret > 0)
* do the search again ret = -EIO;
*/ if (ret < 0)
memcpy(&key, &found_key, sizeof(key)); goto out;
btrfs_release_path(root, path);
goto resched_check;
}
btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]); leaf = path->nodes[0];
if (btrfs_key_type(&found_key) != BTRFS_BLOCK_GROUP_ITEM_KEY) { nritems = btrfs_header_nritems(leaf);
printk("shrinker found key %Lu %u %Lu\n", btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
found_key.objectid, found_key.type, kfree(shrink_block_group);
found_key.offset);
path->slots[0]++;
goto bg_next;
}
ret = get_state_private(&info->block_group_cache,
found_key.objectid, &ptr);
if (!ret)
kfree((void *)(unsigned long)ptr);
clear_extent_bits(&info->block_group_cache, found_key.objectid, clear_extent_bits(&info->block_group_cache, found_key.objectid,
found_key.objectid + found_key.offset - 1, found_key.objectid + found_key.offset - 1,
(unsigned int)-1, GFP_NOFS); (unsigned int)-1, GFP_NOFS);
key.objectid = found_key.objectid + 1; btrfs_del_item(trans, root, path);
btrfs_del_item(trans, root, path); clear_extent_dirty(&info->free_space_cache,
btrfs_release_path(root, path); shrink_start, shrink_last_byte - 1,
resched_check:
if (need_resched()) {
mutex_unlock(&root->fs_info->fs_mutex);
cond_resched();
mutex_lock(&root->fs_info->fs_mutex);
}
}
clear_extent_dirty(&info->free_space_cache, new_size, (u64)-1,
GFP_NOFS); GFP_NOFS);
btrfs_commit_transaction(trans, root); btrfs_commit_transaction(trans, root);
out: out:
...@@ -2652,13 +2632,6 @@ int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 new_size) ...@@ -2652,13 +2632,6 @@ int btrfs_shrink_extent_tree(struct btrfs_root *root, u64 new_size)
return ret; return ret;
} }
int btrfs_grow_extent_tree(struct btrfs_trans_handle *trans,
struct btrfs_root *root, u64 new_size)
{
btrfs_set_super_total_bytes(&root->fs_info->super_copy, new_size);
return 0;
}
int find_first_block_group(struct btrfs_root *root, struct btrfs_path *path, int find_first_block_group(struct btrfs_root *root, struct btrfs_path *path,
struct btrfs_key *key) struct btrfs_key *key)
{ {
...@@ -2726,7 +2699,7 @@ int btrfs_read_block_groups(struct btrfs_root *root) ...@@ -2726,7 +2699,7 @@ int btrfs_read_block_groups(struct btrfs_root *root)
leaf = path->nodes[0]; leaf = path->nodes[0];
btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]); btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
cache = kmalloc(sizeof(*cache), GFP_NOFS); cache = kzalloc(sizeof(*cache), GFP_NOFS);
if (!cache) { if (!cache) {
ret = -ENOMEM; ret = -ENOMEM;
break; break;
...@@ -2736,8 +2709,6 @@ int btrfs_read_block_groups(struct btrfs_root *root) ...@@ -2736,8 +2709,6 @@ int btrfs_read_block_groups(struct btrfs_root *root)
btrfs_item_ptr_offset(leaf, path->slots[0]), btrfs_item_ptr_offset(leaf, path->slots[0]),
sizeof(cache->item)); sizeof(cache->item));
memcpy(&cache->key, &found_key, sizeof(found_key)); memcpy(&cache->key, &found_key, sizeof(found_key));
cache->cached = 0;
cache->pinned = 0;
key.objectid = found_key.objectid + found_key.offset; key.objectid = found_key.objectid + found_key.offset;
btrfs_release_path(root, path); btrfs_release_path(root, path);
...@@ -2789,12 +2760,10 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans, ...@@ -2789,12 +2760,10 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans,
extent_root = root->fs_info->extent_root; extent_root = root->fs_info->extent_root;
block_group_cache = &root->fs_info->block_group_cache; block_group_cache = &root->fs_info->block_group_cache;
cache = kmalloc(sizeof(*cache), GFP_NOFS); cache = kzalloc(sizeof(*cache), GFP_NOFS);
BUG_ON(!cache); BUG_ON(!cache);
cache->key.objectid = chunk_offset; cache->key.objectid = chunk_offset;
cache->key.offset = size; cache->key.offset = size;
cache->cached = 0;
cache->pinned = 0;
btrfs_set_key_type(&cache->key, BTRFS_BLOCK_GROUP_ITEM_KEY); btrfs_set_key_type(&cache->key, BTRFS_BLOCK_GROUP_ITEM_KEY);
memset(&cache->item, 0, sizeof(cache->item)); memset(&cache->item, 0, sizeof(cache->item));
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
* Boston, MA 021110-1307, USA. * Boston, MA 021110-1307, USA.
*/ */
#include <linux/kernel.h>
#include <linux/bio.h> #include <linux/bio.h>
#include <linux/buffer_head.h> #include <linux/buffer_head.h>
#include <linux/fs.h> #include <linux/fs.h>
...@@ -2887,9 +2888,12 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg) ...@@ -2887,9 +2888,12 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg)
{ {
u64 new_size; u64 new_size;
u64 old_size; u64 old_size;
u64 devid = 1;
struct btrfs_ioctl_vol_args *vol_args; struct btrfs_ioctl_vol_args *vol_args;
struct btrfs_trans_handle *trans; struct btrfs_trans_handle *trans;
struct btrfs_device *device = NULL;
char *sizestr; char *sizestr;
char *devstr = NULL;
int ret = 0; int ret = 0;
int namelen; int namelen;
int mod = 0; int mod = 0;
...@@ -2909,9 +2913,25 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg) ...@@ -2909,9 +2913,25 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg)
goto out; goto out;
} }
mutex_lock(&root->fs_info->fs_mutex);
sizestr = vol_args->name; sizestr = vol_args->name;
devstr = strchr(sizestr, ':');
if (devstr) {
char *end;
sizestr = devstr + 1;
*devstr = '\0';
devstr = vol_args->name;
devid = simple_strtoull(devstr, &end, 10);
printk("resizing devid %Lu\n", devid);
}
device = btrfs_find_device(root, devid, NULL);
if (!device) {
printk("resizer unable to find device %Lu\n", devid);
ret = -EINVAL;
goto out_unlock;
}
if (!strcmp(sizestr, "max")) if (!strcmp(sizestr, "max"))
new_size = root->fs_info->sb->s_bdev->bd_inode->i_size; new_size = device->bdev->bd_inode->i_size;
else { else {
if (sizestr[0] == '-') { if (sizestr[0] == '-') {
mod = -1; mod = -1;
...@@ -2923,12 +2943,11 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg) ...@@ -2923,12 +2943,11 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg)
new_size = btrfs_parse_size(sizestr); new_size = btrfs_parse_size(sizestr);
if (new_size == 0) { if (new_size == 0) {
ret = -EINVAL; ret = -EINVAL;
goto out; goto out_unlock;
} }
} }
mutex_lock(&root->fs_info->fs_mutex); old_size = device->total_bytes;
old_size = btrfs_super_total_bytes(&root->fs_info->super_copy);
if (mod < 0) { if (mod < 0) {
if (new_size > old_size) { if (new_size > old_size) {
...@@ -2944,7 +2963,7 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg) ...@@ -2944,7 +2963,7 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg)
ret = -EINVAL; ret = -EINVAL;
goto out_unlock; goto out_unlock;
} }
if (new_size > root->fs_info->sb->s_bdev->bd_inode->i_size) { if (new_size > device->bdev->bd_inode->i_size) {
ret = -EFBIG; ret = -EFBIG;
goto out_unlock; goto out_unlock;
} }
...@@ -2952,13 +2971,14 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg) ...@@ -2952,13 +2971,14 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg)
do_div(new_size, root->sectorsize); do_div(new_size, root->sectorsize);
new_size *= root->sectorsize; new_size *= root->sectorsize;
printk("new size is %Lu\n", new_size); printk("new size for %s is %llu\n", device->name, (unsigned long long)new_size);
if (new_size > old_size) { if (new_size > old_size) {
trans = btrfs_start_transaction(root, 1); trans = btrfs_start_transaction(root, 1);
ret = btrfs_grow_extent_tree(trans, root, new_size); ret = btrfs_grow_device(trans, device, new_size);
btrfs_commit_transaction(trans, root); btrfs_commit_transaction(trans, root);
} else { } else {
ret = btrfs_shrink_extent_tree(root, new_size); ret = btrfs_shrink_device(device, new_size);
} }
out_unlock: out_unlock:
......
This diff is collapsed.
...@@ -128,4 +128,9 @@ int btrfs_cleanup_fs_uuids(void); ...@@ -128,4 +128,9 @@ int btrfs_cleanup_fs_uuids(void);
int btrfs_num_copies(struct btrfs_mapping_tree *map_tree, u64 logical, u64 len); int btrfs_num_copies(struct btrfs_mapping_tree *map_tree, u64 logical, u64 len);
int btrfs_unplug_page(struct btrfs_mapping_tree *map_tree, int btrfs_unplug_page(struct btrfs_mapping_tree *map_tree,
u64 logical, struct page *page); u64 logical, struct page *page);
int btrfs_grow_device(struct btrfs_trans_handle *trans,
struct btrfs_device *device, u64 new_size);
struct btrfs_device *btrfs_find_device(struct btrfs_root *root, u64 devid,
u8 *uuid);
int btrfs_shrink_device(struct btrfs_device *device, u64 new_size);
#endif #endif
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment