Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

vdev_copy_uberblocks() should use abd_alloc_linear() #6713

Closed
thegreatgazoo opened this issue Oct 3, 2017 · 1 comment
Closed

vdev_copy_uberblocks() should use abd_alloc_linear() #6713

thegreatgazoo opened this issue Oct 3, 2017 · 1 comment

Comments

@thegreatgazoo
Copy link

The vdev_copy_uberblocks() function uses abd_to_buf(ub_abd)) but allocates it with abd_alloc(VDEV_UBERBLOCK_SIZE(vd), B_TRUE):
https://github.com/zfsonlinux/zfs/blob/39f56627ae988d09b4e3803c01c22b2026b2310e/module/zfs/vdev_label.c#L1167
https://github.com/zfsonlinux/zfs/blob/39f56627ae988d09b4e3803c01c22b2026b2310e/module/zfs/vdev_label.c#L1179

When the vd's ashift is high, VDEV_UBERBLOCK_SIZE(vd) can be larger than PAGESIZE, which causes abd_alloc() to return a scattered ABD, which will panic abd_to_buf(ub_abd)).
@behlendorf
Copy link
Contributor

Good catch! We do have a test case which exercises 8k uberblocks, create-o_ashift.ksh, but we never use them as part of an expansion test would would run this code. It would be great if you could open a PR with the fix.

thegreatgazoo pushed a commit to thegreatgazoo/zfs that referenced this issue Oct 4, 2017
@huangheintel
vdev_copy_uberblocks() should use abd_alloc_linear()
4b9ff1f 
The vdev_copy_uberblocks() function should use abd_alloc_linear() to
allocate ub_abd, because abd_to_buf(ub_abd)) is used later.

Signed-off-by: Isaac Huang <he.huang@intel.com>
Closes openzfs#6713
@thegreatgazoo thegreatgazoo mentioned this issue Oct 4, 2017
Merged
behlendorf pushed a commit that referenced this issue Oct 6, 2017
@huangheintel @behlendorf
Use linear abd in vdev_copy_uberblocks()
eea2e24 
The vdev_copy_uberblocks() function should use abd_alloc_linear() to
allocate ub_abd, because abd_to_buf(ub_abd)) is used later.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Isaac Huang <he.huang@intel.com>
Closes #6718 
Closes #6713
aerusso pushed a commit to aerusso/zfs that referenced this issue Oct 11, 2017
@huangheintel @aerusso
Use linear abd in vdev_copy_uberblocks()
9d5be0a 
The vdev_copy_uberblocks() function should use abd_alloc_linear() to
allocate ub_abd, because abd_to_buf(ub_abd)) is used later.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Isaac Huang <he.huang@intel.com>
Closes openzfs#6718 
Closes openzfs#6713
tonyhutter pushed a commit that referenced this issue Oct 16, 2017
@huangheintel @tonyhutter
Use linear abd in vdev_copy_uberblocks()
b979482 
The vdev_copy_uberblocks() function should use abd_alloc_linear() to
allocate ub_abd, because abd_to_buf(ub_abd)) is used later.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Isaac Huang <he.huang@intel.com>
Closes #6718
Closes #6713
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@behlendorf @thegreatgazoo