| Message ID | 1307604489-7884-1-git-send-email-lidongyang@novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 09, 2011 at 03:28:09PM +0800, Li Dongyang wrote: > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -45,6 +45,7 @@ struct btrfs_device { > int running_pending; > u64 generation; > > + int discard; can you pick a better name? this does not describe that it's the capability of the device, but rather 'do a discard'. something like has_discard, can_discard, has_trim, etc > int writeable; > int in_fs_metadata; > int missing; otherwise the patch looks good (and matches my view how to do it). I will test it eventually. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday, June 10, 2011 08:00:17 AM David Sterba wrote: > On Thu, Jun 09, 2011 at 03:28:09PM +0800, Li Dongyang wrote: > > --- a/fs/btrfs/volumes.h > > +++ b/fs/btrfs/volumes.h > > @@ -45,6 +45,7 @@ struct btrfs_device { > > int running_pending; > > u64 generation; > > > > + int discard; > > can you pick a better name? this does not describe that it's the > capability of the device, but rather 'do a discard'. I feel the same, I picked the name because there was btrfs_device->barriers, and it was removed in commit c3b9a62c8f932f32a733d6b628f61f3f28345727 > > something like has_discard, can_discard, has_trim, etc > > > int writeable; > > int in_fs_metadata; > > int missing; > > otherwise the patch looks good (and matches my view how to do it). I > will test it eventually. Thanks a lot, I'll resend this this a proper name. > > > david > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 10, 2011 at 01:40:31PM +0800, Li Dongyang wrote: > > otherwise the patch looks good (and matches my view how to do it). I > > will test it eventually. > Thanks a lot, I'll resend this this a proper name. JFYI, tested in the previous scenario, ie. all devices without trim support, does not crash. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 5b9b6b6..507cf8d 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -1774,7 +1774,6 @@ static int btrfs_discard_extent(struct btrfs_root *root, u64 bytenr, u64 discarded_bytes = 0; struct btrfs_multi_bio *multi = NULL; - /* Tell the block device(s) that the sectors can be discarded */ ret = btrfs_map_block(&root->fs_info->mapping_tree, REQ_DISCARD, bytenr, &num_bytes, &multi, 0); @@ -1782,25 +1781,26 @@ static int btrfs_discard_extent(struct btrfs_root *root, u64 bytenr, struct btrfs_bio_stripe *stripe = multi->stripes; int i; - for (i = 0; i < multi->num_stripes; i++, stripe++) { - ret = btrfs_issue_discard(stripe->dev->bdev, - stripe->physical, - stripe->length); - if (!ret) - discarded_bytes += stripe->length; - else if (ret != -EOPNOTSUPP) - break; + if (stripe->dev->discard) { + ret = btrfs_issue_discard(stripe->dev->bdev, + stripe->physical, + stripe->length); + if (!ret) + discarded_bytes += stripe->length; + else if (ret == -EOPNOTSUPP) { + stripe->dev->discard = 0; + ret = 0; + } else + break; + } } kfree(multi); } - if (discarded_bytes && ret == -EOPNOTSUPP) - ret = 0; if (actual_bytes) *actual_bytes = discarded_bytes; - return ret; } diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index da541df..bdf5604 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -341,6 +341,7 @@ static noinline int device_list_add(const char *path, device->work.func = pending_bios_fn; memcpy(device->uuid, disk_super->dev_item.uuid, BTRFS_UUID_SIZE); + device->discard = 1; spin_lock_init(&device->io_lock); device->name = kstrdup(path, GFP_NOFS); if (!device->name) { @@ -408,6 +409,7 @@ static struct btrfs_fs_devices *clone_fs_devices(struct btrfs_fs_devices *orig) device->devid = orig_dev->devid; device->work.func = pending_bios_fn; memcpy(device->uuid, orig_dev->uuid, sizeof(device->uuid)); + device->discard = 1; spin_lock_init(&device->io_lock); INIT_LIST_HEAD(&device->dev_list); INIT_LIST_HEAD(&device->dev_alloc_list); @@ -1616,6 +1618,7 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path) lock_chunks(root); device->writeable = 1; + device->discard = 1; device->work.func = pending_bios_fn; generate_random_uuid(device->uuid); spin_lock_init(&device->io_lock); @@ -3346,6 +3349,7 @@ static struct btrfs_device *add_missing_dev(struct btrfs_root *root, return NULL; list_add(&device->dev_list, &fs_devices->devices); + device->discard = 1; device->dev_root = root->fs_info->dev_root; device->devid = devid; device->work.func = pending_bios_fn; diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 7c12d61..9f7e56c 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -45,6 +45,7 @@ struct btrfs_device { int running_pending; u64 generation; + int discard; int writeable; int in_fs_metadata; int missing;
With discard flag in btrfs_device, we will only push trim request to the devices support that. Now we don't return EOPNOTSUPP to the caller, so we won't trigger BUG_ONs in the walk_log_tree functions if we mount a drive without DISCARD using -o discard, but it is still possible if we get errors from blkdev_issue_discard. This won't affect the return value of fstrim on the drives without DISCARD, because we've already checked that in btrfs_ioctl_fitrim, Thanks Signed-off-by: Li Dongyang <lidongyang@novell.com> --- fs/btrfs/extent-tree.c | 24 ++++++++++++------------ fs/btrfs/volumes.c | 4 ++++ fs/btrfs/volumes.h | 1 + 3 files changed, 17 insertions(+), 12 deletions(-)